🐛 Add a migration dropping the obsolete stroke-per-side attr (#11946)

Individual stroke widths shipped with a `:stroke-per-side` boolean on every
stroke, telling the renderer and the CSS generator whether the four
per-side widths were meaningful. Comparing the sides answers that on its
own, so the attribute was later dropped from the closed stroke schema
and the toggle became ephemeral editor state.

Files written between those two changes still carry the attribute, and
the closed schema rejects the unknown key, so loading such a file fails
`check-file-data` with a `:malli.core/extra-key` error and surfaces as
an internal error in the editor.

Add migration `0030-remove-stroke-per-side-attr`, which drops the
attribute from every stroke of every page and component shape. The
per-side widths are the saved design data and are kept untouched, so a
file whose boolean was `true` renders exactly as before.

The migration is naturally idempotent and a no-op for files that never
carried the attribute, since `dissoc` on a map without the key returns
an equal map.

Cover it with tests for the schema rejection, the per-side and global
widths surviving, component shapes, idempotency, and the run through
`migrate-file`.

Closes #11943

AI-assisted-by: space-bunny-free

.serena/memories/common/file-change-validation-migration-subtleties.md

@@ -28,4 +28,5 @@
 - Prefer optional attrs/default behavior so old files continue working without migration. If absence cannot preserve old behavior, add a migration.
 - Migrations are an ordered set mixing legacy version-derived ids and newer named ids. Keep append order stable; `migrate` applies the set difference between available migrations and file migrations.
 - `migrate-file` synthesizes legacy migration ids from old numeric versions when `:migrations` is absent, migrates legacy features, and records feature flags created through `cfeat/*new*`.
-- When a file had no previous `:migrations`, `migrate-file` marks all migrations as migrated in metadata so callers persist the complete migration set, not only transformations that changed data.
+- When a file had no previous `:migrations`, `migrate-file` marks all migrations as migrated in metadata so callers persist the complete migration set, not only transformations that changed data.
+- Closed schemas (`schema:stroke-attrs` and others) reject a key that was dropped from the declaration, so removing a declared attr needs a migration in the same change, for data already written with it.

common/src/app/common/files/migrations.cljc

@@ -2132,6 +2132,25 @@
         (update :pages-index d/update-vals repair-container)
         (d/update-when :components d/update-vals repair-container))))
 
+(defmethod migrate-data "0030-remove-stroke-per-side-attr"
+  ;; Individual stroke widths shipped with a `:stroke-per-side` boolean on every
+  ;; stroke, telling the renderer and the CSS generator whether the four side
+  ;; widths were meaningful. Comparing the sides answers that on its own, so
+  ;; the attribute was dropped from the closed stroke schema and the toggle
+  ;; became ephemeral editor state. Files written while it was still persisted
+  ;; fail validation with an `:malli.core/extra-key` error; drop the attribute
+  ;; and keep the side widths, which hold the real design data.
+  [data _]
+  (letfn [(repair-shape [shape]
+            (d/update-when shape :strokes d/update-vals #(dissoc % :stroke-per-side)))
+
+          (repair-container [container]
+            (d/update-when container :objects d/update-vals repair-shape))]
+
+    (-> data
+        (update :pages-index d/update-vals repair-container)
+        (d/update-when :components d/update-vals repair-container))))
+
 (def available-migrations
   (into (d/ordered-set)
         ["legacy-2"
@@ -2218,4 +2237,5 @@
          "0026-fix-svg-raw-shapes-uuids"
          "0027-separate-tokens-status"
          "0028-normalize-constrained-values"
-         "0029-move-background-blur-out-of-blur"]))
+         "0029-move-background-blur-out-of-blur"
+         "0030-remove-stroke-per-side-attr"]))

common/test/common_tests/files_migrations_test.cljc

@@ -317,3 +317,145 @@
           "background blur moved before schema validation")
     (t/is (nil? (get-in file' [:data :pages-index page-id :objects shape-id :blur]))
           "mis-typed :blur removed")))
