[tech debt] Stop using ms/BooleanString and only use ms/BooleanValue (#41390)

* [tech debt] Stop using ms/BooleanString and only use ms/BooleanValue

ms/BooleanString will coerce to a string, and we should probably get rid of it and only ever use ms/BooleanValue which will always coerce to a boolean
We want the coercion layer to give us values that are helpful, and true is more helpful than "true".
So we should remove ms/BooleanString and calls to parseBoolean

* Remove ms/BooleanString usage in api/native_query_snippet.clj

* Remove test case related to ms/BooleanString

* Remove the BooleanString schema def as no longer used

src/metabase/api/alert.clj

@@ -32,12 +32,12 @@
   "Fetch alerts which the current user has created or will receive, or all alerts if the user is an admin.
   The optional `user_id` will return alerts created by the corresponding user, but is ignored for non-admin users."
   [archived user_id]
-  {archived [:maybe ms/BooleanString]
+  {archived [:maybe ms/BooleanValue]
    user_id  [:maybe ms/PositiveInt]}
   (let [user-id (if api/*is-superuser?*
                   user_id
                   api/*current-user-id*)]
-    (as-> (pulse/retrieve-alerts {:archived? (Boolean/parseBoolean archived)
+    (as-> (pulse/retrieve-alerts {:archived? archived
                                   :user-id   user-id}) <>
       (filter mi/can-read? <>)
       (t2/hydrate <> :can_write))))
@@ -53,10 +53,10 @@
   "Fetch all alerts for the given question (`Card`) id"
   [id archived]
   {id       [:maybe ms/PositiveInt]
-   archived [:maybe ms/BooleanString]}
+   archived [:maybe ms/BooleanValue]}
   (-> (if api/*is-superuser?*
-        (pulse/retrieve-alerts-for-cards {:card-ids [id], :archived? (Boolean/parseBoolean archived)})
-        (pulse/retrieve-user-alerts-for-card {:card-id id, :user-id api/*current-user-id*, :archived? (Boolean/parseBoolean archived)}))
+        (pulse/retrieve-alerts-for-cards {:card-ids [id], :archived? archived})
+        (pulse/retrieve-user-alerts-for-card {:card-id id, :user-id api/*current-user-id*, :archived?  archived}))
       (t2/hydrate :can_write)))
 
 (defn- only-alert-keys [request]

src/metabase/api/collection.clj

@@ -856,14 +856,14 @@
   [id models archived pinned_state sort_column sort_direction]
   {id             ms/PositiveInt
    models         [:maybe Models]
-   archived       [:maybe ms/BooleanString]
+   archived       [:maybe ms/BooleanValue]
    pinned_state   [:maybe (into [:enum] valid-pinned-state-values)]
    sort_column    [:maybe (into [:enum] valid-sort-columns)]
    sort_direction [:maybe (into [:enum] valid-sort-directions)]}
   (let [model-kwds (set (map keyword (u/one-or-many models)))]
     (collection-children (api/read-check Collection id)
                          {:models       model-kwds
-                          :archived?    (Boolean/parseBoolean archived)
+                          :archived?    archived
                           :pinned-state (keyword pinned_state)
                           :sort-info    [(or (some-> sort_column normalize-sort-choice) :name)
                                          (or (some-> sort_direction normalize-sort-choice) :asc)]})))
@@ -910,7 +910,7 @@
   `snippets`, you can pass the `?namespace=` parameter."
   [models archived namespace pinned_state sort_column sort_direction]
   {models         [:maybe Models]
-   archived       [:maybe ms/BooleanString]
+   archived       [:maybe ms/BooleanValue]
    namespace      [:maybe ms/NonBlankString]
    pinned_state   [:maybe (into [:enum] valid-pinned-state-values)]
    sort_column    [:maybe (into [:enum] valid-sort-columns)]
@@ -923,7 +923,7 @@
     (collection-children
      root-collection
      {:models       model-kwds
-      :archived?    (Boolean/parseBoolean archived)
+      :archived?    archived
       :pinned-state (keyword pinned_state)
       :sort-info    [(or (some-> sort_column normalize-sort-choice) :name)
                      (or (some-> sort_direction normalize-sort-choice) :asc)]})))

src/metabase/api/database.clj

@@ -500,13 +500,13 @@
   and tables, with no additional metadata."
   [id include_hidden include_editable_data_model remove_inactive]
   {id                          ms/PositiveInt
-   include_hidden              [:maybe ms/BooleanString]
-   include_editable_data_model [:maybe ms/BooleanString]
-   remove_inactive             [:maybe ms/BooleanString]}
+   include_hidden              [:maybe ms/BooleanValue]
+   include_editable_data_model [:maybe ms/BooleanValue]
+   remove_inactive             [:maybe ms/BooleanValue]}
   (db-metadata id
-               (Boolean/parseBoolean include_hidden)
-               (Boolean/parseBoolean include_editable_data_model)
-               (Boolean/parseBoolean remove_inactive)))
+               include_hidden
+               include_editable_data_model
+               remove_inactive))
 
 
 ;;; --------------------------------- GET /api/database/:id/autocomplete_suggestions ---------------------------------

src/metabase/api/native_query_snippet.clj

@@ -24,9 +24,9 @@
 (api/defendpoint GET "/"
   "Fetch all snippets"
   [archived]
-  {archived [:maybe ms/BooleanString]}
+  {archived [:maybe ms/BooleanValue]}
   (let [snippets (t2/select NativeQuerySnippet
-                            :archived (Boolean/parseBoolean archived)
+                            :archived archived
                             {:order-by [[:%lower.name :asc]]})]
     (t2/hydrate (filter mi/can-read? snippets) :creator)))
 

src/metabase/api/pulse.clj

@@ -83,11 +83,11 @@
   This may include subscriptions which the current user does not have collection permissions for, in which case
   some sensitive metadata (the list of cards and recipients) is stripped out."
   [archived dashboard_id creator_or_recipient]
-  {archived             [:maybe ms/BooleanString]
+  {archived             [:maybe ms/BooleanValue]
    dashboard_id         [:maybe ms/PositiveInt]
-   creator_or_recipient [:maybe ms/BooleanString]}
-  (let [creator-or-recipient (Boolean/parseBoolean creator_or_recipient)
-        archived?            (Boolean/parseBoolean archived)
+   creator_or_recipient [:maybe ms/BooleanValue]}
+  (let [creator-or-recipient creator_or_recipient
+        archived?            archived
         pulses               (->> (pulse/retrieve-pulses {:archived?    archived?
                                                           :dashboard-id dashboard_id
                                                           :user-id      (when creator-or-recipient api/*current-user-id*)})

src/metabase/api/timeline.clj

@@ -41,8 +41,8 @@
   "Fetch a list of [[Timelines]]. Can include `archived=true` to return archived timelines."
   [include archived]
   {include  [:maybe Include]
-   archived [:maybe ms/BooleanString]}
-  (let [archived? (Boolean/parseBoolean archived)
+   archived [:maybe ms/BooleanValue]}
+  (let [archived? archived
         timelines (->> (t2/select Timeline
                          {:where    [:and
                                      [:= :archived archived?]
@@ -60,10 +60,10 @@
   [id include archived start end]
   {id       ms/PositiveInt
    include  [:maybe Include]
-   archived [:maybe ms/BooleanString]
+   archived [:maybe ms/BooleanValue]
    start    [:maybe ms/TemporalString]
    end      [:maybe ms/TemporalString]}
-  (let [archived? (Boolean/parseBoolean archived)
+  (let [archived? archived
         timeline  (api/read-check (t2/select-one Timeline :id id))]
     (cond-> (t2/hydrate timeline :creator [:collection :can_write])
       ;; `collection_id` `nil` means we need to assoc 'root' collection

src/metabase/api/user.clj

@@ -188,13 +188,13 @@
   {status              [:maybe :string]
    query               [:maybe :string]
    group_id            [:maybe ms/PositiveInt]
-   include_deactivated [:maybe ms/BooleanString]}
+   include_deactivated [:maybe ms/BooleanValue]}
   (or
    api/*is-superuser?*
    (if group_id
      (validation/check-manager-of-group group_id)
      (validation/check-group-manager)))
-  (let [include_deactivated (Boolean/parseBoolean include_deactivated)
+  (let [include_deactivated include_deactivated
         group-id-clause     (when group_id [group_id])
         clauses             (user-clauses status query group-id-clause include_deactivated)]
     {:data (cond-> (t2/select

src/metabase/util/malli/schema.clj

@@ -230,13 +230,6 @@
      [:fn #(u/ignore-exceptions (<= 0 (Integer/parseInt %)))]]
     (deferred-tru "value must be a valid integer greater than or equal to zero.")))
 
-(def BooleanString
-  "Schema for a string that is a valid representation of a boolean (either `true` or `false`).
-   Defendpoint uses this to coerce the value for this schema to a boolean."
-  (mu/with-api-error-message
-    [:enum "true" "false" "TRUE" "FALSE"]
-    (deferred-tru "value must be a valid boolean string (''true'' or ''false'').")))
-
 (def TemporalString
   "Schema for a string that can be parsed by date2/parse."
   (mu/with-api-error-message

test/metabase/util/malli/schema_test.clj

@@ -57,9 +57,6 @@
            {:schema        ms/IntString
             :failed-cases  [:a "a" "1.5"]
             :success-cases ["1"]}
-           {:schema        ms/BooleanString
-            :failed-cases  [:false :true true "f" "t"]
-            :success-cases ["true" "false"]}
            {:schema        ms/TemporalString
             :failed-cases  ["random string"]
             :success-cases ["2019-10-28T13:14:15" "2019-10-28"]}