update `retrieve-scheduled-channels` to handle including monthly pulses.

src/metabase/models/pulse_channel.clj

@@ -155,17 +155,32 @@
      * no input returns any channel scheduled for HOURLY delivery only.
      * just `hour` input returns any HOURLY scheduled channels + DAILY channels for the chosen hour.
      * when `hour` and `day` are supplied we return HOURLY channels + DAILY channels + WEEKLY channels."
-  [hour day]
+  [hour weekday monthday monthweek]
   [:pre [(integer? hour)
-         (day-of-week? day)]]
-  (k/select PulseChannel
-    (k/fields :id :pulse_id :schedule_type :channel_type)
-    (k/where (or (= :schedule_type (name schedule-type-hourly))
-                 (and (= :schedule_type (name schedule-type-daily))
-                      (= :schedule_hour hour))
-                 (and (= :schedule_type (name schedule-type-weekly))
-                      (= :schedule_hour hour)
-                      (= :schedule_day day))))))
+         (day-of-week? weekday)
+         (contains? #{:first :last :mid :other} monthday)
+         (contains? #{:first :last :other} monthweek)]]
+  (let [schedule-frame              (cond
+                                      (= :mid monthday)    (name schedule-frame-mid)
+                                      (= :first monthweek) (name schedule-frame-first)
+                                      (= :last monthweek)  (name schedule-frame-last)
+                                      :else                "invalid")
+        monthly-schedule-day-or-nil (when (= :other monthday)
+                                      weekday)]
+    (k/select PulseChannel
+      (k/fields :id :pulse_id :schedule_type :channel_type)
+      (k/where (or (= :schedule_type (name schedule-type-hourly))
+                   (and (= :schedule_type (name schedule-type-daily))
+                        (= :schedule_hour hour))
+                   (and (= :schedule_type (name schedule-type-weekly))
+                        (= :schedule_hour hour)
+                        (= :schedule_day weekday))
+                   (and (= :schedule_type (name schedule-type-monthly))
+                        (= :schedule_hour hour)
+                        (= :schedule_frame schedule-frame)
+                        (or (= :schedule_day weekday)
+                            ;; this is here specifically to allow for cases where day doesn't have to match
+                            (= :schedule_day monthly-schedule-day-or-nil))))))))
 
 (defn update-recipients!
   "Update the `PulseChannelRecipients` for PULSE-CHANNEL.

src/metabase/task/send_pulses.clj

@@ -41,7 +41,7 @@
         curr-weekday       (->> (time/day-of-week now)
                                 (get pulse-channel/days-of-week)
                                 :id)]
-    (send-pulses curr-hour curr-weekday)))
+    (send-pulses curr-hour curr-weekday :other :other)))
 
 (defn- task-init []
   ;; build our job
@@ -127,11 +127,13 @@
   "Send any `Pulses` which are scheduled to run in the current day/hour.  We use the current time and determine the
    hour of the day and day of the week according to the defined reporting timezone, or UTC.  We then find all `Pulses`
    that are scheduled to run and send them."
-  [hour day]
+  [hour weekday monthday monthweek]
   [:pre [(integer? hour)
          (and (< 0 hour) (> 23 hour))
-         (pulse-channel/day-of-week? day)]]
-  (let [channels-by-pulse (group-by :pulse_id (pulse-channel/retrieve-scheduled-channels hour day))]
+         (pulse-channel/day-of-week? weekday)
+         (contains? #{:first :last :mid :other} monthday)
+         (contains? #{:first :last :other} monthweek)]]
+  (let [channels-by-pulse (group-by :pulse_id (pulse-channel/retrieve-scheduled-channels hour weekday monthday monthweek))]
     (doseq [pulse-id (keys channels-by-pulse)]
       (try
         (log/debug (format "Starting Pulse Execution: %d" pulse-id))

test/metabase/models/pulse_channel_test.clj

@@ -313,7 +313,7 @@
                                      :schedule_type schedule-type-hourly
                                      :schedule_hour nil}]
         (let [retrieve-channels (fn [hour day]
-                                  (->> (retrieve-scheduled-channels hour day)
+                                  (->> (retrieve-scheduled-channels hour day :other :other)
                                        (mapv #(dissoc % :id :pulse_id))))]
           [(retrieve-channels nil nil)
            (retrieve-channels 12 nil)
@@ -356,9 +356,66 @@
                                            :schedule_hour 8
                                            :schedule_day  "mon"}]
               (let [retrieve-channels (fn [hour day]
-                                        (->> (retrieve-scheduled-channels hour day)
+                                        (->> (retrieve-scheduled-channels hour day :other :other)
                                              (mapv #(dissoc % :id :pulse_id))))]
                 [(retrieve-channels nil nil)
                  (retrieve-channels 10 nil)
                  (retrieve-channels 15 nil)
                  (retrieve-channels 8 "mon")]))))))))
+
+;; specific test for various monthly scheduling permutations
+(expect
+  [[]
+   [{:schedule_type "monthly", :channel_type "email"}
+    {:schedule_type "monthly", :channel_type "slack"}]
+   [{:schedule_type "monthly", :channel_type "slack"}]
+   []
+   [{:schedule_type "monthly", :channel_type "slack"}]
+   [{:schedule_type "monthly", :channel_type "email"}]]
+  (tu/with-temp Pulse [{pulse1 :id} {:creator_id (user->id :rasta)
+                                     :name       (tu/random-name)}]
+    (tu/with-temp Pulse [{pulse2 :id} {:creator_id (user->id :crowberto)
+                                       :name       (tu/random-name)}]
+      (tu/with-temp PulseChannel [_ {:pulse_id      pulse1
+                                     :channel_type  :email
+                                     :details       {}
+                                     :schedule_type schedule-type-monthly
+                                     :schedule_hour 12
+                                     :schedule_day  nil
+                                     :schedule_frame schedule-frame-first}]
+        (tu/with-temp PulseChannel [_ {:pulse_id      pulse1
+                                       :channel_type  :slack
+                                       :details       {}
+                                       :schedule_type schedule-type-monthly
+                                       :schedule_hour 12
+                                       :schedule_day  "mon"
+                                       :schedule_frame schedule-frame-first}]
+          (tu/with-temp PulseChannel [_ {:pulse_id      pulse2
+                                         :channel_type  :slack
+                                         :details       {}
+                                         :schedule_type schedule-type-monthly
+                                         :schedule_hour 16
+                                         :schedule_day  nil
+                                         :schedule_frame schedule-frame-mid}]
+            (tu/with-temp PulseChannel [_ {:pulse_id      pulse2
+                                           :channel_type  :email
+                                           :details       {}
+                                           :schedule_type schedule-type-monthly
+                                           :schedule_hour 8
+                                           :schedule_day  "fri"
+                                           :schedule_frame schedule-frame-last}]
+              (let [retrieve-channels (fn [hour weekday monthday monthweek]
+                                        (->> (retrieve-scheduled-channels hour weekday monthday monthweek)
+                                             (mapv #(dissoc % :id :pulse_id))))]
+                ;; simple starter which should be empty
+                [(retrieve-channels nil nil :other :other)
+                 ;; this should capture BOTH first absolute day of month + first monday of month schedules
+                 (retrieve-channels 12 "mon" :first :first)
+                 ;; this should only capture the first monday of the month
+                 (retrieve-channels 12 "mon" :other :first)
+                 ;; this makes sure hour checking is being enforced
+                 (retrieve-channels 8 "mon" :first :first)
+                 ;; middle of the month
+                 (retrieve-channels 16 "fri" :mid :other)
+                 ;; last friday of the month (but not the last day of month)
+                 (retrieve-channels 8 "fri" :other :last)]))))))))