From c1a9bee798ab663ef8ac32e51f8ad91ec467465b Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Wed, 11 Mar 2026 17:36:21 +0800 Subject: [PATCH] fix: undo should skip conflicted move instead of clearing stack --- src/main/frontend/undo_redo.cljs | 6 +- src/test/frontend/undo_redo_test.cljs | 27 +++++++ .../frontend/worker/db_sync_sim_test.cljs | 72 +++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/main/frontend/undo_redo.cljs b/src/main/frontend/undo_redo.cljs index 49a8f983d1..0a487b1aa8 100644 --- a/src/main/frontend/undo_redo.cljs +++ b/src/main/frontend/undo_redo.cljs @@ -391,8 +391,10 @@ (log/error ::undo-redo-failed e) (clear-history! repo))))) (do - (clear-history! repo) - (if undo? ::empty-undo-stack ::empty-redo-stack)))))))) + (log/warn ::undo-redo-skip-conflicted-op + {:undo? undo? + :outliner-op (:outliner-op tx-meta)}) + (undo-redo-aux repo undo?)))))))) (when ((if undo? empty-undo-stack? empty-redo-stack?) repo) (prn (str "No further " (if undo? "undo" "redo") " information")) diff --git a/src/test/frontend/undo_redo_test.cljs b/src/test/frontend/undo_redo_test.cljs index ae3408f1e5..cf6f4b8edb 100644 --- a/src/test/frontend/undo_redo_test.cljs +++ b/src/test/frontend/undo_redo_test.cljs @@ -348,6 +348,33 @@ (is (= child-uuid (:block/uuid (:block/parent parent)))) (is (= page-uuid (:block/uuid (:block/parent child)))))))) +(deftest undo-skips-conflicted-move-and-keeps-earlier-history-test + (testing "undo drops a conflicting move op but still undoes earlier safe ops" + (undo-redo/clear-history! test-db) + (let [conn (db/get-db test-db false) + {:keys [parent-a-uuid parent-b-uuid child-uuid]} (seed-page-two-parents-child!)] + (d/transact! conn + [[:db/add [:block/uuid child-uuid] :block/title "local-title"]] + {:outliner-op :save-block + :local-tx? true}) + (d/transact! conn + [[:db/add [:block/uuid child-uuid] :block/parent [:block/uuid parent-b-uuid]]] + {:outliner-op :move-blocks + :local-tx? true}) + (d/transact! conn + [[:db/retractEntity [:block/uuid parent-a-uuid]]] + {:outliner-op :delete-blocks + :local-tx? false}) + (let [undo-result (undo-redo/undo test-db) + child (d/entity @conn [:block/uuid child-uuid])] + (is (not= :frontend.undo-redo/empty-undo-stack undo-result)) + (is (= "child" (:block/title child))) + (is (= parent-b-uuid + (:block/uuid (:block/parent child)))) + (is (empty? (db-issues @conn)))) + (is (= :frontend.undo-redo/empty-undo-stack + (undo-redo/undo test-db)))))) + (deftest undo-validation-fast-path-skips-db-issues-for-non-structural-tx-test (testing "undo validation skips db-issues for non-structural tx-data" (let [conn (db/get-db test-db false) diff --git a/src/test/frontend/worker/db_sync_sim_test.cljs b/src/test/frontend/worker/db_sync_sim_test.cljs index 8838364da0..16afcf1a08 100644 --- a/src/test/frontend/worker/db_sync_sim_test.cljs +++ b/src/test/frontend/worker/db_sync_sim_test.cljs @@ -1266,6 +1266,78 @@ (sync-loop! server [{:repo repo-a :conn conn-a :client client-a :online? true}]) (is (= "test" (:block/title (d/entity @conn-a [:block/uuid block-uuid]))))))))) +(deftest ^:long two-clients-undo-skips-conflicted-move-but-keeps-db-valid-test + (testing "undo skips a conflicted move while syncing the remaining safe history" + (let [base-uuid (uuid "31111111-1111-1111-1111-111111111111") + parent-a-uuid (uuid "32222222-2222-2222-2222-222222222222") + parent-b-uuid (uuid "33333333-3333-3333-3333-333333333333") + child-uuid (uuid "34444444-4444-4444-4444-444444444444") + conn-a (db-test/create-conn) + conn-b (db-test/create-conn) + ops-a (d/create-conn client-op/schema-in-db) + ops-b (d/create-conn client-op/schema-in-db) + client-a (make-client repo-a) + client-b (make-client repo-b) + server (make-server) + seed 20260311 + history (atom [])] + (with-test-repos {repo-a {:conn conn-a :ops-conn ops-a} + repo-b {:conn conn-b :ops-conn ops-b}} + (fn [] + (let [{:keys [repro restore]} (install-invalid-tx-repro! seed history)] + (try + (reset! db-sync/*repo->latest-remote-tx {}) + (client-op/update-local-tx repo-a 0) + (client-op/update-local-tx repo-b 0) + (ensure-base-page! conn-a base-uuid) + (let [base-a (d/entity @conn-a [:block/uuid base-uuid])] + (create-block! conn-a base-a "parent-a" parent-a-uuid) + (create-block! conn-a base-a "parent-b" parent-b-uuid) + (let [parent-a (d/entity @conn-a [:block/uuid parent-a-uuid])] + (create-block! conn-a parent-a "seed-child" child-uuid))) + (sync-until-idle! server [{:repo repo-a :conn conn-a :client client-a :online? true} + {:repo repo-b :conn conn-b :client client-b :online? true}] + 20) + + (update-title! conn-a child-uuid "local-title") + (move-block! conn-a + {:block/uuid child-uuid} + {:block/uuid parent-b-uuid}) + (sync-until-idle! server [{:repo repo-a :conn conn-a :client client-a :online? true} + {:repo repo-b :conn conn-b :client client-b :online? true}] + 50) + + (delete-block! conn-b parent-a-uuid) + (sync-until-idle! server [{:repo repo-a :conn conn-a :client client-a :online? true} + {:repo repo-b :conn conn-b :client client-b :online? true}] + 50) + + (is (not= :frontend.undo-redo/empty-undo-stack + (undo-redo/undo repo-a))) + + (let [rounds (sync-until-idle! server [{:repo repo-a :conn conn-a :client client-a :online? true} + {:repo repo-b :conn conn-b :client client-b :online? true}] + 50) + child-a (d/entity @conn-a [:block/uuid child-uuid]) + child-b (d/entity @conn-b [:block/uuid child-uuid]) + attrs-a (block-attr-map @conn-a) + attrs-b (block-attr-map @conn-b) + issues-a (db-issues @conn-a) + issues-b (db-issues @conn-b)] + (is (< rounds 50) (str "sync did not become idle rounds=" rounds)) + (is (= "seed-child" (:block/title child-a))) + (is (= "seed-child" (:block/title child-b))) + (is (= parent-b-uuid + (:block/uuid (:block/parent child-a)))) + (is (= parent-b-uuid + (:block/uuid (:block/parent child-b)))) + (is (empty? issues-a) (str "db A issues " (pr-str issues-a))) + (is (empty? issues-b) (str "db B issues " (pr-str issues-b))) + (assert-synced-attrs! seed history attrs-a attrs-b attrs-b) + (assert-no-invalid-tx! seed history repro)) + (finally + (restore))))))))) + (defonce op-runs 200) (defn- run-random-ops!