API-7308 sparkql date function support for day_of_week, day_of_year, and weekdays.

[wlmcewen]

When I brought up this feature in grooming, my understanding of the business need here was simply that the queries needed to know if a date was a specific day of the week (e.g. no Tuesday's allowed). However, it seems the more needed feature was actually to articulate a query using a range of weekdays from a date. The specific use case that just came in could still have been accomplished with the original functions, but it would have been a nasty db query:

(day_of_week(now()) Eq 5 And ShowingDate Le days(16) Or
day_of_week(now()) Eq 6 And ShowingDate Le days(15) Or
ShowingDate Le days(14))

Would now be:

ShowingDate Le weekdays(10)

I steered outside the original estimate to add in this brain teaser of a problem. I believe I've tested the bounds fairly well, but I'm hoping you all would nitpick my approach, or point out some logical or testing holes.

I've also included some project cleanup and rubocop'd the files I touched. These should be in separate commits, so focus on: e5be999 + ac7a0b8. Link for the shift+click struggle bus

One last note: Marcy and I did discuss support for some sort of automatic exclusion for holidays. This seems like a much bigger feature than what we have discussed so far, and we've managed to push back on that as a need for now. This is why I explicitly used the function weekdays() even though the ideal feature would amount to a function for business_days(). I'm booting that idea down the road as far as I can send it.

[wlmcewen]

This is the main test I was concerned about.

[wlmcewen]

And here is the new function. It turned out rather well, just don't look at my first implementation 🤣

[wlmcewen]

Shout out to Mr. Lobdell who helped me realize -4/5 == -1. This is the workaround I came up with to drop the remainder.

[wlmcewen]

Oh right, I did make an opinionated decision here on what to do with zero on a weekend. My take is to move it to the following Monday. I've compared this with some online calculators, and they just didn't support zero, which I would be OK with as well.

[codygustafson]

I'm having a hard time following with the rubocop changes here. From the description, what I had imagined was something like:

(day_of_week(ShowingDate) Eq 5 or day_of_week(ShowingDate) Eq 6) And ShowDate Bt ...

I think I'm missing some context here, is all.

How does this:

(day_of_week(now()) Eq 5 And ShowingDate Le days(16) Or
day_of_week(now()) Eq 6 And ShowingDate Le days(15) Or
ShowingDate Le days(14))

get reduced to this?

ShowingDate Le days(14)

Doh, I should have read the next sentence before continuing to try and decipher the code

I've also included some project cleanup and rubocop'd the files I touched. These should be in separate commits, so focus on

Looking now.

[wlmcewen]

get reduced to this?
ShowingDate Le days(14)

Sorry, that was some bad copy-pasta from my conversations with Marcy. I've corrected my summary to use the weekdays() function instead. Seems you're starting to see the purpose here. 👍

[codygustafson]

Alright, I think I've gotten the gist here. I do like the shortening of the query for the client. My main concern is, the same with the days() function. That is, you can't decide things like timezone differences and such. But for comparing dates, not times, it should work great.

[wlmcewen]

@joshcom and @codygustafson, I needed to make one last change here, as I forgot that sparkql doesn't support underscores for field or function names. I could add that in, but thought renaming my functions to be consistent with what's already here may be reasonable. Does 'dayofyear' or 'doy' and 'dayofweek' or 'dow' fit the bill?

[codygustafson]

I would vote for dayofyear but I'll let @joshcom speak up.

[joshcom]

dayofyear/dayofweek is more consistent, when compared to other functions (e.g. toupper). Let's go with those.

[wlmcewen]

Thanks guys, I'll update that and merge this on through!