From b3770963b02d800484ee1266fb3797944b621bb9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 15 Apr 2020 17:28:57 +0200 Subject: [PATCH] :bug: Fix duplicate ids bug. Caused by :mov-object bad frame-id assignation. --- .../tests/uxbox/tests/test_common_pages.clj | 55 +++++-- common/uxbox/common/pages.cljc | 25 ++-- frontend/src/uxbox/main/data/workspace.cljs | 139 ++++++++---------- 3 files changed, 118 insertions(+), 101 deletions(-) diff --git a/backend/tests/uxbox/tests/test_common_pages.clj b/backend/tests/uxbox/tests/test_common_pages.clj index 05161dbf7..9f8213737 100644 --- a/backend/tests/uxbox/tests/test_common_pages.clj +++ b/backend/tests/uxbox/tests/test_common_pages.clj @@ -213,15 +213,15 @@ )) (t/deftest process-changes-move-objects - (let [frame-a-id (uuid/next) - frame-b-id (uuid/next) - group-a-id (uuid/next) - group-b-id (uuid/next) - rect-a-id (uuid/next) - rect-b-id (uuid/next) - rect-c-id (uuid/next) - rect-d-id (uuid/next) - rect-e-id (uuid/next) + (let [frame-a-id (uuid/custom 1) + frame-b-id (uuid/custom 2) + group-a-id (uuid/custom 3) + group-b-id (uuid/custom 4) + rect-a-id (uuid/custom 5) + rect-b-id (uuid/custom 6) + rect-c-id (uuid/custom 7) + rect-d-id (uuid/custom 8) + rect-e-id (uuid/custom 9) data (-> cp/default-page-data (assoc-in [cp/root :shapes] [frame-a-id]) (assoc-in [:objects frame-a-id] @@ -356,12 +356,47 @@ (t/is (= data res)))))) +(t/deftest process-changes-move-objects-3 + (let [shape-2-id (uuid/custom 1 2) + shape-3-id (uuid/custom 1 3) + frame-id (uuid/custom 1 1) + changes [{:type :add-obj + :id frame-id + :frame-id uuid/zero + :obj {:type :frame + :name "Frame"}} + {:type :add-obj + :frame-id frame-id + :id shape-2-id + :obj {:type :shape + :name "Shape"}} + {:type :add-obj + :id shape-3-id + :frame-id uuid/zero + :obj {:type :rect + :name "Shape"}}] + data (cp/process-changes cp/default-page-data changes)] + (t/testing "move inside->outside-inside" + (let [changes [{:type :mov-objects + :shapes [shape-3-id] + :parent-id frame-id} + {:type :mov-objects + :shapes [shape-3-id] + :parent-id uuid/zero}] + res (cp/process-changes data changes)] + + (t/is (= (get-in res [:objects shape-2-id :frame-id]) + (get-in data [:objects shape-2-id :frame-id]))) + (t/is (= (get-in res [:objects shape-3-id :frame-id]) + (get-in data [:objects shape-3-id :frame-id]))))))) + + (t/deftest process-changes-move-objects-2 (let [shape-1-id (uuid/custom 1 1) shape-2-id (uuid/custom 1 2) shape-3-id (uuid/custom 1 3) shape-4-id (uuid/custom 1 4) - group-1-id (uuid/custom 2 1) + group-1-id (uuid/custom 1 5) changes [{:type :add-obj :id shape-1-id :frame-id cp/root diff --git a/common/uxbox/common/pages.cljc b/common/uxbox/common/pages.cljc index 86070c44f..c2a064ea5 100644 --- a/common/uxbox/common/pages.cljc +++ b/common/uxbox/common/pages.cljc @@ -222,7 +222,6 @@ (defmethod process-change :mod-obj [data {:keys [id operations] :as change}] - (assert (contains? (:objects data) id) "process-change/mod-obj") (update data :objects (fn [objects] (if-let [obj (get objects id)] @@ -285,7 +284,7 @@ (let [invalid (calculate-invalid-targets shape-id (:objects data))] (not (invalid parent-id)))) - valid? (every? is-valid-move shapes) + valid? (every? is-valid-move shapes) ;; Add items into the :shapes property of the target parent-id insert-items @@ -293,7 +292,12 @@ (let [prev-shapes (or prev-shapes [])] (if index (insert-at-index prev-shapes index shapes) - (into prev-shapes shapes)))) + (reduce (fn [acc id] + (if (some #{id} acc) + acc + (conj acc id))) + prev-shapes + shapes)))) strip-id (fn [id] @@ -326,17 +330,18 @@ ;; Updates the frame-id references that might be outdated update-frame-ids - (fn update-frame-ids [data shape-id] - (as-> data $ - (assoc-in $ [:objects shape-id :frame-id] frame-id) - (reduce update-frame-ids $ (get-in $ [:objects shape-id :shapes]))))] + (fn update-frame-ids [data id] + (let [data (assoc-in data [:objects id :frame-id] frame-id) + obj (get-in data [:objects id])] + (cond-> data + (not= :frame (:type obj)) + (as-> $$ (reduce update-frame-ids $$ (:shapes obj))))))] - (if valid? + (when valid? (as-> data $ (update-in $ [:objects parent-id :shapes] insert-items) (reduce remove-in-parent $ shapes) - (reduce update-frame-ids $ (get-in $ [:objects parent-id :shapes]))) - data))) + (reduce update-frame-ids $ (get-in $ [:objects parent-id :shapes])))))) (defmethod process-operation :set [shape op] diff --git a/frontend/src/uxbox/main/data/workspace.cljs b/frontend/src/uxbox/main/data/workspace.cljs index 68e926e02..03d233788 100644 --- a/frontend/src/uxbox/main/data/workspace.cljs +++ b/frontend/src/uxbox/main/data/workspace.cljs @@ -1403,7 +1403,7 @@ (us/verify ::us/uuid ref-id) (us/verify number? index) - (ptk/reify ::reloacate-shape + (ptk/reify ::relocate-shape ptk/WatchEvent (watch [_ state stream] (let [page-id (::page-id state) @@ -1417,28 +1417,6 @@ [] {:commit-local? true})))))) -(defn commit-shape-order-change - [id] - (ptk/reify ::commit-shape-order-change - ptk/WatchEvent - (watch [_ state stream] - (let [pid (::page-id state) - obj (get-in state [:workspace-data pid :objects id]) - - cfrm (get-in state [:workspace-data pid :objects (:frame-id obj)]) - pfrm (get-in state [:pages-data pid :objects (:frame-id obj)]) - - cindex (d/index-of (:shapes cfrm) id) - pindex (d/index-of (:shapes pfrm) id) - - rchange {:type :mod-obj - :id (:id cfrm) - :operations [{:type :abs-order :id id :index cindex}]} - uchange {:type :mod-obj - :id (:id cfrm) - :operations [{:type :abs-order :id id :index pindex}]}] - (rx/of (commit-changes [rchange] [uchange])))))) - ;; --- Shape / Selection Alignment and Distribution @@ -1491,40 +1469,64 @@ ;; --- Temportal displacement for Shape / Selection +(defn- retrieve-toplevel-shapes + [objects] + (let [lookup #(get objects %) + root (lookup uuid/zero) + childs (:shapes root)] + (loop [id (first childs) + ids (rest childs) + res []] + (if (nil? id) + res + (let [obj (lookup id) + typ (:type obj)] + (recur (first ids) + (rest ids) + (if (= :frame typ) + (into res (:shapes obj)) + (conj res id)))))))) + +(defn- calculate-shape-to-frame-relationship-changes + [objects ids] + (loop [id (first ids) + ids (rest ids) + rch [] + uch []] + (if (nil? id) + [rch uch] + (let [obj (get objects id) + fid (calculate-frame-overlap objects obj)] + (if (not= fid (:frame-id obj)) + (recur (first ids) + (rest ids) + (conj rch {:type :mov-objects + :parent-id fid + :shapes [id]}) + (conj uch {:type :mov-objects + :parent-id (:frame-id obj) + :shapes [id]})) + (recur (first ids) + (rest ids) + rch + uch)))))) + (defn- rehash-shape-frame-relationship [ids] - (letfn [(impl-diff [state] - (loop [id (first ids) - ids (rest ids) - rch [] - uch []] - (if (nil? id) - [rch uch] - (let [pid (::page-id state) - objects (get-in state [:workspace-data pid :objects]) - obj (get objects id) - fid (calculate-frame-overlap objects obj)] - (if (not= fid (:frame-id obj)) - (recur (first ids) - (rest ids) - (conj rch {:type :mov-obj - :id id - :frame-id fid}) - (conj uch {:type :mov-obj - :id id - :frame-id (:frame-id obj)})) - (recur (first ids) - (rest ids) - rch - uch))))))] - (ptk/reify ::rehash-shape-frame-relationship - ptk/WatchEvent - (watch [_ state stream] - (let [[rch uch] (impl-diff state)] - (when-not (empty? rch) - (rx/of (commit-changes rch uch {:commit-local? true})))))))) + (ptk/reify ::rehash-shape-frame-relationship + ptk/WatchEvent + (watch [_ state stream] + (let [page-id (::page-id state) + objects (get-in state [:workspace-data page-id :objects]) + ids (retrieve-toplevel-shapes objects) + [rch uch] (calculate-shape-to-frame-relationship-changes objects ids) + ] -(defn- adjust-group-shapes [state ids] + (when-not (empty? rch) + (rx/of (commit-changes rch uch {:commit-local? true}))))))) + +(defn- adjust-group-shapes + [state ids] (let [page-id (::page-id state) objects (get-in state [:workspace-data page-id :objects]) groups-to-adjust (->> ids @@ -1930,25 +1932,6 @@ (let [page-id (::page-id state)] (update-in state [:workspace-data page-id :objects id :segments index] gpt/add delta))))) -;; --- Initial Path Point Alignment - -;; ;; TODO: revisit on alignemt refactor -;; (deftype InitialPathPointAlign [id index] -;; ptk/WatchEvent -;; (watch [_ state s] -;; (let [shape (get-in state [:workspace-data :objects id]) -;; point (get-in shape [:segments index])] -;; (->> (uwrk/align-point point) -;; (rx/map #(update-path id index %)))))) - -;; (defn initial-path-point-align -;; "Event responsible of align a specified point of the -;; shape by `index` with the grid." -;; [id index] -;; {:pre [(uuid? id) -;; (number? index) -;; (not (neg? index))]} -;; (InitialPathPointAlign. id index)) ;; --- Shape Visibility @@ -2216,14 +2199,8 @@ ;; GROUPS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(defn get-parent [object-id objects] - (let [include-object - (fn [object] - (and (:shapes object) - (some #(= object-id %) (:shapes object))))] - (first (filter include-object objects)))) - -(defn group-shape [id frame-id selected selection-rect] +(defn group-shape + [id frame-id selected selection-rect] {:id id :type :group :name (name (gensym "Group-"))