From fdcfae0bd142fc0c63cc51c397ffd8a278e05e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Thu, 26 Mar 2026 15:13:43 +0100 Subject: [PATCH] :wrench: Refactor remove-unneeded-objects using TDD --- .../src/app/common/files/comp_processors.cljc | 18 ++++ common/src/app/common/files/migrations.cljc | 11 +-- common/src/app/common/types/file.cljc | 3 +- .../files/comp_processors_test.cljc | 98 +++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/common/src/app/common/files/comp_processors.cljc b/common/src/app/common/files/comp_processors.cljc index ab7772c8ac..7b04e93e2d 100644 --- a/common/src/app/common/files/comp_processors.cljc +++ b/common/src/app/common/files/comp_processors.cljc @@ -11,6 +11,24 @@ "Repair, migration or transformation utilities for components." +(defn remove-unneeded-objects-in-components + "Some components have an :objects attribute, despite not being deleted. This removes it. + It also adds an empty :objects if it's deleted and does not have it." + [file] + (ctf/update-file-data + file + (fn [file-data] + (ctf/update-components + file-data + (fn [component] + (if (:deleted component) + (if (nil? (:objects component)) + (assoc component :objects {}) + component) + (if (contains? component :objects) + (dissoc component :objects) + component))))))) + (defn fix-missing-swap-slots "Locate shapes that have been swapped (i.e. their shape-ref does not point to the near match) but they don't have a swap slot. In this case, add one pointing to the near match." diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index d6070cdd9d..73cb9064b9 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1830,14 +1830,9 @@ (defmethod migrate-data "0019-remove-unneeded-objects-from-components" [data _] - ;; Some components have an `:objects` attribute, despite not being - ;; deleted. This migration removes it. - (letfn [(check-component [component] - (if (and (not (:deleted component)) - (contains? component :objects)) - (dissoc component :objects) - component))] - (d/update-when data :components check-component))) + (let [file {:id (:id data) :data data}] + (-> (cfcp/remove-unneeded-objects-in-components file) + :data))) (defmethod migrate-data "0020-fix-missing-swap-slots" [data _] diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 7262faad90..c5dde4699e 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -204,7 +204,8 @@ (defn update-file-data [file f] - (update file :data f)) + (when file + (update file :data f))) (defn containers-seq "Generate a sequence of all pages and all components, wrapped as containers" diff --git a/common/test/common_tests/files/comp_processors_test.cljc b/common/test/common_tests/files/comp_processors_test.cljc index 880bb57129..86bf075a35 100644 --- a/common/test/common_tests/files/comp_processors_test.cljc +++ b/common/test/common_tests/files/comp_processors_test.cljc @@ -14,8 +14,106 @@ [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.file :as ctf] [clojure.test :as t])) +(t/deftest test-remove-unneeded-objects-in-components + + (t/testing "nil file should return nil" + (let [file nil + file' (cfcp/remove-unneeded-objects-in-components file)] + (t/is (nil? file')))) + + (t/testing "empty file should not need any action" + (let [file (thf/sample-file :file1) + file' (cfcp/remove-unneeded-objects-in-components file)] + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "file without components should not need any action" + (let [file + (-> (thf/sample-file :file1) + (tho/add-frame-with-child :frame1 :shape1)) + + file' (cfcp/remove-unneeded-objects-in-components file)] + + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "file with non deleted components should not need any action" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :frame1 :shape1)) + + file' (cfcp/remove-unneeded-objects-in-components file)] + + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "file with deleted components should not need any action" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :frame1 :shape1) + (tho/delete-shape :frame1)) + + file' (cfcp/remove-unneeded-objects-in-components file)] + + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "file with non deleted components with :objects nil should remove it" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :frame1 :shape1) + (thc/update-component :component1 {:objects nil})) + + file' (cfcp/remove-unneeded-objects-in-components file) + + diff (d/map-diff file file') + + expected-diff {:data + {:components + {(thi/id :component1) + {}}}}] + + (t/is (= diff expected-diff)))) + + (t/testing "file with non deleted components with :objects should remove it" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :frame1 :shape1) + (thc/update-component :component1 {:objects {:sample 777}})) + + file' (cfcp/remove-unneeded-objects-in-components file) + + diff (d/map-diff file file') + + expected-diff {:data + {:components + {(thi/id :component1) + {:objects + [{:sample 777} nil]}}}}] + + (t/is (= diff expected-diff)))) + + (t/testing "file with deleted components without :objects should add an empty one" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :frame1 :shape1) + (tho/delete-shape :frame1) + (ctf/update-file-data + (fn [file-data] + (ctkl/update-component file-data (thi/id :component1) #(dissoc % :objects))))) + + file' (cfcp/remove-unneeded-objects-in-components file) + + diff (d/map-diff file file') + + expected-diff {:data + {:components + {(thi/id :component1) + {:objects + [nil {}]}}}}] + + (t/is (= diff expected-diff))))) + (t/deftest test-fix-missing-swap-slots (t/testing "nil file should return nil"