Send timer reminder emails #16

laserlemon · 2015-04-21T13:24:02Z

If a user hasn't started a timer by a configured time in the morning, he or she receives an email reminder to either start a timer or to update the Zenefits PTO calendar. If the user already has PTO scheduled in the Zenefits calendar, a reminder is never sent.

TODO

Determine appropriate morning check-in time (10am Eastern)
~~Send reminders at the specified time in the user's time zone~~
Set up rake hourglass:send_timer_reminders on Heroku scheduler
Skip timer reminders for holidays and scheduled PTO
Set ZENEFITS_PTO_CALENDAR_URL in production
Set up rake hourglass:sync_pto on Heroku scheduler

laserlemon · 2015-04-21T14:24:25Z

My thought is that 10am Eastern is a fair time to expect a timer to have started, considering the expectation of attendance at the 9:30am standup.

Also, because the standup is always at 9:30am Eastern time, we might be able to punt for now on sending these reminders in the user's time zone.

This allows us to check timers at multiple times throughout the day without spamming people. This is especially useful so that we can time the email deliveries based on factors like the user's time zone or the application time zone's current UTC offset.

danielmorrison · 2015-04-21T18:08:13Z

app/interactors/send_timer_reminders.rb

+
+  def call
+    User.all.each do |user|
+      next if user.timer_reminder_sent_on.try(:today?)


You can do al this in SQL, because if you pass a TimeWithZone into a query, Rails will figure out the TZ stuff for you.

User.where.not(timer_reminder_sent_on: Date.today) i think.

🆒, I'll drop that in and see if it's 💚.

💣 The problem is that we want to continue past line 11 if the user's never had a timer reminder set, as well.

Or to the rescue!

User.where("timer_reminder_sent_on < ? OR timer_reminder_sent_on IS NULL", Date.current)

I think that performs an inner join so would only match users that have days recorded in hourglass.
The pto check is a more complicated query

Ok @laserlemon you're going to hate this and @trestrantham is going to love it ;)

User.where("timer_reminder_sent_on < ? OR timer_reminder_sent_on IS NULL", context.date). joins( User.arel_table.join(Day.arel_table, Arel::Nodes::OuterJoin).on( Day.arel_table[:user_id].eq(User.arel_table[:id]).and( Day.arel_table[:date].eq(context.date).and(Day.arel_table[:pto].eq(false)) ) ).join_sources )

Tests pass with this locally.

Ha! Remember what I said about clarity?

I'm going to go the route of assuming that the user already has a day recorded because of how often new hours are fetched for the day and because of how this particular interactor is scheduled for later in the morning. That also gives us the benefit of not having to call out to Harvest so we can keep this all in the model.

Updated in 9b74755.

laserlemon · 2015-04-21T22:04:49Z

👀 🙏

danielmorrison · 2015-04-22T14:53:24Z

Have you thought about putting the "sent" flag on the Day class? It would require you to create more Day records but might be clearer.

laserlemon · 2015-04-22T15:38:44Z

@danielmorrison What do you mean by the "sent" flag?

danielmorrison · 2015-04-22T16:06:32Z

Tracking when you sent the reminder

Daniel Morrison
[i] Collective Idea
http://collectiveidea.com
http://get.harmonyapp.com
44 East 8th Street, Suite 410
Holland, MI 49423
616-990-0155
daniel@collectiveidea.com

On Apr 22, 2015, at 11:38 AM, Steve Richert notifications@github.com wrote:

@danielmorrison What do you mean by the "sent" flag?

—
Reply to this email directly or view it on GitHub.

laserlemon · 2015-04-22T16:22:37Z

Oh, that's an interesting thought. I'll take a look when I have some time.

laserlemon · 2015-04-22T17:20:23Z

Yeah, storing that on the day is way better. Great idea. Implementing now.

laserlemon · 2015-04-22T17:54:32Z

app/models/day.rb

+      timer_reminder_sent: false
+    )
+  end
+


@danielmorrison I think this makes a lot more sense for querying for timer reminders.

laserlemon · 2015-04-23T20:51:28Z

❔

This ensures that nobody gets a timer reminder email before he or she is expected to be working.

This information will be used in analytics and in determining whether to send timer reminders to users.

This will be used to parse the PTO calendar provided by Zenefits.

This will run periodically to keep Hourglass in step with Zenefits, and should run before timer reminders are sent out so that nobody is sent a timer reminder on his or her day off.

The last thing we want to do is bug somebody on vacation!

This should be able to be run from the console for any day you like.