+
+(t/deftest migration-0030-removes-stroke-per-side-attr
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        page-id      (uuid/next)
+        shape-id     (uuid/next)
+        stroke       {:stroke-width 2
+                      :stroke-width-top 2
+                      :stroke-width-right 4
+                      :stroke-width-bottom 3
+                      :stroke-width-left 6
+                      :stroke-color "#000000"
+                      :stroke-per-side true}
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        data         {:pages-index {page-id {:objects {shape-id shape}}}}
+        data'        (cfm/migrate-data data migration-id)
+        shape'       (get-in data' [:pages-index page-id :objects shape-id])
+        stroke'      (first (:strokes shape'))]
+
+    (t/is (nil? (:stroke-per-side stroke')) "obsolete :stroke-per-side removed")
+    (t/is (= (dissoc stroke :stroke-per-side) stroke')
+          "no other stroke attr changed")
+    (t/is (= [2 4 3 6] [(:stroke-width-top stroke')
+                        (:stroke-width-right stroke')
+                        (:stroke-width-bottom stroke')
+                        (:stroke-width-left stroke')])
+          "per-side widths preserved")
+    (t/is (= data' (cfm/migrate-data data' migration-id))
+          "migration is idempotent")))
+
+(t/deftest migration-0030-repairs-strokes-rejected-by-schema
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        file-id      (uuid/next)
+        page-id      (uuid/next)
+        shape-id     (uuid/next)
+        stroke       {:stroke-width 1
+                      :stroke-width-top 1
+                      :stroke-width-right 1
+                      :stroke-width-bottom 1
+                      :stroke-width-left 1
+                      :stroke-color "#000000"
+                      :stroke-per-side true}
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        data         (-> (ctf/make-file-data file-id page-id)
+                         (assoc-in [:pages-index page-id :objects shape-id] shape))]
+
+    (t/is (thrown? #?(:clj Exception :cljs js/Error)
+                   (ctf/check-file-data data))
+          "new schema rejects a stroke carrying :stroke-per-side")
+
+    (let [data' (cfm/migrate-data data migration-id)]
+      (t/is (= data' (ctf/check-file-data data'))
+            "migrated data passes the schema"))))
+
+(t/deftest migration-0030-repairs-component-strokes
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        component-id (uuid/next)
+        shape-id     (uuid/next)
+        stroke       {:stroke-width 3
+                      :stroke-width-top 3
+                      :stroke-width-right 5
+                      :stroke-width-bottom 3
+                      :stroke-width-left 3
+                      :stroke-color "#000000"
+                      :stroke-per-side true}
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        data         {:components {component-id {:objects {shape-id shape}}}}
+        data'        (cfm/migrate-data data migration-id)
+        stroke'      (get-in data' [:components component-id :objects shape-id :strokes 0])]
+
+    (t/is (nil? (:stroke-per-side stroke'))
+          "obsolete attr removed from a component stroke")
+    (t/is (= 5 (:stroke-width-right stroke'))
+          "component per-side width preserved")))
+
+(t/deftest migration-0030-keeps-strokes-without-the-obsolete-attr
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        page-id      (uuid/next)
+        shape-id     (uuid/next)
+        stroke       {:stroke-width 2
+                      :stroke-width-top 2
+                      :stroke-width-right 2
+                      :stroke-width-bottom 2
+                      :stroke-width-left 2}
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        data         {:pages-index {page-id {:objects {shape-id shape}}}}]
+
+    (t/is (= data (cfm/migrate-data data migration-id))
+          "a stroke without the obsolete attr is left untouched")))
+
+(t/deftest migration-0030-keeps-distinct-sides-when-the-attr-was-false
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        page-id      (uuid/next)
+        shape-id     (uuid/next)
+        ;; The old toggle set the flag to false without equalizing the sides,
+        ;; so the per-side values are the saved design intent and must survive.
+        stroke       {:stroke-width 1
+                      :stroke-width-top 1
+                      :stroke-width-right 8
+                      :stroke-width-bottom 1
+                      :stroke-width-left 1
+                      :stroke-color "#000000"
+                      :stroke-per-side false}
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        data         {:pages-index {page-id {:objects {shape-id shape}}}}
+        stroke'      (get-in (cfm/migrate-data data migration-id)
+                             [:pages-index page-id :objects shape-id :strokes 0])]
+
+    (t/is (nil? (:stroke-per-side stroke')) "obsolete attr removed")
+    (t/is (= 8 (:stroke-width-right stroke'))
+          "distinct side width kept instead of being equalized")))
+
+(t/deftest migration-0030-runs-through-file-migration
+  (let [migration-id "0030-remove-stroke-per-side-attr"
+        shape-id     (uuid/next)
+        stroke       {:stroke-width 5
+                      :stroke-width-top 5
+                      :stroke-width-right 5
+                      :stroke-width-bottom 5
+                      :stroke-width-left 5
+                      :stroke-color "#000000"
+                      :stroke-per-side true}
+        file         (ctf/make-file {:name "Legacy per-side stroke"})
+        page-id      (first (get-in file [:data :pages]))
+        shape        (-> (cts/setup-shape {:id shape-id :type :rect})
+                         (assoc :strokes [stroke]))
+        file         (-> file
+                         (assoc :migrations (disj cfm/available-migrations migration-id))
+                         (assoc-in [:data :pages-index page-id :objects shape-id] shape))
+        file'        (cfm/migrate-file file {})
+        stroke'      (get-in file' [:data :pages-index page-id :objects shape-id :strokes 0])]
+
+    (t/is (cfm/need-migration? file) "new migration detected")
+    (t/is (not (cfm/need-migration? file')) "new migration recorded")
+    (t/is (contains? (:migrations file') migration-id) "migration id persisted")
+    (t/is (nil? (:stroke-per-side stroke'))
+          "obsolete attr removed before schema validation")
+    (t/is (= 5 (:stroke-width stroke')) "stroke width preserved")))