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.

app/controllers/admin/organizations_controller.rb

@@ -2,7 +2,7 @@
 class Admin::OrganizationsController < AdminController
   def edit
     @organization = Organization.find(params[:id])
-    @organization.from_ical(@organization.reminder_schedule)
+    @organization.get_values_from_reminder_schedule
   end
 
   def update
@@ -31,7 +31,7 @@ def index
 
   def new
     @organization = Organization.new
-    @organization.from_ical(@organization.reminder_schedule)
+    @organization.get_values_from_reminder_schedule
     account_request = params[:token] && AccountRequest.get_by_identity_token(params[:token])
 
     @user = User.new

app/controllers/organizations_controller.rb

@@ -10,7 +10,7 @@ def show
 
   def edit
     @organization = current_organization
-    @organization.from_ical(@organization.reminder_schedule)
+    @organization.get_values_from_reminder_schedule
   end
 
   def update
@@ -100,7 +100,7 @@ def organization_params
       :hide_value_columns_on_receipt, :hide_package_column_on_receipt,
       :signature_for_distribution_pdf, :every_n_months,
       :date_or_week_day, :date, :day_of_week, :every_nth_day,
-      partner_form_fields: []
+      partner_form_fields: [],
       request_unit_names: []
     )
   end

app/controllers/partner_groups_controller.rb

@@ -17,7 +17,7 @@ def create
 
   def edit
     @partner_group = current_organization.partner_groups.find(params[:id])
-    @partner_group.from_ical(@partner_group.reminder_schedule)
+    @partner_group.get_values_from_reminder_schedule
     @item_categories = current_organization.item_categories
   end
 

app/models/concerns/deadlinable.rb

@@ -3,7 +3,7 @@ module Deadlinable
   MIN_DAY_OF_MONTH = 1
   MAX_DAY_OF_MONTH = 28
   EVERY_NTH_COLLECTION = [["First", 1], ["Second", 2], ["Third", 3], ["Fourth", 4]].freeze
-  WEEK_DAY_COLLECTION = [["Sunday"], ["Monday", 1], ["Tuesday", 2], ["Wednesday", 3], ["Thursday", 4], ["Friday", 5], ["Saturday", 6]].freeze
+  WEEK_DAY_COLLECTION = [["Sunday", 0], ["Monday", 1], ["Tuesday", 2], ["Wednesday", 3], ["Thursday", 4], ["Friday", 5], ["Saturday", 6]].freeze
 
   included do
     attr_accessor :every_n_months, :date_or_week_day, :date, :day_of_week, :every_nth_day
@@ -14,7 +14,7 @@ module Deadlinable
     validate :reminder_is_within_range?, if: -> { every_n_months.present? }
     validates :date_or_week_day, inclusion: {in: %w[date week_day]}, if: -> { every_n_months.present? }
     validates :date, presence: true, if: -> { date_or_week_day == "date" && every_n_months.present? }
-    validates :day_of_week, presence: true, if: -> { date_or_week_day == "week_day" && every_n_months.present? }, inclusion: {in: %w[1 2 3 4 5 6 7]}
+    validates :day_of_week, presence: true, if: -> { date_or_week_day == "week_day" && every_n_months.present? }, inclusion: {in: %w[0 1 2 3 4 5 6]}
     validates :every_nth_day, presence: true, if: -> { date_or_week_day == "week_day" && every_n_months.present? }, inclusion: {in: %w[1 2 3 4]}
   end
 
@@ -34,15 +34,29 @@ def from_ical(ical)
     schedule = IceCube::Schedule.from_ical(ical)
     rule = schedule.recurrence_rules.first.instance_values
     date = rule["validations"][:day_of_month]&.first&.value
-    self.every_n_months = rule["interval"]
-    self.date_or_week_day = date ? "date" : "week_day"
-    self.date = date
-    self.day_of_week = rule["validations"][:day_of_week]&.first&.day,
-      self.every_nth_day = rule["validations"][:day_of_week]&.first&.occ
+
+    results = {}
+    results[:every_n_months] = rule["interval"]
+    results[:date_or_week_day] = date ? "date" : "week_day"
+    results[:date] = date
+    results[:day_of_week] = rule["validations"][:day_of_week]&.first&.day
+    results[:every_nth_day] = rule["validations"][:day_of_week]&.first&.occ
+    results
   rescue
     nil
   end
 
