From 9655a2680c3d2e5dcbea62dfc895556e0cee9777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 5 May 2026 18:20:27 +0200 Subject: [PATCH] :bug: Prevent and repair token themes with inexistent sets --- common/src/app/common/files/tokens.cljc | 18 +++-- common/src/app/common/types/tokens_lib.cljc | 72 +++++++++++++------ .../types/tokens_migrations_test.cljc | 70 +++++++++++++++++- frontend/src/app/plugins/tokens.cljs | 12 ++-- frontend/src/app/plugins/utils.cljs | 2 +- frontend/translations/en.po | 4 ++ frontend/translations/es.po | 4 ++ 7 files changed, 147 insertions(+), 35 deletions(-) diff --git a/common/src/app/common/files/tokens.cljc b/common/src/app/common/files/tokens.cljc index 84fc5d93eb..1ae3db5487 100644 --- a/common/src/app/common/files/tokens.cljc +++ b/common/src/app/common/files/tokens.cljc @@ -287,12 +287,18 @@ (defn make-token-theme-schema [tokens-lib group name theme-id] - (sm/merge - ctob/schema:token-theme-attrs - [:map - [:group (make-token-theme-group-schema tokens-lib name theme-id)] ;; TODO how to keep error-fn from here? - [:name (make-token-theme-name-schema tokens-lib group theme-id)] - [:description {:optional true} schema:token-theme-description]])) + [:and + (sm/merge + ctob/schema:token-theme-attrs + [:map + [:group (make-token-theme-group-schema tokens-lib name theme-id)] ;; TODO how to keep error-fn from here? + [:name (make-token-theme-name-schema tokens-lib group theme-id)] + [:description {:optional true} schema:token-theme-description]]) + [:fn {:error/field :sets + :error/fn #(tr "errors.token-theme-not-existing-sets" (str/join ", " (:sets (:value %))))} + (fn [{:keys [sets]}] + (or (nil? tokens-lib) + (every? #(ctob/get-set-by-name tokens-lib %) sets)))]]) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; HELPERS diff --git a/common/src/app/common/types/tokens_lib.cljc b/common/src/app/common/types/tokens_lib.cljc index 3a85391623..177e12ff54 100644 --- a/common/src/app/common/types/tokens_lib.cljc +++ b/common/src/app/common/types/tokens_lib.cljc @@ -1160,25 +1160,26 @@ Will return a value that matches this schema: (if-let [theme (get-theme this id)] (let [group (:group theme) name (:name theme) - theme' (-> (make-token-theme (f theme)) - (assoc :modified-at (ct/now))) - group' (:group theme') - name' (:name theme') - same-group? (= group group') - same-name? (= name name') - same-path? (and same-group? same-name?)] - (TokensLib. sets - (if same-path? - (update themes group' assoc name' theme') - (-> themes - (d/oassoc-in-before [group name] [group' name'] theme') - (d/dissoc-in [group name]))) - (if same-path? - active-themes - (disj active-themes (join-theme-path group name))))) + theme' (make-token-theme (f theme))] + (if (= theme theme') + this + (let [theme' (assoc theme' :modified-at (ct/now)) + group' (:group theme') + name' (:name theme') + same-group? (= group group') + same-name? (= name name') + same-path? (and same-group? same-name?)] + (TokensLib. sets + (if same-path? + (update themes group' assoc name' theme') + (-> themes + (d/oassoc-in-before [group name] [group' name'] theme') + (d/dissoc-in [group name]))) + (if same-path? + active-themes + (disj active-themes (join-theme-path group name))))))) this)) - (delete-theme [this id] (let [theme (get-theme this id) [group name] [(:group theme) (:name theme)]] @@ -2183,6 +2184,18 @@ Will return a value that matches this schema: tokens-lib (get-sets tokens-lib)))) +(defn update-all-themes + "Walk through all themes in the library and apply the given function to them. + The function receives the library and the theme as arguments, + and should return the updated theme." + [tokens-lib update-fn] + (reduce (fn [lib theme] + (update-theme lib + (get-id theme) + #(update-fn lib %))) + tokens-lib + (get-themes tokens-lib))) + (defn fix-duplicate-token-set-ids "Given an instance of TokensLib fixes it internal sets data sturcture for ensure each set has unique id; @@ -2233,6 +2246,19 @@ Will return a value that matches this schema: (rename token (generate-name name match)) token)))))) +(defn fix-missing-sets-in-themes + [tokens-lib] + (let [existing-set-names (into #{} (map get-name) (get-sets tokens-lib)) + fix-theme-sets + (fn [_ theme] + (let [current-sets (:sets theme) + valid-sets (clojure.set/intersection current-sets existing-set-names)] + (if-not (= valid-sets current-sets) + (assoc theme :sets valid-sets) + theme)))] + + (update-all-themes tokens-lib fix-theme-sets))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; SERIALIZATION (FRESIAN) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -2350,7 +2376,8 @@ Will return a value that matches this schema: (migrate-to-v1-3) (migrate-to-v1-4) (map->tokens-lib) - (fix-conflicting-token-names))))) + (fix-conflicting-token-names) + (fix-missing-sets-in-themes))))) #?(:clj (defn- read-tokens-lib-v1-2 @@ -2366,7 +2393,8 @@ Will return a value that matches this schema: (migrate-to-v1-3) (migrate-to-v1-4) (map->tokens-lib) - (fix-conflicting-token-names))))) + (fix-conflicting-token-names) + (fix-missing-sets-in-themes))))) #?(:clj (defn- read-tokens-lib-v1-3 @@ -2382,7 +2410,8 @@ Will return a value that matches this schema: :active-themes active-themes} (migrate-to-v1-4) (map->tokens-lib) - (fix-conflicting-token-names))))) + (fix-conflicting-token-names) + (fix-missing-sets-in-themes))))) #?(:clj (defn- read-tokens-lib-v1-4 @@ -2396,7 +2425,8 @@ Will return a value that matches this schema: :themes themes :active-themes active-themes} (map->tokens-lib) - (fix-conflicting-token-names))))) + (fix-conflicting-token-names) + (fix-missing-sets-in-themes))))) #?(:clj (defn- write-tokens-lib diff --git a/common/test/common_tests/types/tokens_migrations_test.cljc b/common/test/common_tests/types/tokens_migrations_test.cljc index 62e07bef25..aa0c7c5fc8 100644 --- a/common/test/common_tests/types/tokens_migrations_test.cljc +++ b/common/test/common_tests/types/tokens_migrations_test.cljc @@ -209,4 +209,72 @@ (t/is (= "name1-1" (ctob/get-name token1'))) (t/is (= "name2-2" (ctob/get-name token2'))) (t/is (= "name1.subname1" (ctob/get-name token3'))) - (t/is (= "name2.subname2" (ctob/get-name token4')))))) \ No newline at end of file + (t/is (= "name2.subname2" (ctob/get-name token4')))))) + +(t/deftest test-v1-6-fix-token-names + + (t/testing "empty tokens-lib should not need any action" + (let [tokens-lib (ctob/make-tokens-lib) + tokens-lib' (ctob/fix-missing-sets-in-themes tokens-lib)] + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "library with a valid theme should not need any action" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1")) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2")) + (ctob/add-theme (ctob/make-token-theme + :id (thi/new-id! :theme1) + :name "theme1" + :sets #{"set1"}))) + tokens-lib' (ctob/fix-missing-sets-in-themes tokens-lib)] + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "library with a theme containing a non-existent set should have it removed, and the rest of the library should be unchanged" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1")) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2")) + (ctob/add-theme (ctob/make-token-theme + :id (thi/new-id! :theme1) + :name "theme1" + :sets #{"set1" "set3"})) ;; "set3" does not exist + (ctob/add-theme (ctob/make-token-theme + :id (thi/new-id! :theme2) + :name "theme2" + :sets #{"set1" "set2"}))) + tokens-lib' (ctob/fix-missing-sets-in-themes tokens-lib) + + set1 (ctob/get-set tokens-lib (thi/id :set1)) + set2 (ctob/get-set tokens-lib (thi/id :set2)) + theme1 (ctob/get-theme tokens-lib (thi/id :theme1)) + theme2 (ctob/get-theme tokens-lib (thi/id :theme2)) + set1' (ctob/get-set tokens-lib' (thi/id :set1)) + set2' (ctob/get-set tokens-lib' (thi/id :set2)) + theme1' (ctob/get-theme tokens-lib' (thi/id :theme1)) + theme2' (ctob/get-theme tokens-lib' (thi/id :theme2))] + + (t/is (= (:sets theme1') #{"set1"})) + (t/is (= (:sets theme2') #{"set1" "set2"})) + + (t/is (= (ctob/get-id set1') (ctob/get-id set1))) + (t/is (= (ctob/get-name set1') (ctob/get-name set1))) + (t/is (= (ctob/get-description set1') (ctob/get-description set1))) + (t/is (= (ctob/get-modified-at set1') (ctob/get-modified-at set1))) + (t/is (= (ctob/get-id set2') (ctob/get-id set2))) + (t/is (= (ctob/get-name set2') (ctob/get-name set2))) + (t/is (= (ctob/get-description set2') (ctob/get-description set2))) + (t/is (= (ctob/get-modified-at set2') (ctob/get-modified-at set2))) + + (t/is (= (ctob/get-id theme1') (ctob/get-id theme1))) + (t/is (= (ctob/get-name theme1') (ctob/get-name theme1))) + (t/is (= (ctob/get-description theme1') (ctob/get-description theme1))) + (t/is (= (ctob/get-id theme2') (ctob/get-id theme2))) + (t/is (= (ctob/get-name theme2') (ctob/get-name theme2))) + (t/is (= (ctob/get-description theme2') (ctob/get-description theme2)))))) diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index c1070d3bcd..d1cf441fc3 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -494,12 +494,12 @@ :addTheme {:enumerable false - :schema (fn [attrs] - [:tuple (-> (sm/schema (cfo/make-token-theme-schema - (u/locate-tokens-lib file-id) - (or (obj/get attrs "group") "") - (or (obj/get attrs "name") "") - nil)) + :schema (fn [args] + [:tuple (-> (cfo/make-token-theme-schema + (u/locate-tokens-lib file-id) + (get (first args) :group "") + (get (first args) :name "") + nil) (sm/dissoc-key :id))]) ;; We don't allow plugins to set the id :fn (fn [attrs] (let [theme (ctob/make-token-theme attrs)] diff --git a/frontend/src/app/plugins/utils.cljs b/frontend/src/app/plugins/utils.cljs index 81dfb6cb82..5765b6cc6b 100644 --- a/frontend/src/app/plugins/utils.cljs +++ b/frontend/src/app/plugins/utils.cljs @@ -280,7 +280,7 @@ [explain] (->> (:errors explain) (reduce csm/interpret-schema-problem {}) - (mapcat (comp seq val)) + #_(mapcat (comp seq val)) ;; FIXME: why is this for? it breaks the message (map (fn [[field {:keys [message]}]] (tr "plugins.validation.message" (name field) message))) (str/join ". "))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 674594a15c..cb04cdcfe6 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1679,6 +1679,10 @@ msgstr "Cannot complete drop, a set with same name already exists at path." msgid "errors.token-theme-already-exists" msgstr "Theme Option with the same name exists" +#: src/app/common/files/tokens.cljc:298 +msgid "errors.token-theme-not-existing-sets" +msgstr "The theme refers to some not existing sets: %s" + #: src/app/main/data/media.cljs:73 msgid "errors.unexpected-error" msgstr "An unexpected error occurred." diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 7e1b13564a..e1e90a5c44 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1644,6 +1644,10 @@ msgstr "" msgid "errors.token-theme-already-exists" msgstr "Ya existe un theme con este nombre" +#: src/app/common/files/tokens.cljc:298 +msgid "errors.token-theme-not-existing-sets" +msgstr "El tema referencia sets que no existen: %s" + #: src/app/main/data/media.cljs:73 msgid "errors.unexpected-error" msgstr "Ha ocurrido un error inesperado."