Don't dissoc `:id` in `before-update` (#50375)

* Don't dissoc `:id` in `before-update`

There seems to be a nasty footgun in toucan2 here.

Say you're executing an `update!` command on a set of IDs, e.g.:

```
(t2/update! :model/Card :id [:in 1 2] {:view_count 1})
```

This works if:

- both cards 1 and 2 have `view_count=1` already

- both cards 1 and 2 have `view_count!=1`

However.

If one of the two cards has `view_count=1` and another has a different
view_count, then (if the `before-update` method doesn't have the primary
key attached) Toucan emits a call to update *every card in the
database*, without a where clause at all.

src/metabase/models/card.clj

@@ -595,8 +595,7 @@
         card.metadata/populate-result-metadata)
       pre-update
       populate-query-fields
-      maybe-populate-initially-published-at
-      (dissoc :id)))
+      maybe-populate-initially-published-at))
 
 ;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
 (t2/define-before-delete :model/Card

test/metabase/models/card_test.clj

@@ -1089,3 +1089,17 @@
     [[1 {:parameter_mappings [{:target [:dimension [:field 10 {:temporal-unit :month}]]}
                               {:target [:dimension [:field 33 {:temporal-unit :month}]]}
                               {:target [:dimension [:field 10 {:temporal-unit :month}]]}]}]]))
+
+(deftest update-does-not-break
+  ;; There's currently a footgun in Toucan2 - if 1) the result of `before-update` doesn't have an ID, 2) part of your
+  ;; `update` would change a subset of selected rows, and 3) part of your `update` would change *every* selected row
+  ;; (in this case, that's the `updated_at` we automatically set), then it emits an update without a `WHERE` clause.
+  ;;
+  ;;This can be removed after https://github.com/camsaul/toucan2/pull/196 is merged.
+  (mt/with-temp [:model/Card {card-1-id :id} {:name "Flippy"}
+                 :model/Card {card-2-id :id} {:name "Dog Man"}
+                 :model/Card {card-3-id :id} {:name "Petey"}]
+    (testing "only the two cards specified get updated"
+      (t2/update! :model/Card :id [:in [card-1-id card-2-id]]
+                  {:name "Flippy"})
+      (is (= "Petey" (t2/select-one-fn :name :model/Card :id card-3-id))))))