Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand possible dates for reminder emails #4473

Open
2 tasks
cielf opened this issue Jun 23, 2024 · 16 comments · May be fixed by #4606
Open
2 tasks

Expand possible dates for reminder emails #4473

cielf opened this issue Jun 23, 2024 · 16 comments · May be fixed by #4606
Labels

Comments

@cielf
Copy link
Collaborator

cielf commented Jun 23, 2024

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
@jlandiseigsti
Copy link

I'm interested on taking this issue on – is it spoken for yet?

@cielf
Copy link
Collaborator Author

cielf commented Jul 11, 2024

@jlandiseigsti It's yours!

@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Jul 11, 2024
@jlandiseigsti
Copy link

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?

@jlandiseigsti
Copy link

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!

@cielf
Copy link
Collaborator Author

cielf commented Jul 12, 2024

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)

@cielf
Copy link
Collaborator Author

cielf commented Jul 12, 2024

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.

@cielf
Copy link
Collaborator Author

cielf commented Jul 12, 2024

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)

@jlandiseigsti
Copy link

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.

@cielf
Copy link
Collaborator Author

cielf commented Jul 15, 2024

Pointing @dorner at the above comment

@dorner
Copy link
Collaborator

dorner commented Jul 16, 2024

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.

Copy link
Contributor

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 github-actions bot added the stale label Aug 16, 2024
@jlandiseigsti jlandiseigsti linked a pull request Aug 22, 2024 that will close this issue
Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

Copy link
Contributor

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 github-actions bot added the stale label Sep 23, 2024
@cielf cielf removed the stale label Sep 23, 2024
Copy link
Contributor

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 github-actions bot added the stale label Oct 24, 2024
Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

@cielf
Copy link
Collaborator Author

cielf commented Oct 31, 2024

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!

@cielf cielf removed the stale label Oct 31, 2024
@cielf cielf added the Help Wanted Groomed + open to all! label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants