fix: page references with same name in one block (#12017)

* fix: page references with the same name in one block

* fix: empty :block/name for Library pages

* fix: invalid call to uuid-string?

trace suddenly showing up in multiple CLI calls

---------

Co-authored-by: Gabriel Horner <[email protected]>

deps/common/src/logseq/common/util.cljs

@@ -250,7 +250,7 @@
   {:malli/schema [:=> [:cat :any :string] [:or :nil :string [:vector [:maybe :string]]]]}
   [pattern s]
   (when-not (string? s)
-       ;; TODO: sentry
+    ;; TODO: sentry
     (js/console.trace))
   (when (string? s)
     (re-find pattern s)))

deps/db/src/logseq/db/common/entity_plus.cljc

@@ -99,7 +99,9 @@
              ;; replace block id refs if there're cycle references of blocks
                refs (:block/refs e)
                result' (if (and (string? result) refs)
-                         (db-content/id-ref->title-ref result refs)
+                         (db-content/id-ref->title-ref result refs
+                                                       {:db db
+                                                        :replace-pages-with-same-name? false})
                          result)]
            (or result' default-value))
          (lookup-entity e k default-value))))))
@@ -160,14 +162,8 @@
            :block.temp/property-keys
            (get-property-keys e)
 
-           ;; cache :block/title
            :block/title
-           (or
-            (:block.temp/cached-title @(.-cache e))
-            (let [title (get-block-title e k default-value)]
-              (vreset! (.-cache e) (assoc @(.-cache e)
-                                          :block.temp/cached-title title))
-              title))
+           (get-block-title e k default-value)
 
            :block/_parent
            (->> (lookup-entity e k default-value)
@@ -188,12 +184,8 @@
 
 (defn- cache-with-kv
   [^js this]
-  (let [v @(.-cache this)
-        v' (if (:block/title v)
-             (assoc v :block/title
-                    (db-content/id-ref->title-ref (:block/title v) (:block/refs this)))
-             v)]
-    (concat (seq v')
+  (let [v @(.-cache this)]
+    (concat (seq v)
             (seq (.-kv this)))))
 
 #?(:org.babashka/nbb

deps/db/src/logseq/db/common/entity_util.cljs

@@ -16,4 +16,4 @@
 (defn page?
   [entity]
   (or (entity-util/page? entity)
-      (file-entity-util/page? entity)))
+      (file-entity-util/page? entity)))

deps/db/src/logseq/db/common/initial_data.cljs

@@ -14,17 +14,13 @@
             [logseq.db.frontend.entity-util :as entity-util]
             [logseq.db.frontend.rules :as rules]))
 
