Incorrect progressbar alert 10899 (#19649)

* Comparison for goals depends on alert_above_goal and if progressbar.

This commit sets up the timeseries predicate and writes tests for what I think the correct behaviours should be:

timeseries, alert above -> (<= goal-value value)
 - (<= 5.9 6) true  -> alert sent
 - (<= 6 6)   true  -> alert sent
 - (<= 6.1 6) false -> alert not sent

progressbar, alert above -> (<= goal-value value)
 - (<= 5.9 6) true  -> alert sent
 - (<= 6 6)   true  -> alert sent
 - (<= 6.1 6) false -> alert not sent

timeseries, alert below -> (>= goal-value value)
 - (>= 6.1 6) true   -> alert sent
 - (>= 6 6)   true   -> alert sent. NOTE: this is to match prior behaviour, but I don't understand why it is this way?
 - (>= 5.9 6) faluse -> alert not sent

progressbar, alert below -> (> goal-value value)
 - (> 6.1 6) true   -> alert sent
 - (> 6 6)   false  -> alert not sent. NOTE: this is what should fix the bug (#10899)
 - (> 5.9 6) faluse -> alert not sent

I think there may be a cleaner way to write and present the tests, so the next commit(s) will address that.

* Simplified Tests for goal-met? predicate.

The goal-met predicate is tested directly with mock data to make the tests more readable.

The change to `metabase.pulse/goal-met?` fixes #10899 while preserving the goal-met behaviour for any non-progress
goal. The behaviour should now be as follows:

```
| Timeseries? | alert_above? | goal | val | goal-met? |
+-------------+--------------+------+-----+-----------+
|     f       |      t       |  5   |  4  |     f     |
|     f       |      t       |  5   |  5  |     t     |
|     f       |      t       |  5   |  6  |     t     |
|     f       |      f       |  5   |  4  |     t     |
|     f       |      f       |  5   |  5  |     f     | <--- this is new behaviour
|     f       |      f       |  5   |  6  |     f     |

|     t       |      t       |  5   |  4  |     f     |
|     t       |      t       |  5   |  5  |     t     |
|     t       |      t       |  5   |  6  |     t     |
|     t       |      f       |  5   |  4  |     t     |
|     t       |      f       |  5   |  5  |     t     |
|     t       |      f       |  5   |  6  |     f     |
```

Otherwise, alerts should be triggered as before.

* added issue number to test

* fixed alignment issue

* swapped goal and value around in comparison for readability

* Fixing nits

* Timeseries and progress bar now follow same logic

I asked for clarification about when alerts fire in #ama-product in Metabase's Slack. There was consensus around the
idea that 'reaches the goal' does generally imply 'meets the goal', so we can use the same comparators between
progress bar and timeseries goals now.

This simplifies the goal-met? function again because it's no longer necessary to check if the given data is a
timeseries goal.

* Adjusted text in goal line alert modal

* ran prettier

frontend/src/metabase/query_builder/components/AlertModals.jsx

@@ -504,9 +504,7 @@ export const AlertGoalToggles = ({ alertType, alert, onAlertChange }) => {
             ? t`Alert me when the line…`
             : t`Alert me when the progress bar…`
         }
-        trueText={
-          isTimeseries ? t`Goes above the goal line` : t`Reaches the goal`
-        }
+        trueText={isTimeseries ? t`Reaches the goal line` : t`Reaches the goal`}
         falseText={
           isTimeseries ? t`Goes below the goal line` : t`Goes below the goal`
         }

frontend/test/metabase/scenarios/sharing/alert/alert-types.cy.spec.js

@@ -81,7 +81,7 @@ describe("scenarios > alert > types", () => {
 
       openAlertModal();
 
-      cy.findByText("Goes above the goal line").click();
+      cy.findByText("Reaches the goal line").click();
       cy.findByText("The first time").click();
 
       cy.button("Done").click();

src/metabase/pulse.clj

@@ -208,7 +208,7 @@
   (every? is-card-empty? results))
 
 (defn- goal-met? [{:keys [alert_above_goal], :as pulse} [first-result]]
-  (let [goal-comparison      (if alert_above_goal <= >=)
+  (let [goal-comparison      (if alert_above_goal >= <)
         goal-val             (ui/find-goal-value first-result)
         comparison-col-rowfn (ui/make-goal-comparison-rowfn (:card first-result)
                                                             (get-in first-result [:result :data]))]
@@ -217,9 +217,10 @@
       (throw (ex-info (tru "Unable to compare results to goal for alert.")
                       {:pulse  pulse
                        :result first-result})))
-    (some (fn [row]
-            (goal-comparison goal-val (comparison-col-rowfn row)))
-          (get-in first-result [:result :data :rows]))))
+    (boolean
+     (some (fn [row]
+             (goal-comparison (comparison-col-rowfn row) goal-val))
+           (get-in first-result [:result :data :rows])))))
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+

test/metabase/pulse_test.clj

@@ -608,6 +608,41 @@
                                     [test-card-result png-attachment png-attachment])
                  (mt/summarize-multipart-email test-card-regex))))}})))
 
+(deftest goal-met-test
+  (let [alert-above-pulse {:alert_above_goal true}
+        alert-below-pulse {:alert_above_goal false}
+        progress-result (fn [val] [{:card {:display :progress
+                                            :visualization_settings {:progress.goal 5}}
+                                     :result {:data {:rows [[val]]}}}])
+        timeseries-result (fn [val] [{:card {:display :bar
+                                             :visualization_settings {:graph.goal_value 5}}
+                                      :result {:data {:cols [{:source :breakout}
+                                                             {:name "avg"
+                                                              :source :aggregation
+                                                              :base_type :type/Integer
+                                                              :effective-type :type/Integer
+                                                              :semantic_type :type/Quantity}]
+                                                      :rows [["2021-01-01T00:00:00Z" val]]}}}])
+        goal-met? (fn [pulse [first-result]] (#'metabase.pulse/goal-met? pulse [first-result]))]
+    (testing "Progress bar"
+      (testing "alert above"
+        (testing "value below goal"  (is (= false (goal-met? alert-above-pulse (progress-result 4)))))
+        (testing "value equals goal" (is (=  true (goal-met? alert-above-pulse (progress-result 5)))))
+        (testing "value above goal"  (is (=  true (goal-met? alert-above-pulse (progress-result 6))))))
+      (testing "alert below"
+        (testing "value below goal"  (is (=  true (goal-met? alert-below-pulse (progress-result 4)))))
+        (testing "value equals goal (#10899)" (is (= false (goal-met? alert-below-pulse (progress-result 5)))))
+        (testing "value above goal"  (is (= false (goal-met? alert-below-pulse (progress-result 6)))))))
+    (testing "Timeseries"
+      (testing "alert above"
+        (testing "value below goal"  (is (= false (goal-met? alert-above-pulse (timeseries-result 4)))))
+        (testing "value equals goal" (is (=  true (goal-met? alert-above-pulse (timeseries-result 5)))))
+        (testing "value above goal"  (is (=  true (goal-met? alert-above-pulse (timeseries-result 6))))))
+      (testing "alert below"
+        (testing "value below goal"  (is (=  true (goal-met? alert-below-pulse (timeseries-result 4)))))
+        (testing "value equals goal" (is (= false (goal-met? alert-below-pulse (timeseries-result 5)))))
+        (testing "value above goal"  (is (= false (goal-met? alert-below-pulse (timeseries-result 6)))))))))
+
 (deftest native-query-with-user-specified-axes-test
   (testing "Native query with user-specified x and y axis"
     (mt/with-temp Card [{card-id :id} {:name                   "Test card"