fix: undo doesn't handle retracting property history block

This commit is contained in:
Tienson Qin
2026-04-02 12:47:24 +08:00
parent 114d9f2ae1
commit 2185a5aecd
7 changed files with 304 additions and 45 deletions

View File

@@ -444,7 +444,7 @@
[:logseq.property.reaction/target :int]
[:block/created-at :int]
[:block/tx-id {:optional true} :int]
[:logseq.property/created-by-ref {:optional true} :int]
[:block/properties {:optional true} block-properties]
[:block/refs {:optional true} [:set :int]]]))
(def property-history-block*

View File

@@ -568,46 +568,135 @@
(get property-id))]
(property-ref-value db property-id value)))
(defn- property-history-refs-from-tx-data
[db-before db-after tx-data block-ids property-id]
(let [target-block-ids (->> block-ids
(keep (fn [block-id]
(:db/id (block-entity db-before block-id))))
set)
target-property-id (some-> (d/entity db-before property-id) :db/id)
history-eid->block-id (reduce (fn [acc d]
(if (and (:added d)
(= :logseq.property.history/block (:a d)))
(assoc acc (:e d) (:v d))
acc))
{}
tx-data)
history-eid->property-id (reduce (fn [acc d]
(if (and (:added d)
(= :logseq.property.history/property (:a d)))
(assoc acc (:e d) (:v d))
acc))
{}
tx-data)]
(->> history-eid->block-id
(keep (fn [[history-eid history-block-id]]
(when (and (contains? target-block-ids history-block-id)
(= target-property-id (get history-eid->property-id history-eid))
(nil? (d/entity db-before history-eid)))
(stable-entity-ref db-after history-eid))))
distinct
vec
seq)))
(defn- normalize-op-or-ops
[op-or-ops]
(cond
(nil? op-or-ops)
[]
(and (sequential? op-or-ops)
(seq op-or-ops)
(sequential? (first op-or-ops)))
(vec op-or-ops)
:else
[op-or-ops]))
(defn- prepend-history-cleanup-op
[cleanup-op op-or-ops]
(let [ops (normalize-op-or-ops op-or-ops)
ops' (if cleanup-op
(into [cleanup-op] ops)
ops)]
(seq ops')))
(defn- inverse-property-op
[db-before op args]
[db-before db-after tx-data op args]
(case op
:set-block-property
(let [[block-id property-id _value] args
before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(if (nil? before-value)
[:remove-block-property [block-ref property-id]]
[:set-block-property [block-ref property-id before-value]]))
block-ref (stable-entity-ref db-before block-id)
cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
db-before
db-after
tx-data
[block-id]
property-id)]
[:delete-blocks [history-refs {}]])]
(prepend-history-cleanup-op
cleanup-op
(if (nil? before-value)
[:remove-block-property [block-ref property-id]]
[:set-block-property [block-ref property-id before-value]])))
:remove-block-property
(let [[block-id property-id] args
before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(when (some? before-value)
[:set-block-property [block-ref property-id before-value]]))
block-ref (stable-entity-ref db-before block-id)
cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
db-before
db-after
tx-data
[block-id]
property-id)]
[:delete-blocks [history-refs {}]])]
(prepend-history-cleanup-op
cleanup-op
(when (some? before-value)
[:set-block-property [block-ref property-id before-value]])))
:batch-set-property
(let [[block-ids property-id _value _opts] args]
(->> block-ids
(keep (fn [block-id]
(let [before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(if (nil? before-value)
[:remove-block-property [block-ref property-id]]
[:set-block-property [block-ref property-id before-value]]))))
vec
seq))
(let [[block-ids property-id _value _opts] args
cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
db-before
db-after
tx-data
block-ids
property-id)]
[:delete-blocks [history-refs {}]])]
(prepend-history-cleanup-op
cleanup-op
(->> block-ids
(keep (fn [block-id]
(let [before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(if (nil? before-value)
[:remove-block-property [block-ref property-id]]
[:set-block-property [block-ref property-id before-value]]))))
vec
seq)))
:batch-remove-property
(let [[block-ids property-id _opts] args]
(->> block-ids
(keep (fn [block-id]
(let [before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(when (some? before-value)
[:set-block-property [block-ref property-id before-value]]))))
vec
seq))
(let [[block-ids property-id _opts] args
cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
db-before
db-after
tx-data
block-ids
property-id)]
[:delete-blocks [history-refs {}]])]
(prepend-history-cleanup-op
cleanup-op
(->> block-ids
(keep (fn [block-id]
(let [before-value (block-property-value db-before block-id property-id)
block-ref (stable-entity-ref db-before block-id)]
(when (some? before-value)
[:set-block-property [block-ref property-id before-value]]))))
vec
seq)))
nil))
@@ -853,16 +942,16 @@
(build-inverse-delete-page db-before page-uuid))
:set-block-property
(inverse-property-op db-before op args)
(inverse-property-op db-before db-after tx-data op args)
:remove-block-property
(inverse-property-op db-before op args)
(inverse-property-op db-before db-after tx-data op args)
:batch-set-property
(inverse-property-op db-before op args)
(inverse-property-op db-before db-after tx-data op args)
:batch-remove-property
(inverse-property-op db-before op args)
(inverse-property-op db-before db-after tx-data op args)
:create-property-text-block
(let [[_block-id _property-id _value opts] args

View File

@@ -364,6 +364,94 @@
:logical-outdenting? nil}]]]
inverse-outliner-ops)))))
(deftest derive-history-outliner-ops-property-history-blocks-undo-cleanup-test
(testing ":set-block-property inverse deletes newly created property history blocks"
(let [conn (db-test/create-conn-with-blocks
{:properties {:pnum {:logseq.property/type :number
:db/cardinality :db.cardinality/one}}
:pages-and-blocks
[{:page {:block/title "page"}
:blocks [{:block/title "task"
:build/properties {:pnum 1}}]}]})
block (db-test/find-block-by-content @conn "task")
block-id (:db/id block)
block-ref [:block/uuid (:block/uuid block)]
property-id (:db/id (d/entity @conn :user.property/pnum))
history-uuid (random-uuid)
{:keys [db-after tx-data]}
(d/with @conn
[[:db/add block-id :user.property/pnum 2]
{:db/id -1
:block/uuid history-uuid
:logseq.property.history/block block-id
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}]
{})
{:keys [inverse-outliner-ops]}
(op-construct/derive-history-outliner-ops
@conn
db-after
tx-data
{:outliner-op :set-block-property
:outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})]
(is (= :delete-blocks (ffirst inverse-outliner-ops)))
(is (= #{[:block/uuid history-uuid]}
(set (get-in inverse-outliner-ops [0 1 0]))))
(is (= [:set-block-property [block-ref :user.property/pnum 1]]
(second inverse-outliner-ops)))))
(testing ":batch-set-property inverse deletes all newly created property history blocks"
(let [conn (db-test/create-conn-with-blocks
{:properties {:pnum {:logseq.property/type :number
:db/cardinality :db.cardinality/one}}
:pages-and-blocks
[{:page {:block/title "page"}
:blocks [{:block/title "task-1"
:build/properties {:pnum 1}}
{:block/title "task-2"}]}]})
block-1 (db-test/find-block-by-content @conn "task-1")
block-2 (db-test/find-block-by-content @conn "task-2")
block-1-id (:db/id block-1)
block-2-id (:db/id block-2)
block-1-ref [:block/uuid (:block/uuid block-1)]
block-2-ref [:block/uuid (:block/uuid block-2)]
property-id (:db/id (d/entity @conn :user.property/pnum))
history-uuid-1 (random-uuid)
history-uuid-2 (random-uuid)
{:keys [db-after tx-data]}
(d/with @conn
[[:db/add block-1-id :user.property/pnum 2]
[:db/add block-2-id :user.property/pnum 2]
{:db/id -1
:block/uuid history-uuid-1
:logseq.property.history/block block-1-id
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}
{:db/id -2
:block/uuid history-uuid-2
:logseq.property.history/block block-2-id
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}]
{})
{:keys [inverse-outliner-ops]}
(op-construct/derive-history-outliner-ops
@conn
db-after
tx-data
{:outliner-op :batch-set-property
:outliner-ops [[:batch-set-property [[block-1-id block-2-id]
:user.property/pnum
2
{}]]]})]
(is (= :delete-blocks (ffirst inverse-outliner-ops)))
(is (= #{[:block/uuid history-uuid-1]
[:block/uuid history-uuid-2]}
(set (get-in inverse-outliner-ops [0 1 0]))))
(is (= [:set-block-property [block-1-ref :user.property/pnum 1]]
(second inverse-outliner-ops)))
(is (= [:remove-block-property [block-2-ref :user.property/pnum]]
(nth inverse-outliner-ops 2))))))
(deftest derive-history-outliner-ops-direct-outdent-with-extra-moved-blocks-keeps-semantic-ops-test
(testing "direct outdent keeps semantic indent-outdent op and inverse"
(let [conn (db-test/create-conn-with-blocks

View File

@@ -208,17 +208,18 @@
(when (and (true? (get property :logseq.property/enable-history?))
(:added d))
{:property property
:value (:v d)}))) datoms)]
(map
(fn [{:keys [property value]}]
(let [ref? (= :db.type/ref (:db/valueType property))
value-key (if ref? :logseq.property.history/ref-value :logseq.property.history/scalar-value)]
(sqlite-util/block-with-timestamps
{:block/uuid (ldb/new-block-id)
value-key value
:logseq.property.history/block (:db/id entity)
:logseq.property.history/property (:db/id property)})))
changes)))
:value (:v d)}))) datoms)
data (map
(fn [{:keys [property value]}]
(let [ref? (= :db.type/ref (:db/valueType property))
value-key (if ref? :logseq.property.history/ref-value :logseq.property.history/scalar-value)]
(sqlite-util/block-with-timestamps
{:block/uuid (ldb/new-block-id)
value-key value
:logseq.property.history/block (:db/id entity)
:logseq.property.history/property (:db/id property)})))
changes)]
data))
(defmethod handle-command :default [command _db entity datoms]
(throw (ex-info "Unhandled command"

View File

@@ -23,7 +23,7 @@
[logseq.outliner.pipeline :as outliner-pipeline]))
(def ^:private rtc-tx-or-download-graph?
(let [p (some-fn :rtc-op? :rtc-tx? :rtc-download-graph?)]
(let [p (some-fn :rtc-op? :rtc-tx? :rtc-download-graph? :transact-remote?)]
(fn [tx-meta]
(p tx-meta))))
@@ -437,7 +437,7 @@
(toggle-page-and-block db tx-report))
display-blocks-tx-data (add-missing-properties-to-typed-display-blocks db-after tx-data tx-meta)
ensure-query-tx-data (ensure-query-property-on-tag-additions tx-report)
commands-tx (when-not (rtc-tx-or-download-graph? tx-meta)
commands-tx (when-not (or (:undo? tx-meta) (rtc-tx-or-download-graph? tx-meta))
(commands/run-commands tx-report))
insert-templates-tx (when-not (rtc-tx-or-download-graph? tx-meta)
(insert-tag-templates tx-report))

View File

@@ -342,6 +342,7 @@
:gen-undo-ops? false
:persist-op? true
:undo? undo?
:redo? (:redo? tx-meta)
:db-sync/tx-id history-tx-id
:db-sync/source-tx-id (or (:db-sync/source-tx-id tx-meta)
tx-id)}

View File

@@ -35,6 +35,7 @@
[logseq.db.test.helper :as db-test]
[logseq.outliner.core :as outliner-core]
[logseq.outliner.op :as outliner-op]
[logseq.outliner.op.construct :as op-construct]
[logseq.outliner.page :as outliner-page]
[logseq.outliner.property :as outliner-property]
[promesa.core :as p]))
@@ -3096,6 +3097,39 @@
(set (map :db/ident (:block/tags block-restored)))))
(is (= base-history-count restored-history-count)))))))))
(deftest derive-history-set-block-property-inverse-includes-property-history-cleanup-test
(testing "derive-history-outliner-ops should delete created property-history block for set-block-property"
(let [conn (db-test/create-conn-with-blocks
{:properties {:pnum {:logseq.property/type :number
:db/cardinality :db.cardinality/one}}
:pages-and-blocks
[{:page {:block/title "page1"}
:blocks [{:block/title "task"
:build/properties {:pnum 1}}]}]})
block-before (db-test/find-block-by-content @conn "task")
block-id (:db/id block-before)
property-id (:db/id (d/entity @conn :user.property/pnum))
history-uuid (random-uuid)
{:keys [db-after tx-data]}
(d/with @conn
[[:db/add block-id :user.property/pnum 2]
{:db/id -1
:block/uuid history-uuid
:logseq.property.history/block block-id
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}]
{})
{:keys [inverse-outliner-ops]}
(op-construct/derive-history-outliner-ops
@conn
db-after
tx-data
{:outliner-op :set-block-property
:outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})]
(is (= :delete-blocks (ffirst inverse-outliner-ops)))
(is (= #{[:block/uuid history-uuid]}
(set (get-in inverse-outliner-ops [0 1 0])))))))
(deftest pending-reversed-txs-for-batch-status-changes-restore-base-db-test
(testing "fresh persisted reversed tx rows from repeated batch status changes should restore the base db"
(let [conn (db-test/create-conn-with-blocks
@@ -3139,6 +3173,52 @@
(set (map :db/ident (:block/tags block-restored)))))
(is (= base-history-count restored-history-count)))))))))
(deftest derive-history-batch-set-property-inverse-includes-property-history-cleanup-test
(testing "derive-history-outliner-ops should delete created property-history blocks for batch-set-property"
(let [conn (db-test/create-conn-with-blocks
{:properties {:pnum {:logseq.property/type :number
:db/cardinality :db.cardinality/one}}
:pages-and-blocks
[{:page {:block/title "page1"}
:blocks [{:block/title "task-1"
:build/properties {:pnum 1}}
{:block/title "task-2"}]}]})
block-1 (db-test/find-block-by-content @conn "task-1")
block-2 (db-test/find-block-by-content @conn "task-2")
property-id (:db/id (d/entity @conn :user.property/pnum))
history-uuid-1 (random-uuid)
history-uuid-2 (random-uuid)
{:keys [db-after tx-data]}
(d/with @conn
[[:db/add (:db/id block-1) :user.property/pnum 2]
[:db/add (:db/id block-2) :user.property/pnum 2]
{:db/id -1
:block/uuid history-uuid-1
:logseq.property.history/block (:db/id block-1)
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}
{:db/id -2
:block/uuid history-uuid-2
:logseq.property.history/block (:db/id block-2)
:logseq.property.history/property property-id
:logseq.property.history/scalar-value 2}]
{})
{:keys [inverse-outliner-ops]}
(op-construct/derive-history-outliner-ops
@conn
db-after
tx-data
{:outliner-op :batch-set-property
:outliner-ops [[:batch-set-property [[(:db/id block-1)
(:db/id block-2)]
:user.property/pnum
2
{}]]]})]
(is (= :delete-blocks (ffirst inverse-outliner-ops)))
(is (= #{[:block/uuid history-uuid-1]
[:block/uuid history-uuid-2]}
(set (get-in inverse-outliner-ops [0 1 0])))))))
(deftest normalize-rebased-pending-tx-keeps-reconstructive-reverse-for-retract-entity-test
(testing "rebased pending tx should keep non-empty reverse datoms even when forward tx collapses to retractEntity"
(let [conn (db-test/create-conn-with-blocks