Skip to content

Commit a95a278

Browse files
4473 expand reminder date possibilities (#5190)
* Replaces reminder_day with reminder_schedule Via db migrations, replaces the simple integer `reminder_day` with the more complex `reminder_schedule` which is an ical string that can be parsed to a repeating Schedule class. Adjusts the logic in the `fetch_partners_to_reminder_now_service` to use the repeating schedule. Updates related tests. * Adds input for the reminder schedule in the Edit Partner Group page Builds an ActiveModel, ReminderSchedule, which takes the necessary information and turns it into an IceCubeSchedule, which is what ultimate get saved in the db. Builds the form for this reminder schedule. Still needs to conditionally show or hide sections depending on if the user wants date or day of the week. * Adds Reminder Schedule fields to New and Edit Organization Now either while creating new Organization/Partner Groups or editing them, all the transfers from Reminder Date to Reminder Day have been made. * Makes the day of week or date hide depending on selection This uses css for ease and accessibility. * Adds tests and does refactoring Adds a number of refactors, including moving all the create_schedule logic to the deadlinable helper, and creates unit and system tests for the new functionality. * 4473 - Makes same warning appear for reminder date Reintroduces the functionality of having warnings for having the reminder date the same as the deadline date. * Only create new schedule if details have changed We don't want to create a new schedule if nothing has changed, as it will reset the start date and potentially mess with those who have every 2/3/etc months reminders. * 4473 - Removes Every N Months All reminders should be Monthly, so every N Months is removed. Also allows users to select "Last" as an option. * 4473 - Updates names of Month vs Week fields Rather than "date" and "week_day", we are now using "day_of_month" and "day_of_week" for clarity. * 4473 - fixes reminder day vs deadline display bug There was a bug in which if the reminder day of the month was the same as the deadline day, it would dislplay the error message even if the user changed to the day of the week option. Now the error message is hidden if the user is selecting the day of the week option. * Changes made by linter * Removed references to factored out ReminderSchedule model * Removed reference to factored out reminder_day * Removed redundant/outdated additions to the organizations detail page * Fixed accidentally removing receive_email_on_requests from organization_params during merge * Updated organization system specs to reflect current organization details page and verify new reminder schedule interface * Added every_nth_month field to let users specify a monthly frequency for reminders * Cleaned up _deadline_day_fields.html.erb and reworked it to hook up to new stimulus controller, replaced conditional SASS rule with JS implementation for sake of readability/maintainability * Updated deadline_day_controller to calculate the reminder and deadline dates using new rrule library * Reworked deadlinable specs to use explicit, hardcoded values * Updated check for same reminder and deadline days to only consider dayOfMonth and deadlineDay fields, as exact remidner date will shift when using byDayOfWeek * Renamed validations to clairfy they validate the day_of_month field and not the reminder date more generally * Updated should_update_reminder_schedule and create_schedule to be public since they don't have side effects, tweaked should_update_reminder_schedule * Added specs for remaining deadlinable functions * Fixed specs using wrong type of params for creating schedules * Added spec to validate that if a partner has reminders disabled but is part of a group with reminders, it still receives a reminder * Added spec to verify that the partner group deadline day is prioritized over the organization deadline day * Removed redundant checks on deadlinable fields from other models' specs * Fixed labels not being correctly hooked up to radio buttons * Updated description of deadlines to specify they always happen after the reminder * Updated deadline_day_controller to let user know when their input violates the between 1 and 28 constraint on dayOfMonth and deadlineDay * Added specs for shared deadline day form in the three pages where that form is used * Added spec to verify that the reminder and deadlines dates are consistent across the front and back end * Fixed spec refering to old radio button id * Changes made by linter * Updated and modified documentation to explain how reminder schedules work * Fixed PartnerGroup not checking if reminder_schedule needs to be updates, added comments explaining need for the check * Forgot to include Gemfile.lock * Changes made by linter * Updated spec to not use Organization.short_name * Forgot to run linter * Fixed migration not correctly converting reminder_day to reminder_schedule * First pass at adding start_date to deadline day form * Updated tests to consider start date * Forgot to add tests for get_values_from_reminder_schedule * Changes made by linter * Reworked create_schedule to start the schedule at the current date time if a start_date isn't provided * Reworked wording of deadline day form, updated javascript controller to calculate next reminder day after the current date * Removed confusing commented out configurations * Expanded tests to cover all combinations of the order of the start, current, and reminder dates * Fixed tests not setting deadline_day which made partner_groups fail validation * Removed unecessary should_update_reminder_schedule and from_ical functions * Extended untilDate calculation to account for week day rules that would be more than exactly monthylInterval months out * Updated factories to not set a deadline_day as it was causing issues with deadline_day_fields_shared_example tests * Changes made by linter * Renamed reminder schedule field, removed migration that removed reminder_day field, and added first pass at ReminderScheduleService [skip ci] * Removed validations from ReminderScheduleService that rely on the parent object * Minor change to make conditional cleaner * Factored out ReminderScheduleable concern, updated _deadline_day_fields form to use a ReminderScheduleService object instead of an ActiveRecord object [skip ci] * Slightly reworked how attributes are assigned to the reminder_schedule * Updated partner groups and admin organizations controller to use the new ReminderScheduleService object * Updated deadline day fields tests to only fill out the form with dates between 1 and 28 * Exposed IceCube occurs_on? function through ReminderScheduleService and updated FetchPartnersToRemindNowService to use it * Fixed day_of_week and every_nth_day validations not casting before comparing * Reverted change to _details because it didn't show 'Not defined' for an empty schedule object * Updated shared deadline day field tests to reflect the new form * Forgot to update partner group form to use new deadline_day_fields partial * Moved test comparing output of FetchPartnersToRemindNowService to what is shown in the form out of the shared examples, as orgs created via the admin interface won't have partners * Removed outdated test on removed edit_admin_organization_path, moved tests on deadline day form to happen on new_admin_organization_path * Made reminder_schedule_service params permitted but optional * Removed reminder schedule definitions from org and partner group factories as they aren't necessary to initializing those models * Updated tests to use new reminder schedule service * Updated reminder_schedule_is_empty_or_valid? to not raise errors if the user hasn't filled out the form at all, added a check to partner groups to make sure a valid reminder schedule is present when send_reminders is true * Teaked custom validation functions to explicitly check for presence of field * Added tests for the ReminderScheduleService * Removed old concerns and associated tests * Changes made by linter * Addressed linter's warning about using mixed logical operators in an unless condition * Reworked DeadlineService and exposed IceCube schedule next_occurrence to show user next reminder and deadline date in views * Added tests to verify reminder and deadilne dates added to organization and partner group views * Updated the reminder schedule form to instruct users how to set the start date for non-monthly schedules and warn them about same-day schedules likely not firing * Changes made by linter * Removed start_date and every_nth_month fields from ReminderScheduleService, now assuming all schedules are monthyl and start today; removed associated tests * Changed field label to be consistent with reminder day of week label * Forgot to remove unnecessary test now that start_date isn't tracked * Changed wording of deadline day field * Fixed deadline day controller incorrectly predicting deadline day if reminder would be sent on the 31st of a month * Updated user guide to reflect reduced scope of reminder schedule form * Fixed deadline_day being checked against day_of_month even for by_day_of_week schedules, added tests to verify * Forgot to add comment * Reworked reminder_schedule_is_empty_or_valid? conditionals to be more readable * Combined multiple asserts into larger it blocks for the sake of performance * Moved tests that didn't rely on javascript to requests * Moved definition of safe_add/subtract_days so they can be used in other tests, used them to fix test failing intermitently on the 14th --------- Co-authored-by: Jesse Landis-Eigsti <[email protected]>
1 parent 7f5b420 commit a95a278

File tree

50 files changed

+1657
-288
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+1657
-288
lines changed

Gemfile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ gem "flipper-ui"
9494
gem "geocoder"
9595
# Generate .ics calendars for use with Google Calendar
9696
gem 'icalendar', require: false
97+
# Offers functionality for date reocccurances
98+
gem "ice_cube"
9799
# JSON Web Token encoding / decoding (e.g. for links in e-mails)
98100
gem "jwt"
99101
# Use Newrelic for logs and APM

Gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ GEM
275275
concurrent-ruby (~> 1.1)
276276
webrick (~> 1.7)
277277
websocket-driver (~> 0.7)
278-
ffi (1.17.2)
279278
ffi (1.17.2-arm64-darwin)
280279
ffi (1.17.2-x86_64-darwin)
281280
ffi (1.17.2-x86_64-linux-gnu)
@@ -784,6 +783,7 @@ DEPENDENCIES
784783
geocoder
785784
guard-rspec
786785
icalendar
786+
ice_cube
787787
importmap-rails (~> 2.2)
788788
jbuilder
789789
jwt

app/controllers/admin/organizations_controller.rb

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ def new
3030
end
3131

3232
def create
33-
@organization = Organization.new(organization_params)
3433
@user = User.new(user_params)
35-
34+
@organization = Organization.new(organization_params)
35+
@organization.reminder_schedule.assign_attributes(reminder_schedule_params)
3636
if @organization.save
3737
Organization.seed_items(@organization)
3838
UserInviteService.invite(name: user_params[:name],
@@ -71,10 +71,15 @@ def destroy
7171

7272
def organization_params
7373
params.require(:organization)
74-
.permit(:name, :street, :city, :state, :zipcode, :email, :url, :logo, :intake_location, :default_email_text, :account_request_id, :reminder_day, :deadline_day, :bank_is_set_up,
74+
.permit(:name, :street, :city, :state, :zipcode, :email, :url, :logo, :intake_location, :default_email_text, :account_request_id, :bank_is_set_up,
75+
:reminder_schedule_definition, :deadline_day,
7576
users_attributes: %i(name email organization_admin), account_request_attributes: %i(ndbn_member_id id))
7677
end
7778

79+
def reminder_schedule_params
80+
params.require(:organization).fetch(:reminder_schedule_service, {}).permit([*ReminderScheduleService::REMINDER_SCHEDULE_FIELDS])
81+
end
82+
7883
def user_params
7984
params.require(:organization).require(:user).permit(:name, :email)
8085
end

app/controllers/organizations_controller.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ def edit
1717

1818
def update
1919
@organization = current_organization
20-
20+
@organization.reminder_schedule.assign_attributes(reminder_schedule_params)
2121
if OrganizationUpdateService.update(@organization, organization_params)
22-
redirect_back(fallback_location: organization_path, notice: "Updated your organization!")
22+
redirect_to organization_path, notice: "Updated your organization!"
2323
else
2424
flash.now[:error] = @organization.errors.full_messages.join("\n")
2525
render :edit
@@ -95,7 +95,7 @@ def organization_params
9595
:name, :street, :city, :state,
9696
:zipcode, :email, :url, :logo, :intake_location,
9797
:default_storage_location, :default_email_text, :reminder_email_text,
98-
:invitation_text, :reminder_day, :deadline_day,
98+
:invitation_text, :reminder_schedule_definition, :deadline_day,
9999
:repackage_essentials, :distribute_monthly,
100100
:ndbn_member_id, :enable_child_based_requests,
101101
:enable_individual_requests, :enable_quantity_based_requests,
@@ -109,6 +109,10 @@ def organization_params
109109
)
110110
end
111111

112+
def reminder_schedule_params
113+
params.require(:organization).fetch(:reminder_schedule_service, {}).permit([*ReminderScheduleService::REMINDER_SCHEDULE_FIELDS])
114+
end
115+
112116
def request_type_formatter(params)
113117
if params[:organization][:enable_individual_requests] == "false"
114118
params[:organization][:enable_child_based_requests] = false

app/controllers/partner_groups_controller.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ class PartnerGroupsController < ApplicationController
44
def new
55
@partner_group = current_organization.partner_groups.new
66
set_items_categories
7+
@item_categories = current_organization.item_categories
78
end
89

910
def create
1011
@partner_group = current_organization.partner_groups.new(partner_group_params)
12+
@partner_group.reminder_schedule.assign_attributes(reminder_schedule_params)
1113
if @partner_group.save
1214
# Redirect to groups tab in Partner page.
1315
redirect_to partners_path + "#nav-partner-groups", notice: "Partner group added!"
@@ -21,10 +23,12 @@ def create
2123
def edit
2224
@partner_group = current_organization.partner_groups.find(params[:id])
2325
set_items_categories
26+
@item_categories = current_organization.item_categories
2427
end
2528

2629
def update
2730
@partner_group = current_organization.partner_groups.find(params[:id])
31+
@partner_group.reminder_schedule.assign_attributes(reminder_schedule_params)
2832
if @partner_group.update(partner_group_params)
2933
redirect_to partners_path + "#nav-partner-groups", notice: "Partner group edited!"
3034
else
@@ -52,7 +56,11 @@ def set_partner_group
5256
end
5357

5458
def partner_group_params
55-
params.require(:partner_group).permit(:name, :send_reminders, :deadline_day, :reminder_day, item_category_ids: [])
59+
params.require(:partner_group).permit(:name, :send_reminders, :reminder_schedule_definition, :deadline_day, item_category_ids: [])
60+
end
61+
62+
def reminder_schedule_params
63+
params.require(:partner_group).fetch(:reminder_schedule_service, {}).permit([*ReminderScheduleService::REMINDER_SCHEDULE_FIELDS])
5664
end
5765

5866
def set_items_categories

app/javascript/application.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import 'controllers'
2828

2929
import 'utils/barcode_items'
3030
import 'utils/barcode_scan'
31-
import 'utils/deadline_day_pickers'
3231
import 'utils/distributions_and_transfers'
3332
import 'utils/donations'
3433
import 'utils/purchases'
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { Controller } from "@hotwired/stimulus"
2+
import $ from 'jquery';
3+
import { RRule } from 'rrule'
4+
import 'tslib'
5+
6+
const WEEKDAY_NUM_TO_OBJ = {
7+
0: RRule.SU,
8+
1: RRule.MO,
9+
2: RRule.TU,
10+
3: RRule.WE,
11+
4: RRule.TH,
12+
5: RRule.FR,
13+
6: RRule.SA
14+
}
15+
16+
export default class extends Controller {
17+
static targets = [
18+
'byDayOfMonth', 'byDayOfWeek', 'dayOfMonthFields', 'dayOfMonth',
19+
'dayOfWeekFields', 'everyNthDay', 'dayOfWeek', 'deadlineDay', 'reminderText', 'deadlineText'
20+
]
21+
22+
static dateParser = /(\d{4})-(\d{2})-(\d{2})/;
23+
24+
getFirstOccurrenceAfterToday( occurrences, today ) {
25+
let index = occurrences.length - 1
26+
let firstOccurrence = null
27+
while (index >= 0){
28+
if (occurrences[index].getTime() > today.getTime()) {
29+
firstOccurrence = occurrences[index]
30+
index--
31+
} else {
32+
break
33+
}
34+
}
35+
return firstOccurrence
36+
}
37+
38+
sourceChange() {
39+
let reminder_date = null;
40+
let deadline_date = null;
41+
42+
// For now, we are assuming that all schedules are monthly and start on the current date
43+
let monthlyInterval = 1;
44+
let today = new Date();
45+
let untilDate = new Date( today );
46+
untilDate.setMonth( untilDate.getMonth() + monthlyInterval + 1 )
47+
48+
if (this.byDayOfMonthTarget.checked && this.dayOfMonthTarget.value) {
49+
const rule = new RRule({
50+
dtstart: today,
51+
freq: RRule.MONTHLY,
52+
interval: monthlyInterval,
53+
bymonthday: parseInt(this.dayOfMonthTarget.value),
54+
until: untilDate
55+
})
56+
reminder_date = this.getFirstOccurrenceAfterToday( rule.all(), today )
57+
}
58+
if (this.byDayOfWeekTarget.checked && this.everyNthDayTarget.value && (this.dayOfWeekTarget.value)) {
59+
const rule = new RRule({
60+
dtstart: today,
61+
freq: RRule.MONTHLY,
62+
interval: monthlyInterval,
63+
byweekday: WEEKDAY_NUM_TO_OBJ[ parseInt(this.dayOfWeekTarget.value) ].nth( parseInt(this.everyNthDayTarget.value) ),
64+
wkst: RRule.SU,
65+
until: untilDate
66+
})
67+
reminder_date = this.getFirstOccurrenceAfterToday( rule.all(), today )
68+
}
69+
if (reminder_date && this.deadlineDayTarget.value) {
70+
deadline_date = new Date(reminder_date.getTime());
71+
deadline_date.setDate(parseInt(this.deadlineDayTarget.value))
72+
if( reminder_date.getDate() >= parseInt(this.deadlineDayTarget.value)){
73+
deadline_date.setMonth( deadline_date.getMonth() + 1 )
74+
}
75+
}
76+
77+
if (this.byDayOfMonthTarget.checked && this.dayOfMonthTarget.value
78+
&& this.deadlineDayTarget.value && this.dayOfMonthTarget.value === this.deadlineDayTarget.value) {
79+
$(this.reminderTextTarget).removeClass('text-muted').addClass('text-danger');
80+
$(this.reminderTextTarget).text('Reminder day cannot be the same as deadline day.');
81+
$(this.deadlineTextTarget).text("");
82+
} else {
83+
let dayOfMonth = parseInt(this.dayOfMonthTarget.value);
84+
let deadlineDay = parseInt(this.deadlineDayTarget.value);
85+
if (dayOfMonth < 1 || dayOfMonth > 28){
86+
$(this.reminderTextTarget).removeClass('text-muted').addClass('text-danger');
87+
$(this.reminderTextTarget).text("Reminder day must be between 1 and 28");
88+
} else {
89+
$(this.reminderTextTarget).removeClass('text-danger').addClass('text-muted');
90+
$(this.reminderTextTarget).text(reminder_date ? `Your next reminder date is ${reminder_date.toDateString()}.` : "");
91+
}
92+
if (deadlineDay < 1 || deadlineDay > 28){
93+
$(this.deadlineTextTarget).removeClass('text-muted').addClass('text-danger');
94+
$(this.deadlineTextTarget).text("Deadline day must be between 1 and 28");
95+
} else {
96+
$(this.deadlineTextTarget).removeClass('text-danger').addClass('text-muted');
97+
$(this.deadlineTextTarget).text(deadline_date ? `The deadline on your next reminder email will be ${deadline_date.toDateString()}.` : "");
98+
}
99+
}
100+
}
101+
102+
monthOrWeekChanged() {
103+
$(this.dayOfMonthFieldsTarget).toggleClass("d-none", !this.byDayOfMonthTarget.checked );
104+
$(this.dayOfWeekFieldsTarget).toggleClass("d-none", !this.byDayOfWeekTarget.checked );
105+
}
106+
107+
connect() {
108+
this.monthOrWeekChanged()
109+
this.sourceChange()
110+
}
111+
}

app/javascript/utils/deadline_day_pickers.js

Lines changed: 0 additions & 74 deletions
This file was deleted.

app/mailers/reminder_deadline_mailer.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def notify_deadline(partner)
1515
private
1616

1717
def deadline_date(partner)
18-
date = DeadlineService.new(partner: partner).next_deadline
18+
date = DeadlineService.new(deadline_day: DeadlineService.get_deadline_for_partner(partner)).next_deadline
1919

2020
return date if date
2121

app/models/concerns/deadlinable.rb

Lines changed: 0 additions & 15 deletions
This file was deleted.

0 commit comments

Comments
 (0)