diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 1c8f023464..cc7f7e64ad 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -756,23 +756,27 @@ (vary-meta dissoc ::fmg/migrated)))) (defn process-imported-file - "Process a file read from a binfile. The swap slots whose near main is in a - library not linked yet are left for `convert-deferred-swaps!`." + "Process a file read from a binfile. The swap slots and old text overrides + whose near main is in a library not linked yet are left for + `convert-deferred!`." [cfg {:keys [id] :as file}] - (let [file (binding [cfcp/*defer-unreachable-swaps* true] - (process-file cfg file))] - (when (::cfcp/deferred-swaps (meta (:data file))) - (vswap! *state* update :deferred-swaps (fnil conj #{}) id)) + (let [file (binding [cfcp/*defer-unreachable* true] + (process-file cfg file)) + data (meta (:data file))] + (when (or (::cfcp/deferred-swaps data) (::cfcp/deferred-texts data)) + (vswap! *state* update :deferred (fnil conj #{}) id)) file)) -(defn convert-deferred-swaps! - "Convert the swap slots the import left in its files, once the libraries - of the files are linked." +(defn convert-deferred! + "Convert the swap slots and old text overrides the import left in its + files, once the libraries of the files are linked." [cfg] - (doseq [file-id (:deferred-swaps @*state*)] + (doseq [file-id (:deferred @*state*)] (let [file (get-file cfg file-id :realize? true :migrate? false) libs (get-resolved-file-libraries cfg file)] - (update-file! cfg (update file :data cfcp/convert-swap-slots libs))))) + (update-file! cfg (update file :data #(-> % + (cfcp/convert-swap-slots libs) + (cfcp/convert-text-overrides libs))))))) (defn- encode-file [cfg {:keys [id features] :as file}] diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index 0494ec9680..7436f734d3 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -479,7 +479,7 @@ (read-section options)))) [:v1/metadata :v1/files :v1/rels :v1/sobjects]) - (bfc/convert-deferred-swaps! cfg) + (bfc/convert-deferred! cfg) (bfm/apply-pending-migrations! cfg) ;; Knowing that the ids of the created files are in index, diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index 944b537ecf..63f755ffb9 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -1255,7 +1255,7 @@ (let [resolution (resolve-and-link-libraries cfg files-info)] - (bfc/convert-deferred-swaps! cfg) + (bfc/convert-deferred! cfg) (bfm/apply-pending-migrations! cfg) {:file-ids file-ids :resolution resolution}))) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index bc3e938aad..b368ef305e 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -21,6 +21,8 @@ [app.common.test-helpers.compositions :as ctho] [app.common.test-helpers.files :as cthf] [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.common.test-helpers.texts :as cthtx] [app.common.thumbnails :as thc] [app.common.time :as ct] [app.common.types.component :as ctk] @@ -2679,14 +2681,14 @@ ;; the library is not linked yet: the swap is left as it is (let [imported (bfc/process-imported-file cfg file)] (bfc/insert-file! cfg imported) - (t/is (= #{(:id file)} (:deferred-swaps @bfc/*state*))) + (t/is (= #{(:id file)} (:deferred @bfc/*state*))) (t/is (contains? (:features imported) "components/swap-group")) (t/is (some? (-> imported :data :pages-index (get page-id) :objects (get (cthi/id :outer-star)) cfss/get-swap-slot)))) (db/insert! cfg :file-library-rel {:file-id (:id file) :library-file-id (:id library)}) - (bfc/convert-deferred-swaps! cfg) + (bfc/convert-deferred! cfg) (let [head (-> (bfc/get-file cfg (:id file) :migrate? false :realize? true) :data :pages-index (get page-id) :objects @@ -2694,3 +2696,45 @@ (t/is (= (cthi/id :inner-star) (:shape-ref head))) (t/is (ctk/swapped? head)) (t/is (nil? (cfss/get-swap-slot head))))))))) + +(t/deftest import-recomputes-deferred-text-overrides-once-libraries-are-linked + (cthi/reset-idmap!) + (let [profile (th/create-profile* 1) + project-id (:default-project-id profile) + old-migs (disj fmg/available-migrations "0034-recompute-text-overrides") + main [[["Price: "] ["$10" {:weight 700}]]] + library (-> (cthf/sample-file :library) + (cthtx/add-text-component-with-copy main)) + ;; a file with a copy of the library's text, recolored, with the + ;; style flag of older files + file (-> (cthf/sample-file :file1) + (cthc/instantiate-component :component1 :lib-copy + :library library + :children-labels [:lib-copy-text]) + (cths/update-shape :lib-copy-text :content + (cthtx/content [[["Price: " {:color :red}] ["$10" {:weight 700 :color :red}]]])) + (cths/update-shape :lib-copy-text :touched #{:content-group :text-content-attribute}) + (assoc :project-id project-id) + (assoc :migrations old-migs) + (update :features disj "components/text-overrides")) + page-id (-> file :data :pages first) + touched #(-> % :data :pages-index (get page-id) :objects (get (cthi/id :lib-copy-text)) :touched)] + + (db/tx-run! th/*system* + (fn [cfg] + (binding [bfc/*state* (volatile! {:index {}})] + (bfc/insert-file! cfg (assoc library :project-id project-id)) + + ;; the library is not linked yet: the flag is left as it is + (let [imported (bfc/process-imported-file cfg file)] + (bfc/insert-file! cfg imported) + (t/is (= #{(:id file)} (:deferred @bfc/*state*))) + (t/is (contains? (:features imported) "components/text-overrides")) + (t/is (= #{:content-group :text-content-attribute} (touched imported)))) + + (db/insert! cfg :file-library-rel {:file-id (:id file) + :library-file-id (:id library)}) + (bfc/convert-deferred! cfg) + + (t/is (= #{:content-group :text-content-fills} + (touched (bfc/get-file cfg (:id file) :migrate? false :realize? true))))))))) diff --git a/common/src/app/common/features.cljc b/common/src/app/common/features.cljc index 9c4d9617fe..f606abfebf 100644 --- a/common/src/app/common/features.cljc +++ b/common/src/app/common/features.cljc @@ -49,6 +49,7 @@ "fdata/path-data" "components/v2" "components/swap-group" + "components/text-overrides" "styles/v2" "layout/grid" "plugins/runtime" @@ -69,6 +70,7 @@ "layout/grid" "components/v2" "components/swap-group" + "components/text-overrides" "plugins/runtime" "design-tokens/v1" "tokens/numeric-input" @@ -99,7 +101,8 @@ (def no-team-inheritable-features #{"fdata/path-data" "fdata/shape-data-type" - "components/swap-group"}) + "components/swap-group" + "components/text-overrides"}) ;; This is a set of features that does not require an explicit ;; migration like components/v2 or the migration is not mandatory to @@ -111,6 +114,7 @@ "fdata/shape-data-type" "fdata/path-data" "components/swap-group" + "components/text-overrides" "tokens/numeric-input" "variants/v1"} (into frontend-only-features) diff --git a/common/src/app/common/files/comp_processors.cljc b/common/src/app/common/files/comp_processors.cljc index 1eeafdc487..d5f9c92bfa 100644 --- a/common/src/app/common/files/comp_processors.cljc +++ b/common/src/app/common/files/comp_processors.cljc @@ -16,7 +16,9 @@ [app.common.types.component :as ctk] [app.common.types.container :as ctn] [app.common.types.file :as ctf] - [app.common.types.shape.layout :as ctsl])) + [app.common.types.shape.layout :as ctsl] + [app.common.types.text :as cttx] + [clojure.set :as set])) (log/set-level! :warn) @@ -256,9 +258,11 @@ next-data (recur next-data (inc iteration))))))) -(def ^:dynamic *defer-unreachable-swaps* - "When true, `convert-swap-slots` leaves as they are the swap slots whose - near main it cannot reach, for a later pass with more libraries." +(def ^:dynamic *defer-unreachable* + "When true, the conversions of older forms that need a copy's near main + (`convert-swap-slots`, `convert-text-overrides`) leave as they are the + shapes whose near main they cannot reach, for a later pass with more + libraries." false) (defn- find-swap-position @@ -353,7 +357,7 @@ other slot (a swap in a main, or a slot a copy inherited) is dropped. Every pairing is found on the data as it was. With - `*defer-unreachable-swaps*`, a slot whose near main cannot be reached is + `*defer-unreachable*`, a slot whose near main cannot be reached is left, and the data carries `::deferred-swaps` in its metadata." [file-data libraries] (let [file {:id (:id file-data) :data file-data} @@ -373,7 +377,7 @@ (not (swap-made-in-copy? shape slot parent-ref file libraries)) shape' - (and (nil? parent-ref) *defer-unreachable-swaps*) + (and (nil? parent-ref) *defer-unreachable*) (do (vreset! deferred? true) shape) @@ -401,3 +405,66 @@ (cond-> file-data @deferred? (vary-meta assoc ::deferred-swaps true)))) + +(def ^:private legacy-text-groups + #{:text-content-attribute :text-content-structure}) + +(defn legacy-text-touched? + "Whether `shape` holds the text override groups of files older than the + feature components/text-overrides." + [shape] + (boolean (some legacy-text-groups (:touched shape)))) + +(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 + its near main `ref`: each text override group the old ones allowed + (`cttx/text-overrides`) where the copy differs from `ref`, or all of them + 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) + #{}) + groups (if (some? ref) + (set/intersection allowed (cttx/override-groups (:content ref) (:content shape))) + allowed)] + (cond-> (set/difference touched cttx/text-groups legacy-text-groups #{:content-group}) + (seq groups) + (into (conj groups :content-group))))) + +(defn convert-text-overrides + "Store the text overrides of the file's copies per letters and per style. + 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 + 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." + [file-data libraries] + (let [file {:id (:id file-data) :data file-data} + deferred? (volatile! false) + + convert-container + (fn [container] + (reduce (fn [container shape] + (if-not (legacy-text-touched? shape) + container + (let [ref (when (ctk/in-component-copy? shape) + (ctf/find-ref-shape file container libraries shape + :include-deleted? true))] + (if (and (nil? ref) (ctk/in-component-copy? shape) *defer-unreachable*) + (do (vreset! deferred? true) + container) + (assoc-in container [:objects (:id shape) :touched] + (recompute-text-touched shape ref)))))) + container + (vals (:objects container)))) + + file-data (ctf/update-containers file-data convert-container)] + (cond-> file-data + @deferred? + (vary-meta assoc ::deferred-texts true)))) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 14073ad0b0..49f139b6ef 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -2226,6 +2226,14 @@ {})] (cfcp/convert-swap-slots data libraries))) +(defmethod migrate-data "0034-recompute-text-overrides" + [data _] + (some-> cfeat/*new* (swap! conj "components/text-overrides")) + (let [libraries (if (:libs data) + (deref (:libs data)) + {})] + (cfcp/convert-text-overrides data libraries))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -2316,4 +2324,5 @@ "0030-remove-stroke-per-side-attr" "0031-migrate-stroke-width-token-attr" "0032-fix-children-not-in-parent" - "0033-convert-swap-slots-to-swap-groups"])) + "0033-convert-swap-slots-to-swap-groups" + "0034-recompute-text-overrides"])) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 89daab8c59..ecaa90e145 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -573,6 +573,18 @@ (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) +(defmethod repair-error :legacy-text-touched + [_ {:keys [shape page-id] :as error} file-data libraries] + (let [page (ctpl/get-page file-data page-id) + file {:id (:id file-data) :data file-data} + ref (when (ctk/in-component-copy? shape) + (ctf/find-ref-shape file (ctn/make-container page :page) libraries shape + :include-deleted? true))] + (log/debug :hint "repairing shape :legacy-text-touched" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] #(assoc % :touched (cfcp/recompute-text-touched % ref)))))) + (defmethod repair-error :misplaced-slot [_ {:keys [shape page-id] :as error} file-data _] (let [repair-shape diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index ab483c649d..ae612dc221 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.exceptions :as ex] + [app.common.files.comp-processors :as cfcp] [app.common.files.helpers :as cfh] [app.common.files.swap-slots :as cfss] [app.common.files.variant :as cfv] @@ -19,6 +20,7 @@ [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.shape-tree :as ctst] + [app.common.types.text :as cttx] [app.common.types.variant :as ctv] [app.common.uuid :as uuid] [cuerdas.core :as str])) @@ -61,6 +63,7 @@ :non-deleted-component-cannot-have-objects :instance-head-not-frame :invalid-text-touched + :legacy-text-touched :misplaced-slot :misplaced-swap-group :ref-not-in-near-main @@ -464,17 +467,20 @@ shape file page))) (defn- check-valid-touched - "Validate that the text touched flags are coherent." + "Validate that the text touched flags are coherent: a text override group + implies `:content-group`, and the groups of files older than the feature + components/text-overrides are gone." [shape file page] - (let [touched-groups (or (:touched shape) #{}) + (let [touched-groups (or (:touched shape) #{}) content-touched? (touched-groups :content-group) - text-touched? (or (touched-groups :text-content-text) - (touched-groups :text-content-attribute) - (touched-groups :text-content-structure))] - ;; For now we only check this combination, that has been reported in some bugs + text-touched? (some cttx/text-groups touched-groups)] (when (and text-touched? (not content-touched?)) (report-error :invalid-text-touched "This thape has text type touched but not content touched" + shape file page)) + (when (cfcp/legacy-text-touched? shape) + (report-error :legacy-text-touched + "This shape has the text touched groups of older files" shape file page)))) (defn- check-shape-main-root-top diff --git a/common/src/app/common/types/text.cljc b/common/src/app/common/types/text.cljc index 5cec2de518..d1599f6e31 100644 --- a/common/src/app/common/types/text.cljc +++ b/common/src/app/common/types/text.cljc @@ -985,7 +985,7 @@ [src-ks dst-ks] (if (map? src-ks) (= src-ks dst-ks) - (= 1 (count (into (set src-ks) dst-ks))))) + (<= (count (into (set src-ks) dst-ks)) 1))) (defn merge-overrides "The content of a copy text whose content is `copy` and whose main's is diff --git a/common/test/common_tests/files/convert_swap_slots_test.cljc b/common/test/common_tests/files/convert_swap_slots_test.cljc index e257fb9f5a..0abc4dacea 100644 --- a/common/test/common_tests/files/convert_swap_slots_test.cljc +++ b/common/test/common_tests/files/convert_swap_slots_test.cljc @@ -186,7 +186,7 @@ (t/deftest unreachable-near-main-can-be-left-for-later (let [[library file] (setup-library) - file' (binding [cfcp/*defer-unreachable-swaps* true] + file' (binding [cfcp/*defer-unreachable* true] (convert file))] (t/is (= (swap-state file :outer-star) (swap-state file' :outer-star))) (t/is (::cfcp/deferred-swaps (meta (:data file')))) diff --git a/common/test/common_tests/files/convert_text_overrides_test.cljc b/common/test/common_tests/files/convert_text_overrides_test.cljc new file mode 100644 index 0000000000..e45f4fc7e9 --- /dev/null +++ b/common/test/common_tests/files/convert_text_overrides_test.cljc @@ -0,0 +1,118 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns common-tests.files.convert-text-overrides-test + "The conversion of the text override groups of files older than the + feature components/text-overrides (`:text-content-attribute` and + `:text-content-structure`) to one group for the letters and one per + style. Texts are written in the notation of + `app.common.test-helpers.texts`." + (:require + [app.common.features :as cfeat] + [app.common.files.comp-processors :as cfcp] + [app.common.files.migrations :as fmg] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.common.test-helpers.texts :as thtx] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +(def ^:private main [[["Price: "] ["$10" {:weight 700}]]]) + +(defn- old-copy + "A file whose copy text is written as `copy` and holds the old touched + groups `touched`." + [copy touched] + (-> (thf/sample-file :file1) + (thtx/add-text-component-with-copy main) + (ths/update-shape :copy-child :content (thtx/content copy)) + (ths/update-shape :copy-child :touched touched))) + +(defn- convert + ([file] (convert file {})) + ([file libraries] + (update file :data cfcp/convert-text-overrides libraries))) + +(defn- touched + [file label] + (:touched (ths/get-shape file label))) + +(t/deftest a-style-flag-becomes-the-styles-that-differ + (t/is (= #{:content-group :text-content-fills} + (-> (old-copy [[["Price: " {:color :red}] ["$10" {:weight 700 :color :red}]]] + #{:content-group :text-content-attribute}) + (convert) + (touched :copy-child)))) + (t/testing "letters that differ are not an override the old groups had" + (t/is (= #{:content-group :text-content-fills} + (-> (old-copy [[["Cost: " {:color :red}] ["$10" {:weight 700 :color :red}]]] + #{:content-group :text-content-attribute}) + (convert) + (touched :copy-child)))))) + +(t/deftest a-structure-flag-becomes-the-letters-and-styles-that-differ + (t/is (= #{:content-group :text-content-text :text-content-weight} + (-> (old-copy [[["Price: " {:weight 700}] ["$10" {:weight 700}]] [["Ends Sunday"]]] + #{:content-group :text-content-structure}) + (convert) + (touched :copy-child))))) + +(t/deftest an-old-flag-where-nothing-differs-sets-nothing + (t/is (= #{:fill-group} + (-> (old-copy main #{:content-group :text-content-attribute :fill-group}) + (convert) + (touched :copy-child))))) + +(t/deftest a-letters-flag-alone-stays + (let [file (old-copy [[["Price: "] ["$20" {:weight 700}]]] #{:content-group :text-content-text})] + (t/is (= (touched file :copy-child) (touched (convert file) :copy-child))))) + +(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." + [] + (let [library (-> (thf/sample-file :library) + (thtx/add-text-component-with-copy main)) + file (-> (thf/sample-file :file1) + (thc/instantiate-component :component1 :lib-copy + :library library + :children-labels [:lib-copy-text]) + (ths/update-shape :lib-copy-text :content + (thtx/content [[["Price: " {:color :red}] ["$10" {:weight 700 :color :red}]]])) + (ths/update-shape :lib-copy-text :touched #{:content-group :text-content-attribute}))] + [library file])) + +(t/deftest an-unreachable-near-main-keeps-every-group-the-old-flag-allowed + (let [[library file] (library-and-file) + all-styles #{:content-group :text-content-family :text-content-weight :text-content-size + :text-content-line-height :text-content-spacing :text-content-decoration + :text-content-case :text-content-fills :text-content-typography + :text-content-align :text-content-direction :text-content-valign}] + (t/is (= all-styles (touched (convert file) :lib-copy-text))) + (t/is (= #{:content-group :text-content-fills} + (touched (convert file {(:id library) library}) :lib-copy-text))))) + +(t/deftest an-unreachable-near-main-can-be-left-for-later + (let [[library file] (library-and-file) + file' (binding [cfcp/*defer-unreachable* true] + (convert file))] + (t/is (= (touched file :lib-copy-text) (touched file' :lib-copy-text))) + (t/is (::cfcp/deferred-texts (meta (:data file')))) + (t/is (= #{:content-group :text-content-fills} + (touched (convert file' {(:id library) library}) :lib-copy-text))))) + +(t/deftest migration-converts-and-adds-the-feature + (let [old (-> (old-copy [[["Price: " {:color :red}] ["$10" {:weight 700 :color :red}]]] + #{:content-group :text-content-attribute}) + (update :features disj "components/text-overrides") + (assoc :migrations (disj fmg/available-migrations "0034-recompute-text-overrides"))) + file (fmg/migrate-file old (delay {}))] + (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))))) diff --git a/common/test/common_tests/files/repair_components_test.cljc b/common/test/common_tests/files/repair_components_test.cljc index 922a9d51db..113ba138f0 100644 --- a/common/test/common_tests/files/repair_components_test.cljc +++ b/common/test/common_tests/files/repair_components_test.cljc @@ -315,6 +315,14 @@ (t/is (nil? errors)) (t/is (contains? (:touched (ths/get-shape file' :copy-child)) :content-group)))) +(t/deftest legacy-text-touched + ;; the copy has the main's content, so the old flag stands for nothing + (let [[found file' errors] (repair (ths/update-shape (simple) :copy-child :touched + #{:content-group :text-content-attribute}))] + (t/is (= #{:legacy-text-touched} found)) + (t/is (nil? errors)) + (t/is (empty? (:touched (ths/get-shape file' :copy-child)))))) + ;; ---- Nesting (t/deftest root-main-not-allowed diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 2bfb14cdc0..03c828625b 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -24,6 +24,7 @@ [common-tests.files-migrations-test] [common-tests.files.comp-processors-test] [common-tests.files.convert-swap-slots-test] + [common-tests.files.convert-text-overrides-test] [common-tests.files.helpers-test] [common-tests.files.repair-components-test] [common-tests.files.repair-test] @@ -142,6 +143,7 @@ 'common-tests.files-migrations-test 'common-tests.files.comp-processors-test 'common-tests.files.convert-swap-slots-test + 'common-tests.files.convert-text-overrides-test 'common-tests.files.helpers-test 'common-tests.files.repair-components-test 'common-tests.files.repair-test