Page tags and aliases should be deleted when removed by user

Fix #8627, fix #7720 and fix #8282

src/main/frontend/modules/outliner/core.cljs

@@ -141,42 +141,49 @@
           m (if (state/enable-block-timestamps?) (block-with-timestamps m) m)
           other-tx (:db/other-tx m)
           id (:db/id (:data this))
-          block-entity (db/entity id)
-          remove-self-page #(remove (fn [b]
-                                      (= (:db/id b) (:db/id (:block/page block-entity)))) %)
-          old-refs (remove-self-page (:block/refs block-entity))
-          new-refs (remove-self-page (:block/refs m))]
+          block-entity (db/entity id)]
       (when (seq other-tx)
         (swap! txs-state (fn [txs]
                            (vec (concat txs other-tx)))))
 
       (when id
+        ;; Retract attributes to prepare for tx which rewrites block attributes
         (swap! txs-state (fn [txs]
                            (vec
                             (concat txs
                                     (map (fn [attribute]
                                            [:db/retract id attribute])
-                                      db-schema/retract-attributes)))))
+                                         db-schema/retract-attributes)))))
 
+        ;; Update block's page attributes
         (when-let [e (:block/page block-entity)]
-          (let [m' {:db/id (:db/id e)
-                   :block/updated-at (util/time-ms)}
-                m' (if (:block/created-at e)
-                    m'
-                    (assoc m' :block/created-at (util/time-ms)))
-                m' (if (or (:block/pre-block? block-entity)
-                           (:block/pre-block? m))
-                     (let [properties (:block/properties m)
-                           alias (set (:alias properties))
-                           tags (set (:tags properties))
-                           alias (map (fn [p] {:block/name (util/page-name-sanity-lc p)}) alias)
-                           tags (map (fn [p] {:block/name (util/page-name-sanity-lc p)}) tags)]
-                       (assoc m'
-                              :block/alias alias
-                              :block/tags tags
-                              :block/properties properties))
-                     m')]
-            (swap! txs-state conj m'))
+          (let [m' (cond-> {:db/id (:db/id e)
+                            :block/updated-at (util/time-ms)}
+                     (not (:block/created-at e))
+                     (assoc :block/created-at (util/time-ms)))
+                txs (if (or (:block/pre-block? block-entity)
+                            (:block/pre-block? m))
+                      (let [properties (:block/properties m)
+                            alias (set (:alias properties))
+                            tags (set (:tags properties))
+                            alias (map (fn [p] {:block/name (util/page-name-sanity-lc p)}) alias)
+                            tags (map (fn [p] {:block/name (util/page-name-sanity-lc p)}) tags)
+                            deleteable-page-attributes {:block/alias alias
+                                                        :block/tags tags
+                                                        :block/properties properties
+                                                        :block/properties-text-values (:block/properties-text-values m)}
+                            ;; Retract page attributes to allow for deletion of page attributes
+                            page-retractions
+                            (mapv #(vector :db/retract (:db/id e) %) (keys deleteable-page-attributes))]
+                        (conj page-retractions (merge m' deleteable-page-attributes)))
+                      [m'])]
+            (swap! txs-state into txs)))
+
+        ;; Remove orphaned refs from block
+        (let [remove-self-page #(remove (fn [b]
+                                          (= (:db/id b) (:db/id (:block/page block-entity)))) %)
+              old-refs (remove-self-page (:block/refs block-entity))
+              new-refs (remove-self-page (:block/refs m))]
           (remove-orphaned-page-refs! (:db/id block-entity) txs-state old-refs new-refs)))
 
       (swap! txs-state conj (dissoc m :db/other-tx))

src/test/frontend/modules/outliner/core_test.cljs

@@ -10,7 +10,7 @@
             [clojure.walk :as walk]
             [logseq.graph-parser.block :as gp-block]
             [datascript.core :as d]
-            [frontend.test.helper :as test-helper]
+            [frontend.test.helper :as test-helper :refer [load-test-files]]
             [clojure.set :as set]))
 
 (def test-db test-helper/test-db)
@@ -440,6 +440,63 @@
          '(16 17)
          (map :block/uuid (tree/get-sorted-block-and-children test-db (:db/id (get-block 16))))))))
 
+(defn- save-block!
+  [block]
+  (outliner-tx/transact! {:graph test-db}
+                         (outliner-core/save-block! block)))
+
+(deftest save-test
+  (load-test-files [{:file/path "pages/page1.md"
+                     :file/content "alias:: foo, bar
+tags:: tag1, tag2
+- block #blarg #bar"}])
+  (testing "save deletes a page's tags"
+      (let [conn (db/get-db test-helper/test-db false)
+            pre-block (->> (d/q '[:find (pull ?b [*])
+                                  :where [?b :block/pre-block? true]]
+                                @conn)
+                           ffirst)
+            _ (save-block! (-> pre-block
+                               (update :block/properties dissoc :tags)
+                               (update :block/properties-text-values dissoc :tags)))
+            updated-page (-> (d/q '[:find (pull ?bp [* {:block/alias [*]}])
+                                    :where [?b :block/pre-block? true]
+                                    [?b :block/page ?bp]]
+                                  @conn)
+                             ffirst)]
+        (is (nil? (:block/tags updated-page))
+            "Page's tags are deleted")
+        (is (= #{"foo" "bar"} (set (map :block/name (:block/alias updated-page))))
+            "Page's aliases remain the same")
+        (is (= {:block/properties {:alias #{"foo" "bar"}}
+                :block/properties-text-values {:alias "foo, bar"}}
+               (select-keys updated-page [:block/properties :block/properties-text-values]))
+            "Page property attributes are correct")
+        (is (= {:block/properties {:alias #{"foo" "bar"}}
+                :block/properties-text-values {:alias "foo, bar"}}
+               (-> (d/q '[:find (pull ?b [*])
+                          :where [?b :block/pre-block? true]]
+                        @conn)
+                   ffirst
+                   (select-keys [:block/properties :block/properties-text-values])))
+            "Pre-block property attributes are correct")))
+
+  (testing "save deletes orphaned pages when a block's refs change"
+    (let [conn (db/get-db test-helper/test-db false)
+          pages (set (map first (d/q '[:find ?bn :where [?b :block/name ?bn]] @conn)))
+          _ (assert (set/subset? #{"blarg" "bar"} pages) "Pages from block exist")
+          block-with-refs (ffirst (d/q '[:find (pull ?b [* {:block/refs [*]}])
+                                         :where [?b :block/content "block #blarg #bar"]]
+                                       @conn))
+          _ (save-block! (-> block-with-refs
+                             (assoc :block/content "block"
+                                    :block/refs [])))
+          updated-pages (set (map first (d/q '[:find ?bn :where [?b :block/name ?bn]] @conn)))]
+      (is (not (contains? updated-pages "blarg"))
+          "Deleted, orphaned page no longer exists")
+      (is (contains? updated-pages "bar")
+          "Deleted but not orphaned page still exists"))))
+
 ;;; Fuzzy tests
 
 (def init-id (atom 100))