+  def get_values_from_reminder_schedule
+    return if reminder_schedule.blank?
+    results = from_ical(reminder_schedule)
+    return if results.nil?
+    self.every_n_months = results[:every_n_months]
+    self.date_or_week_day = results[:date_or_week_day]
+    self.date = results[:date]
+    self.day_of_week = results[:day_of_week]
+    self.every_nth_day = results[:every_nth_day]
+  end
+
   private
 
   def reminder_on_deadline_day?
@@ -59,6 +73,18 @@ def reminder_is_within_range?
     end
   end
 
+  def should_update_reminder_schedule
+    if reminder_schedule.blank?
+      return every_n_months.present?
+    end
+    sched = from_ical(reminder_schedule)
+    every_n_months != sched[:every_n_months].presence.to_s ||
+      date_or_week_day != sched[:date_or_week_day].presence.to_s ||
+      date != sched[:date].presence.to_s ||
+      day_of_week != sched[:day_of_week].presence.to_s ||
+      every_nth_day != sched[:every_nth_day].presence.to_s
+  end
+
   def create_schedule
     schedule = IceCube::Schedule.new(Time.zone.now.to_date)
     return nil if every_n_months.blank? || every_n_months.to_i.zero?

app/models/organization.rb

@@ -111,7 +111,9 @@ def upcoming
   end
 
   before_save do
-    self.reminder_schedule = create_schedule
+    if should_update_reminder_schedule
+      self.reminder_schedule = create_schedule
+    end
   end
 
   after_create do

app/views/shared/_deadline_day_fields.html.erb

@@ -39,7 +39,6 @@
       label: 'Reminder day of the week' do %>
       <%= f.input :every_nth_day, collection: Deadlinable::EVERY_NTH_COLLECTION,
       class: "deadline-day-pickers__reminder-day form-control",
-      default: 1,
       label: false %>
 
       <%= f.input :day_of_week, collection: Deadlinable::WEEK_DAY_COLLECTION,

spec/services/partners/fetch_partners_to_remind_now_service_spec.rb

@@ -22,7 +22,7 @@
           context "as matched by day of the week" do
             before do
               partner.organization.update(every_n_months: 1, date_or_week_day: "week_day",
-              day_of_week: 2, every_nth_day: 2, deadline_day: current_day + 2)
+                day_of_week: 2, every_nth_day: 2, deadline_day: current_day + 2)
             end
             it "should include that partner" do
               schedule = IceCube::Schedule.from_ical partner.organization.reminder_schedule
@@ -55,7 +55,7 @@
         context "that is not for today" do
           before do
             partner.organization.update(every_n_months: 1, date_or_week_day: "date",
-            date: current_day - 1, deadline_day: current_day + 2)
+              date: current_day - 1, deadline_day: current_day + 2)
           end
 
           it "should NOT include that partner" do
@@ -66,11 +66,11 @@
         context "AND a partner group that does have them defined" do
           before do
             partner_group = create(:partner_group, every_n_months: 1, date_or_week_day: "date",
-            date: current_day, deadline_day: current_day + 2)
+              date: current_day, deadline_day: current_day + 2)
             partner_group.partners << partner
 
             partner.organization.update(every_n_months: 1, date_or_week_day: "date",
-            date: current_day - 1, deadline_day: current_day + 2)
+              date: current_day - 1, deadline_day: current_day + 2)
           end
 
           it "should remind based on the partner group instead of the organization level reminder" do
@@ -98,7 +98,7 @@
           context "that is for today" do
             before do
               partner_group = create(:partner_group, every_n_months: 1, date_or_week_day: "date",
-              date: current_day, deadline_day: current_day + 2)
+                date: current_day, deadline_day: current_day + 2)
               partner_group.partners << partner
             end
 
@@ -120,7 +120,7 @@
           context "that is not for today" do
             before do
               partner_group = create(:partner_group, every_n_months: 1, date_or_week_day: "date",
-              date: current_day - 1, deadline_day: current_day + 2)
+                date: current_day - 1, deadline_day: current_day + 2)
               partner_group.partners << partner
             end