From b4c2240a600dd1d762edc08e2be14f0a9428fa15 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Thu, 15 Oct 2026 15:10:09 +0200 Subject: [PATCH] :bug: Fix export detach and the migrations of copies Fixes in what export does to the copies of libraries it leaves out, and in the migrations of swap slots and copy texts (Phase 3c). - Drop library color refs in fills and strokes on detached export. The binfile export that detaches libraries drops each reference to another file. For colors it read only where the older format kept them (:fill-color-ref-file and :stroke-color-ref-file on the shape and on text nodes), so a library color ref inside :fills or :strokes, the current format, stayed and the exported file pointed at a library it does not carry (F57). Every color of the shape, its text nodes included, now goes through cts/process-shape-colors, which detaches the ones that refer to another file. file-test: a fill ref inside :fills is dropped. - Detach copies by where their refs and components point on export. The binfile export that detaches libraries decided for each copy shape from one file, the :component-file of the shape or of the closest head above it, and then dropped its component and its :shape-ref together (F52). That is wrong both ways: - a library copy nested in a local component refers, in each copy of that component, to the nested copy in the local main; using the head's own library, its :shape-ref was dropped, and the next sync treated the shape as one the copy added; - a dropped link kept :touched, :remote-synced and :main-instance; - the shapes stored in deleted components were not detached at all. A shape's :shape-ref now points into the file of the component of the top head of its copy, or of the closest swapped head above it, and the link goes with the shape's other component attributes (ctk/detach-shape) only when that file is another one. A head whose own component is in another file stops being a head and loses :swap-group, but stays a copy while its main shape is in this file. Deleted components' shapes are detached the same way. file-test: a dropped link loses :touched; a library copy nested in a local component keeps its ref in the component's copy, and the file validates; a deleted component's nested library copy is detached. - Follow a swap slot's chain through the pages of its mains. find-ref-id-for-swapped, which migration 0010 runs on files older than the swap group, walks a swapped shape's ref chain to the main child its swap slot names. It read each step's children from the page of the step's own component, not from the page the step lives on, so when the mains of a nesting sat on different pages it found nothing, and the migration then removed a valid slot: the swap was lost (F56). Each step now goes on in the container the ref lookup found it in. file-test: the original ref of a swap is found with the mains on two pages. - Remove ctf/get-component-container-from-head. Its last caller, the swap slot chain walk of migration 0010, now takes each step's container from the ref lookup. - Recompute copy texts touched with the content group alone. Files older than the text override groups mark a copy text whose content changed with :content-group alone. Sync reads that as nothing touched in the text and writes the main's content, so the copy lost its text and styles at the next sync of its main (F50). Migration 0034 converted the later groups but left this one. convert-text-overrides now also converts a copy text with :content-group and no text group, reading it as allowing every text group: compared with its near main, it takes the groups where it differs (all of them when the near main cannot be reached). Migration 0035 runs it on files that already passed 0034; a binfile import converts them with the other old text groups, deferred as they are. convert-text-overrides-test: such a copy takes the letters and styles that differ, nothing where nothing differs, and through the migration. text-sync-test: once migrated, the copy keeps its text and takes the main's new size. AI-assisted-by: claude-opus-5-5 --- .../src/app/common/files/comp_processors.cljc | 49 +++++++++------ common/src/app/common/files/migrations.cljc | 12 +++- common/src/app/common/types/file.cljc | 60 ++++++++++++------- .../files/convert_text_overrides_test.cljc | 21 +++++++ .../common_tests/logic/text_sync_test.cljc | 14 ++--- common/test/common_tests/types/file_test.cljc | 36 +++++++---- 6 files changed, 134 insertions(+), 58 deletions(-) diff --git a/common/src/app/common/files/comp_processors.cljc b/common/src/app/common/files/comp_processors.cljc index 724daa307c..5900fd6e03 100644 --- a/common/src/app/common/files/comp_processors.cljc +++ b/common/src/app/common/files/comp_processors.cljc @@ -74,12 +74,10 @@ (defn- find-next-related-swap-shape-id "Go up from the chain of references shapes that will eventually lead to the shape - with swap-slot-id as id. Returns the next shape on the chain" - [parent swap-slot-id libraries] - (let [container (ctf/get-component-container-from-head parent libraries) - objects (:objects container) - - children (cfh/get-children objects (:id parent)) + with swap-slot-id as id. Returns the next shape on the chain. `container` + holds `parent`." + [parent container swap-slot-id libraries] + (let [children (cfh/get-children (:objects container) (:id parent)) original-shape-id (->> children (filter #(= swap-slot-id (:id %))) first @@ -88,11 +86,14 @@ ;; Return the children which id is the swap-slot-id original-shape-id ;; No children with swap-slot-id as id, go up - (let [referenced-shape (ctf/find-ref-shape nil container libraries parent) + (let [referenced-shape (ctf/find-ref-shape nil container libraries parent :with-context? true) ;; Recursive call that will get the id of the next shape on ;; the chain that ends on a shape with swap-slot-id as id next-shape-id (when referenced-shape - (find-next-related-swap-shape-id referenced-shape swap-slot-id libraries))] + (find-next-related-swap-shape-id referenced-shape + (:container (meta referenced-shape)) + swap-slot-id + libraries))] ;; Return the children which shape-ref points to the next-shape-id (->> children (filter #(= next-shape-id (:shape-ref %))) @@ -108,10 +109,10 @@ parent (get objects (:parent-id shape)) parent-head (ctn/get-head-shape objects parent) - parent-ref (ctf/find-ref-shape nil container libraries parent-head)] + parent-ref (ctf/find-ref-shape nil container libraries parent-head :with-context? true)] (when (and swap-slot parent-ref) - (find-next-related-swap-shape-id parent-ref swap-slot libraries)))) + (find-next-related-swap-shape-id parent-ref (:container (meta parent-ref)) swap-slot libraries)))) ;; ---- Repairs @@ -416,6 +417,18 @@ [shape] (boolean (some legacy-text-groups (:touched shape)))) +(defn- content-group-alone? + "Whether the copy text `shape` has `:content-group` touched with no text + override group, as files older than the text override groups mark a copy + text whose content changed in any way." + [shape] + (let [touched (:touched shape)] + (and (cfh/text-shape? shape) + (ctk/in-component-copy? shape) + (contains? touched :content-group) + (not-any? cttx/text-groups touched) + (not-any? legacy-text-groups touched)))) + (defn recompute-text-touched "The touched groups of a copy text `shape` that holds the text override groups of files older than the feature components/text-overrides, given @@ -424,9 +437,10 @@ when `ref` is nil. A shape that is not a copy keeps none." [shape ref] (let [touched (:touched shape) - allowed (if (ctk/in-component-copy? shape) - (cttx/text-overrides touched) - #{}) + allowed (cond + (not (ctk/in-component-copy? shape)) #{} + (content-group-alone? shape) cttx/text-groups + :else (cttx/text-overrides touched)) groups (if (some? ref) (set/intersection allowed (cttx/override-groups (:content ref) (:content shape))) allowed)] @@ -439,9 +453,10 @@ Files older than the feature components/text-overrides mark a copy text with `:text-content-attribute` (any style) and `:text-content-structure` (the number of paragraphs or runs); now it holds `:text-content-text` and - one group per style unit (`cttx/style-units`). Each copy text with the - old groups is compared with its near main, and takes the groups the old - ones allowed where it differs. When the near main cannot be reached it + one group per style unit (`cttx/style-units`). Older files still mark it + with `:content-group` alone, which allows every group. Each copy text with + the old groups is compared with its near main, and takes the groups the + old ones allowed where it differs. When the near main cannot be reached it takes every group they allowed, so that it loses no override; with `*defer-unreachable*` it is left, and the data carries `::deferred-texts` in its metadata." @@ -452,7 +467,7 @@ convert-container (fn [container] (reduce (fn [container shape] - (if-not (legacy-text-touched? shape) + (if-not (or (legacy-text-touched? shape) (content-group-alone? shape)) container (let [ref (when (ctk/in-component-copy? shape) (ctf/find-ref-shape file container libraries shape diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 49f139b6ef..8ccdacdcc3 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -2234,6 +2234,15 @@ {})] (cfcp/convert-text-overrides data libraries))) +(defmethod migrate-data "0035-recompute-content-group-alone" + [data _] + ;; copy texts that older files mark with `:content-group` alone, which + ;; 0034 left; it converts nothing else again + (let [libraries (if (:libs data) + (deref (:libs data)) + {})] + (cfcp/convert-text-overrides data libraries))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -2325,4 +2334,5 @@ "0031-migrate-stroke-width-token-attr" "0032-fix-children-not-in-parent" "0033-convert-swap-slots-to-swap-groups" - "0034-recompute-text-overrides"])) + "0034-recompute-text-overrides" + "0035-recompute-content-group-alone"])) diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 428082fea6..1d1dc2e2b1 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -366,13 +366,6 @@ (cfh/make-container component-page :page)) (cfh/make-container component :component))) -(defn get-component-container-from-head - [instance-head libraries & {:keys [include-deleted?] :or {include-deleted? true}}] - (let [library-data (-> (get-component-library libraries instance-head) - :data) - component (ctkl/get-component library-data (:component-id instance-head) include-deleted?)] - (get-component-container library-data component))) - (defn get-component-root "Retrieve the root shape of the component." [file-data component] @@ -1161,20 +1154,21 @@ ;; Export -(defn- get-component-ref-file +(defn- get-shape-ref-file + "The file of the main shape the copy shape `shape` refers to: the file of + the component of the top head of its copy, or of the closest swapped head + above it, whose children refer into its own component." [objects shape] - - (cond - (contains? shape :component-file) - (get shape :component-file) - - (contains? shape :shape-ref) - (recur objects (get objects (:parent-id shape))) - - :else - nil)) + (let [parent (get objects (:parent-id shape))] + (cond + (not (ctk/in-component-copy? parent)) (:component-file shape) + (ctk/swapped? parent) (:component-file parent) + :else (recur objects parent)))) (defn detach-external-references + "Remove from `file` every reference to another file: colors, + typographies, components and the copy links to their mains, in the pages + and in the shapes of deleted components." [file file-id] (let [detach-text (fn [content] @@ -1187,17 +1181,31 @@ (not= file-id (:typography-ref-file %)) (dissoc :typography-ref-id :typography-ref-file))))) + detach-color + (fn [shape position {:keys [ref-file]} _ _ detach-fn] + (if (and (some? ref-file) (not= file-id ref-file)) + (detach-fn shape position) + shape)) + + ;; a head whose component is in another file stops being a head; it + ;; stays a copy while the main shape it refers to is in this file detach-shape (fn [objects shape] - (let [shape' (cond-> shape + (let [shape' (cond-> (cts/process-shape-colors shape detach-color) (not= file-id (:fill-color-ref-file shape)) (dissoc :fill-color-ref-id :fill-color-ref-file) (not= file-id (:stroke-color-ref-file shape)) (dissoc :stroke-color-ref-id :stroke-color-ref-file) - (not= file-id (get-component-ref-file objects shape)) - (dissoc :component-id :component-file :shape-ref :component-root) + (and (ctk/instance-head? shape) + (not= file-id (:component-file shape))) + (-> (ctk/unhead-shape) + (ctk/remove-swap-group)) + + (and (ctk/in-component-copy? shape) + (not= file-id (get-shape-ref-file objects shape))) + (ctk/detach-shape) (= :text (:type shape)) (update :content detach-text))] @@ -1215,9 +1223,15 @@ detach-pages (fn [pages-index] - (d/update-vals pages-index #(update % :objects detach-objects)))] + (d/update-vals pages-index #(update % :objects detach-objects))) - (update-in file [:data :pages-index] detach-pages))) + detach-components + (fn [components] + (d/update-vals components #(cond-> % (:deleted %) (update :objects detach-objects))))] + + (-> file + (update-in [:data :pages-index] detach-pages) + (d/update-in-when [:data :components] detach-components)))) ;; Base font size diff --git a/common/test/common_tests/files/convert_text_overrides_test.cljc b/common/test/common_tests/files/convert_text_overrides_test.cljc index e45f4fc7e9..f26935202a 100644 --- a/common/test/common_tests/files/convert_text_overrides_test.cljc +++ b/common/test/common_tests/files/convert_text_overrides_test.cljc @@ -73,6 +73,20 @@ (let [file (old-copy [[["Price: "] ["$20" {:weight 700}]]] #{:content-group :text-content-text})] (t/is (= (touched file :copy-child) (touched (convert file) :copy-child))))) +(t/deftest a-content-group-alone-becomes-the-letters-and-styles-that-differ + ;; older files mark a copy text whose content changed with + ;; `:content-group` alone + (t/is (= #{:content-group :text-content-text :text-content-fills} + (-> (old-copy [[["Cost: " {:color :red}] ["$10" {:weight 700 :color :red}]]] + #{:content-group}) + (convert) + (touched :copy-child)))) + (t/testing "where nothing differs it sets nothing" + (t/is (nil? (-> (old-copy main #{:content-group}) + (convert) + (touched :copy-child) + (not-empty)))))) + (defn- library-and-file "A library with the text component :component1, and a file with a copy of it whose text is red and holds the old style flag." @@ -116,3 +130,10 @@ (t/is (contains? (:features file) "components/text-overrides")) (t/is (contains? cfeat/default-features "components/text-overrides")) (t/is (= #{:content-group :text-content-fills} (touched file :copy-child))))) + +(t/deftest migration-converts-a-content-group-alone + (let [old (-> (old-copy [[["Cost: "] ["$10" {:weight 700}]]] #{:content-group}) + (assoc :migrations (disj fmg/available-migrations "0035-recompute-content-group-alone"))) + file (fmg/migrate-file old (delay {}))] + (t/is (= #{:content-group :text-content-text} (touched file :copy-child))))) + diff --git a/common/test/common_tests/logic/text_sync_test.cljc b/common/test/common_tests/logic/text_sync_test.cljc index a1827071ef..32296284fa 100644 --- a/common/test/common_tests/logic/text_sync_test.cljc +++ b/common/test/common_tests/logic/text_sync_test.cljc @@ -7,10 +7,10 @@ (ns common-tests.logic.text-sync-test (:require [app.common.files.changes-builder :as pcb] + [app.common.files.comp-processors :as cfcp] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [app.common.test-helpers.components :as thc] - [app.common.test-helpers.composable.core :as tm] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] @@ -890,7 +890,7 @@ (t/deftest sync-copy-touched-only-in-content-group ;; A copy text whose `:touched` has `:content-group` but no text subgroup, ;; as files from before the subgroups hold it, keeps its own content - ;; when the main changes. + ;; when the main changes, once the file is migrated. (let [;; ==== Setup file (-> (thf/sample-file :file1) (tho/add-frame-with-text :main-root :main-child "hello world") @@ -902,7 +902,8 @@ (assoc-in (:content copy-child) [:children 0 :children 0 :children 0 :text] "copy text")) - (ths/update-shape :copy-child :touched #{:content-group})) + (ths/update-shape :copy-child :touched #{:content-group}) + (update :data cfcp/convert-text-overrides {})) page (thf/current-page file) ;; ==== Action @@ -929,7 +930,6 @@ ;; ==== Get copy-child' (ths/get-shape file' :copy-child) line (get-in copy-child' [:content :children 0 :children 0 :children 0])] - ;; with no subgroup, sync takes the "nothing touched" branch and writes - ;; the main's content - (tm/known-failure "F50" "copy text" "hello world" (:text line)) - (t/is (= #{:content-group} (:touched copy-child'))))) + (t/is (= "copy text" (:text line))) + (t/is (= "32" (:font-size line))) + (t/is (= #{:content-group :text-content-text} (:touched copy-child'))))) diff --git a/common/test/common_tests/types/file_test.cljc b/common/test/common_tests/types/file_test.cljc index f6bb368d8c..3a1e0e19b0 100644 --- a/common/test/common_tests/types/file_test.cljc +++ b/common/test/common_tests/types/file_test.cljc @@ -11,16 +11,18 @@ components." (:require [app.common.data :as d] + [app.common.files.changes-builder :as pcb] [app.common.files.comp-processors :as cfcp] [app.common.files.swap-slots :as cfss] [app.common.files.validate :as cfv] + [app.common.logic.shapes :as cls] [app.common.test-helpers.components :as thc] - [app.common.test-helpers.composable.core :as tm] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.types.component :as ctk] + [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.pages-list :as ctpl] @@ -61,7 +63,7 @@ (t/testing "a copy of a library component becomes plain shapes" (t/is (= {} (links file' :lib-copy))) (t/is (= {} (links file' :lib-copy-child))) - (tm/known-failure "F52" nil #{:fill-group} (:touched (ths/get-shape file' :lib-copy-child)))) + (t/is (nil? (:touched (ths/get-shape file' :lib-copy-child))))) (t/testing "a copy of a local component keeps its links" (t/is (= (links file :copy) (links file' :copy))) (t/is (= (links file :copy-child) (links file' :copy-child)))) @@ -81,12 +83,26 @@ (t/testing "the nested library copy in the main becomes plain shapes" (t/is (= {} (links file' :nested-head)))) (t/testing "its copy keeps the ref to the main's shape, which is local" - (tm/known-failure "F52" - {:shape-ref (thi/id :nested-head)} - {} - (links file' :copy-nested-head))) - (t/testing "the validation does not see the lost ref" - (t/is (= #{} (validation-errors file')))))) + (t/is (= {:shape-ref (thi/id :nested-head)} (links file' :copy-nested-head)))) + (t/is (= #{} (validation-errors file'))))) + +(t/deftest detach-external-references-in-a-deleted-component + (let [library (library-with-component) + file (-> (thf/sample-file :file) + (tho/add-frame :main {:name "Frame2"}) + (thc/instantiate-component :lib-component :nested-head + :library library + :parent-label :main) + (thc/make-component :component :main)) + page (thf/current-page file) + [_ del] (cls/generate-delete-shapes (pcb/empty-changes nil (:id page)) + file page (:objects page) #{(thi/id :main)} {}) + file (thf/apply-changes file del :validate? false) + file' (ctf/detach-external-references file (:id file)) + stored #(get-in (ctkl/get-component (:data %) (thi/id :component) true) + [:objects (thi/id :nested-head)])] + (t/is (= (thi/id :lib-component) (:component-id (stored file)))) + (t/is (= {} (select-keys (stored file') link-attrs))))) ;; Fill and typography refs on the shape and on the text nodes themselves ;; (the older format), next to a fill ref inside `:fills`. @@ -141,7 +157,7 @@ [:fill-color-ref-file :stroke-color-ref-file])))) (t/is (= [{} {:typography-ref-file (:id file)}] (text-leaves file'))) (t/testing "a fill ref inside `:fills`" - (tm/known-failure "F57" nil library-id (-> rect' :fills first :fill-color-ref-file))))) + (t/is (nil? (-> rect' :fills first :fill-color-ref-file)))))) ;; ----- Asset usage @@ -230,7 +246,7 @@ (thc/instantiate-component :c3 :copy3) (swap-at-second-level))] (t/is (= (thi/id :nested-head1) (cfss/get-swap-slot (ths/get-shape file :swapped)))) - (tm/known-failure "F56" (thi/id :nested-subhead2) nil (ref-id-for-swapped file)))) + (t/is (= (thi/id :nested-subhead2) (ref-id-for-swapped file))))) ;; ----- Deleted components