Rather than making an external call to Harvest, we can rely on the data we have being up to date enough to determine whether a user has started a timer yet today. This also removes the ability for this particular interactor to be callable with a custom date because it doesn't make as much sense in this context as it does for an interactor like FetchDailyHours. The change also fixes a bug where a user's timer_reminder_sent_on column was updated even if no reminder was necessary.

…rather than users. This makes it much easier to keep track of several days that have already had reminders sent, rather than just the last one.

This now makes more sense as timer reminders are controlled by individual days themselves rather than by user records.

laserlemon · 2015-04-24T12:57:48Z

This is now deployed for "real world" testing.

Send timer reminder emails

laserlemon added 2 commits April 21, 2015 09:06

Add an interactor to send reminders to users who haven't started timers

fb26c41

Add a rake task for sending timer reminder emails on weekdays

bc9d180

laserlemon added the WIP label Apr 21, 2015

laserlemon added 2 commits April 21, 2015 10:04

Eliminate a test ordering dependency

70420cd

Move the repeated Interactor#harvest method into its own concern

2dda66b

danielmorrison reviewed Apr 21, 2015
View reviewed changes

laserlemon removed the WIP label Apr 21, 2015

laserlemon reviewed Apr 22, 2015
View reviewed changes

laserlemon mentioned this pull request Apr 23, 2015

Refactor how the application deals with time ranges #17

Merged

laserlemon added 12 commits April 23, 2015 16:57

Update the hourglass:send_timer_reminders rake task to wait until 10am

29a52d8

This ensures that nobody gets a timer reminder email before he or she is expected to be working.

Add the days.pto column to track which days are Paid Time Off

3f4dfe5

This information will be used in analytics and in determining whether to send timer reminders to users.

Add the icalendar runtime gem dependency

1afa731

This will be used to parse the PTO calendar provided by Zenefits.

Add an interactor for pulling PTO down from Zenefits

440950d

Add the hourglass:sync_pto task

178d385

This will run periodically to keep Hourglass in step with Zenefits, and should run before timer reminders are sent out so that nobody is sent a timer reminder on his or her day off.

Avoid recreating days that have already been rolled up into months

56099ce

Skip timer reminder delivery for a user on a PTO day

a89fa7b

The last thing we want to do is bug somebody on vacation!

Only require WebMock in the test environment by default

526385e

Don't lock SendTimerReminders execution down to today

4b04061

This should be able to be run from the console for any day you like.

Swap users.timer_reminder_sent_on column for days.timer_reminder_sent

84dfb27

Refactor timer reminder delivery to read/write from/to days

62f1488

…rather than users. This makes it much easier to keep track of several days that have already had reminders sent, rather than just the last one.

Reintroduce the ability to send timer reminders for arbitrary days

c27529b

This now makes more sense as timer reminders are controlled by individual days themselves rather than by user records.

laserlemon force-pushed the timer-reminder branch from dc944c3 to c27529b Compare April 23, 2015 20:58

danielmorrison added a commit that referenced this pull request Apr 24, 2015

Merge pull request #16 from collectiveidea/timer-reminder

2e44fe2

Send timer reminder emails

danielmorrison merged commit 2e44fe2 into master Apr 24, 2015

danielmorrison deleted the timer-reminder branch April 24, 2015 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send timer reminder emails #16

Send timer reminder emails #16

laserlemon commented Apr 21, 2015

laserlemon commented Apr 21, 2015

danielmorrison Apr 21, 2015

danielmorrison Apr 21, 2015

laserlemon Apr 21, 2015

laserlemon Apr 21, 2015

danielmorrison Apr 21, 2015

albus522 Apr 21, 2015

danielmorrison Apr 21, 2015

laserlemon Apr 21, 2015

laserlemon Apr 21, 2015

laserlemon Apr 21, 2015

laserlemon commented Apr 21, 2015

danielmorrison commented Apr 22, 2015

laserlemon commented Apr 22, 2015

danielmorrison commented Apr 22, 2015

laserlemon commented Apr 22, 2015

laserlemon commented Apr 22, 2015

laserlemon Apr 22, 2015

laserlemon commented Apr 23, 2015

laserlemon commented Apr 24, 2015

Send timer reminder emails #16

Send timer reminder emails #16

Conversation

laserlemon commented Apr 21, 2015

TODO

laserlemon commented Apr 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laserlemon commented Apr 21, 2015

danielmorrison commented Apr 22, 2015

laserlemon commented Apr 22, 2015

danielmorrison commented Apr 22, 2015

laserlemon commented Apr 22, 2015

laserlemon commented Apr 22, 2015

Choose a reason for hiding this comment

laserlemon commented Apr 23, 2015

laserlemon commented Apr 24, 2015