rubyforgood
Expand possible dates for reminder emails
Summary
Allow reminder emails to be sent out on different bases -- e.g. quarterly or on the 3rd Friday of the month
Details
This will impact the reminder email dates both on the organization and the partner group levels -- and will impact the view and edit of the organization and the partner group, as well as the sending out of the reminder emails (services/partners/fetch_partners_to_remind_now)
Expand how the banks can specify the reminder date -- so they can, for instance, send out reminders the third Thursday of every month, or every n months on the 5th,
Hints: Look at the icecube gem. We think that's what we need.
Criteria for completion
- [ ] expanded ability to specify reminder months
- [ ] tests to support
I wanted to list out the new possibilities we are including, so that I can ensure that they are all covered.
The reminder emails can be sent out:
Every n months On the xth (the 11th) OR On the first/second/third/fourth/last day of the week (eg Monday, Tuesday) OR On the first/second/third/fourth/last weekday/weekend day
Question: Are multiple reminders per month allowed?
Some brainstorming on backward compatibility:
Currently, reminder_day, in both the Organization and PartnerGroup models, is just an integer that represents the day of the month. My thought was to add a new attribute, something like reminder_schedule, that would be theIceCube::Schedule converted to either an iCal string or a hash.
We could change the logic in the FetchPartnersToRemindNowService so that it returns a partner to notify if either the reminder_schedule or reminder_day matches the current day, and have a comment that the reminder_schedule is preferred. The new and edit pages could always create reminder_schedules so future organizations would use that, but reminder_day is still in place for older orgs.
Or we could figure out what it takes to remove reminder_day. It would involve some creating reminder_schedules for all the existing models that currently use day and updating the db. This feels a little scary to me since it's A) running a lot of SQL on a lot of info and B) timing-sensitive, as this would be a breaking change and the release would have to match it.
I'm inclined towards option A over B, but wanted to hear from those who know this app better than I do. Perhaps there is a C that I am overlooking!
Instead of backward compatibility, I would lean to migrating the current data to the new way of doing things (in my experience, that is the rails way of doing things)
As for multiple reminders per month, I don't think we'd be reminding the same partners more than once a month at this point.
Also " On the first/second/third/fourth/last weekday/weekend day" I'm not sure we need this, but if we get it "for free" I'm not going to say to suppress it. (Though I am curious what it does about holidays)
Thanks for the quick responses! One other question: the gem recurring_select has built a slick front end element for this: however, the last update was six months ago. If anyone knows of a similar front end gem that is updated a little more regularly, let me know; I'm a little hesitant to use something that may not be well-maintained.
Ideally we'd use a straight JavaScript module rather than a gem... but I spent some time googling and couldn't find any that weren't part of a huge framework. 😦
I agree that recurring_select might not be the best choice - this issue concerns me, for example. But to the best of my ability I couldn't find anything else. ice_cube seems to be pretty well maintained, so if it's not a ton of work to build a frontend on top of that, I'd probably like that better. But if it seems daunting, I'm OK with using recurring_select. Worse comes to worst, we can fork the gem ourselves.
This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.
This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.
If you're interested in picking this up, please continue on with @jlandiseigsti's PR, #4606. It needs some functional tweaking, but is pretty close!
A question during office hours about how to calculate the next deadline expanded into a broader question about how the reminders work.
@cielf, here is how things currently work.
At a high level: theFetchPartnersToRemindNowService determines which partners need to be sent reminders and the DeadlineService calculates the next deadline date which is included in the email.
To answer the question that came up in office hours: if reminders aren't enabled for a partner group, partners in that group get reminders based on the org's reminder schedule if the individual partner's send_reminders field is set to true. If a partner group has reminders set up, partners in that group will get reminders regardless of their individual send_reminders setting.
Now, the nitty gritty.
FetchPartnersToRemindNowService returns a list of partners to email based on two filters.
If a partner is part of a group with reminders set up (reminder_schedule AND deadline_day are not nil), that partner is added to the list if the partner group’s reminder schedule is set to occur on the current time.
- The
send_remindersfield ofPartnerGroupisn’t used in this case. - It is currently possible to click the “Do you want to send deadline reminders to them every month?” checkbox and then save the partner group with a
deadline_daybut NOreminder_schedule. So the partner group technically has reminders enabled, but partners in that group won't pass this filter.
If a partner has reminders enabled (that is Partner.send_reminders is true; this DOESN’T care about PartnerGroup.send_reminders ) is part of a group with no reminders set up (reminder_schedule is nil; deadline_day can be whatever) but is part of an org with reminders (reminder_schedule AND deadline_day are not nil, also the org isn’t deactivated), that partner is added to the list if the org’s reminder schedule is set to occur on the current time.
- Individual partners must have their reminders enabled, while that wasn’t the case in the previous filter.
- This filter doesn’t care about the
PartnerGroup.deadline_dayeven though, as we will see, that can influence the next deadline calculation - It could be intentional that the partner group form can save with a
deadline_daybut noreminder_scheduleas a way to let you override the org wide deadline day? If that’s the case, the language used in the form needs to be changed. To me, this smells like bug as opposed to an intentional feature.
DeadlineService calculates the next deadline for a given partner. It simply uses the partner’s partner group deadline day, or org deadline day if the former isn’t available, and returns that day next month.
- This is just a service that needs to be updated to account for the updates this issue makes to reminder emails. My assumption is that whichever reminder schedule is used (org vs partner_group), the associated deadline day should be used in the calculation?
- Possible it would be easier to define deadline_day as “the deadline is X days after the reminder is sent out” given the new options for the reminder schedule? For example, if the reminder goes out on the first Monday every month and the deadline is on the 21st, the number of days between the reminder and the deadline will vary from month to month.
Let me know if anything is unclear. Going forward, I think we need to first straighten out the filters in FetchPartnersToRemindNowService, then tell DeadlineService which deadline day to use based on which filter the partner passes through.
"This is just a service that needs to be updated to account for the updates this issue makes to reminder emails. My assumption is that whichever reminder schedule is used (org vs partner_group), the associated deadline day should be used in the calculation?"
I'm assuming there is a reason that we set up the reminders and deadlines the way we did in terms of overriding. In any case, we shouldn't change them without a good reason, as this would impact current users.
Should we improve the messaging around that so it is clearer? Absolutely. A statement to the effect of "This value, if set, overrides the (reminder date / deadline date) in your organization" on the partner group edit screen may do the trick.
We'll want to do something around the partner-level enabling reminders, too. If we don't already. Because it seems to me that it should only be available if it makes a difference. Can that wait for another PR? I think so.
"Possible it would be easier to define deadline_day as “the deadline is X days after the reminder is sent out” given the new options for the reminder schedule? For example, if the reminder goes out on the first Monday every month and the deadline is on the 21st, the number of days between the reminder and the deadline will vary from month to month."
Nope. I don't think that would be easier from the business pov. They probably have the same deadline -- whether it's, like, the 15th, or the second Tuesday, every month -- and I'm pretty sure that they don't think in terms of "the deadline is 5 days after we sent out the reminder".
@cielf The diagram looks correct.
It's fair that the issue around the deadline day is maybe more one of messaging. For example, the partner group reminder schedule form could let the user know if there is an org deadline day that will be overridden.
You mentioned the deadline could be the second Tuesday, which is something the deadline day currently can't express. Do you think it makes sense to rework it to be similar to the updated reminder schedule, with options for Xth day of the month and Xth weekday?
Ooops! We haven't been asked for that -- it was just a possibility in my head. Let's start with the reminder schedule, and see if anyone asks for similar for the deadline.
@dorner Currently, the reminder/deadline form uses some Javascript in deadline_day_pickers.js to calculate when the next reminder/deadline will be as the user changes the form to make it clear what will happen. It needs to be updated to handle the reminder being on the Xth weekday.
Since I'm updating it anyway, does it make sense to re-implement it as a stimulus controller?
@dorner @cielf I have an implementation question. I have two ideas on how to rework the deadline_day_pickers.js.
The first is to write some JS to calculate the next reminder/deadline day based off the form inputs. I'm hesitant to implement this solution as it will, fundamentally, involve writing a chunk of JS that has to replicate the icecube gem's logic.
The second is to have the JS make an AJAX request to get that same calculation from the icecube gem itself. However, this obviously is an extra AJAX request in a form that is supposed to be responsive.
Which approach do you prefer?
@Benjamin-Couey is there a JavaScript library you could use instead that does something similar? Maybe something like https://www.npmjs.com/package/rrule ?
@Benjamin-Couey is there a JavaScript library you could use instead that does something similar? Maybe something like https://www.npmjs.com/package/rrule ?
I had gotten it into my head we were trying to avoid adding more libraries on this issue. I'll take a look and see what I can find.
This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.