-(defn- get-pages-by-name
-  [db page-name]
-  (d/datoms db :avet :block/name (common-util/page-name-sanity-lc page-name)))
-
 ;; FIXME: For DB graph built-in pages, look up by name -> uuid like
 ;; get-built-in-page instead of this approach which is more error prone
 (defn get-first-page-by-name
   "Return the oldest page's db id for :block/name"
   [db page-name]
   (when (and db (string? page-name))
-    (first (sort (map :e (get-pages-by-name db page-name))))))
+    (first (sort (map :e (entity-util/get-pages-by-name db page-name))))))
 
 (defn get-first-page-by-title
   "Return the oldest page's db id for :block/title"

deps/db/src/logseq/db/frontend/content.cljs

@@ -4,7 +4,8 @@
             [datascript.core :as d]
             [logseq.common.util :as common-util]
             [logseq.common.util.page-ref :as page-ref]
-            [logseq.db.common.entity-util :as common-entity-util]))
+            [logseq.db.common.entity-util :as common-entity-util]
+            [logseq.db.frontend.entity-util :as entity-util]))
 
 ;; [[uuid]]
 (def id-ref-pattern
@@ -40,14 +41,18 @@
 
 (defn id-ref->title-ref
   "Convert id ref backs to page name refs using refs."
-  [content* refs* & {:keys [replace-block-id?]
-                     :or {replace-block-id? false}}]
+  [content* refs* & {:keys [db replace-block-id? replace-pages-with-same-name?]
+                     :or {replace-block-id? false
+                          replace-pages-with-same-name? true}}]
   (when (string? content*)
     (let [refs (if replace-block-id?
-               ;; The caller need to handle situations including
-               ;; mutual references and circle references.
+                 ;; The caller need to handle situations including
+                 ;; mutual references and circle references.
                  refs*
-                 (filter common-entity-util/page? refs*))
+                 (cond->> (filter common-entity-util/page? refs*)
+                   (and db (false? replace-pages-with-same-name?))
+                   (remove (fn [e]
+                             (> (count (entity-util/get-pages-by-name db (:block/title e))) 1)))))
           content (str content*)]
       (if (re-find id-ref-pattern content)
         (reduce
@@ -107,15 +112,20 @@
   [title refs & {:keys [replace-tag?]
                  :or {replace-tag? true}}]
   (assert (string? title))
-  (let [refs' (->>
-               (map
-                (fn [ref]
-                  (if (and (vector? ref) (= :block/uuid (first ref)))
-                    {:block/uuid (second ref)
-                     :block/title (str (first ref))}
-                    ref))
-                refs)
-               sort-refs)]
+  (let [refs' (->> refs
+                   (remove (fn [ref]
+                             ;; remove uuid references since they're introduced to detect multiple pages
+                             ;; that have the same name
+                             (and (map? ref)
+                                  (:block.temp/original-page-name ref)
+                                  (common-util/uuid-string? (:block.temp/original-page-name ref)))))
+                   (map
+                    (fn [ref]
+                      (if (and (vector? ref) (= :block/uuid (first ref)))
+                        {:block/uuid (second ref)
+                         :block/title (str (first ref))}
+                        ref)))
+                   sort-refs)]
     (reduce
      (fn [content {uuid' :block/uuid :block/keys [title] :as block}]
        (let [title' (or (:block.temp/original-page-name block) title)]

deps/db/src/logseq/db/frontend/entity_util.cljs

@@ -1,8 +1,10 @@
 (ns logseq.db.frontend.entity-util
   "Lower level entity util fns for DB graphs"
   (:require [clojure.string :as string]
+            [datascript.core :as d]
             [datascript.db]
-            [datascript.impl.entity :as de])
+            [datascript.impl.entity :as de]
+            [logseq.common.util :as common-util])
   (:refer-clojure :exclude [object?]))
 
 (defn- has-tag?
@@ -86,3 +88,7 @@
   "Built-in page or block"
   [entity]
   (:logseq.property/built-in? entity))
+
+(defn get-pages-by-name
+  [db page-name]
+  (d/datoms db :avet :block/name (common-util/page-name-sanity-lc page-name)))

deps/db/src/logseq/db/frontend/schema.cljs

@@ -37,7 +37,7 @@
          (map (juxt :major :minor)
               [(parse-schema-version x) (parse-schema-version y)])))
 
-(def version (parse-schema-version "65.7"))
+(def version (parse-schema-version "65.8"))
 
 (defn major-version
   "Return a number.

deps/outliner/src/logseq/outliner/core.cljs

@@ -273,7 +273,7 @@
                                    (not= block-title (:block/title block-entity)))
           _ (when (and db-based? page? block-title)
               (outliner-validate/validate-page-title-characters block-title {:node m*}))
-          m* (if (and db-based? page-title-changed? (not (and page? (:block/parent block-entity))))
+          m* (if (and db-based? page-title-changed?)
                (let [_ (outliner-validate/validate-page-title (:block/title m*) {:node m*})
                      page-name (common-util/page-name-sanity-lc (:block/title m*))]
                  (assoc m* :block/name page-name))

src/main/frontend/db/model.cljs

@@ -99,8 +99,7 @@ independent of format as format specific heading characters are stripped"
                 (fn content-matches? [block-content external-content block-id]
                   (let [block (db-utils/entity repo block-id)
                         ref-tags (distinct (concat (:block/tags block) (:block/refs block)))]
-                    (= (-> block-content
-                           (db-content/id-ref->title-ref ref-tags)
+                    (= (-> (db-content/id-ref->title-ref block-content ref-tags)
                            (db-content/content-id-ref->page ref-tags)
                            heading-content->route-name)
                        (string/lower-case external-content))))

src/main/frontend/handler/page.cljs

@@ -261,8 +261,9 @@
   (fn [chosen-result e]
     (util/stop e)
     (state/clear-editor-action!)
-    (p/let [_ (when-let [id (:block/uuid chosen-result)]
-                (db-async/<get-block (state/get-current-repo) id {:children? false}))
+    (p/let [repo (state/get-current-repo)
+            _ (when-let [id (:block/uuid chosen-result)]
+                (db-async/<get-block repo id {:children? false}))
             chosen-result (if (:block/uuid chosen-result)
                             (db/entity [:block/uuid (:block/uuid chosen-result)])
                             chosen-result)
@@ -278,7 +279,10 @@
                                                   page (date/js-date->journal-title gd)]
                                               [page (db/get-page page)])))
                                         [chosen' chosen-result])
-            ref-text (if (and (de/entity? chosen-result) (not (ldb/page? chosen-result)))
+            datoms (state/<invoke-db-worker :thread-api/datoms repo :avet :block/name (util/page-name-sanity-lc chosen'))
+            multiple-pages-same-name? (> (count datoms) 1)
+            ref-text (if (and (de/entity? chosen-result)
+                              (or multiple-pages-same-name? (not (ldb/page? chosen-result))))
                        (ref/->page-ref (:block/uuid chosen-result))
                        (get-page-ref-text chosen'))
             result (when db-based?

src/main/frontend/worker/db/migrate.cljs

@@ -343,6 +343,17 @@
                      sqlite-create-graph/mark-block-as-built-in))]
     [page]))
 
+(defn- add-missing-page-name
+  [db]
+  (let [pages (d/datoms db :avet :block/name "")]
+    (keep
+     (fn [d]
+       (let [page (d/entity db (:e d))]
+         (when-not (string/blank? (:block/title page))
+           {:db/id (:db/id page)
+            :block/name (common-util/page-name-sanity-lc (:block/title page))})))
+     pages)))
+
 (def schema-version->updates
   "A vec of tuples defining datascript migrations. Each tuple consists of the
    schema version integer and a migration map. A migration map can have keys of :properties, :classes
@@ -354,7 +365,8 @@
    ["65.4" {:fix fix-using-properties-as-tags}]
    ["65.5" {:fix remove-block-order-for-tags}]
    ["65.6" {:fix update-extends-to-cardinality-many}]
-   ["65.7" {:fix add-quick-add-page}]])
+   ["65.7" {:fix add-quick-add-page}]
+   ["65.8" {:fix add-missing-page-name}]])
 
 (let [[major minor] (last (sort (map (comp (juxt :major :minor) db-schema/parse-schema-version first)
                                      schema-version->updates)))]

src/main/frontend/worker/db_worker.cljs

@@ -412,6 +412,12 @@
   (when-let [conn (worker-state/get-datascript-conn repo)]
     (apply d/q (first inputs) @conn (rest inputs))))
 
+(def-thread-api :thread-api/datoms
+  [repo & args]
+  (when-let [conn (worker-state/get-datascript-conn repo)]
+    (let [result (apply d/datoms @conn args)]
+      (map (fn [d] [(:e d) (:a d) (:v d) (:tx d) (:added d)]) result))))
+
 (def-thread-api :thread-api/pull
   [repo selector id]
   (when-let [conn (worker-state/get-datascript-conn repo)]

src/test/frontend/worker/commands_test.cljs

@@ -71,7 +71,7 @@
       (let [next-time (get-next-time now month-unit 1)]
         (is (= 1 (in-months next-time))))
       (let [next-time (get-next-time one-month-ago month-unit 1)]
-        (is (= 1 (in-months next-time))))
+        (is (> (in-days next-time) 1)))
       (let [next-time (get-next-time one-month-ago month-unit 3)]
         (is (= 2 (in-months next-time))))
       (let [next-time (get-next-time one-month-ago month-unit 5)]
@@ -117,6 +117,6 @@
       (let [next-time (get-next-time (t/minus now (t/weeks 10)) week-unit 1)]
         (is (= 1 (in-weeks next-time))))
       (let [next-time (get-next-time (t/minus now (t/months 10)) month-unit 1)]
-        (is (= 1 (in-months next-time))))
+        (is (> (in-days next-time) 1)))
       (let [next-time (get-next-time (t/minus now (t/years 10)) year-unit 1)]
         (is (= 1 (in-years next-time)))))))