fix: db-import of property-classes option fails on docs graph

Fixed by ensuring that tag names and namespace names don't conflict
when making class idents. The specific bug was a conflict between
Tool class and Whiteboard/Tool. Interestingly the bug didn't appear
if the Tool's description property wasn't present.

Also fixed some importer tests not parsing namespaces correctly
or taking :verbose option

deps/graph-parser/src/logseq/graph_parser/exporter.cljs

@@ -62,17 +62,26 @@
                {:block/uuid (get-page-uuid page-names-to-uuids (get-in block [:block/namespace :block/name]))}))
     block))
 
+(defn- build-class-ident-name
+  [class-name]
+  (string/replace class-name "/" "___"))
+
 (defn- find-or-create-class
   ([db class-name all-idents]
    (find-or-create-class db class-name all-idents {}))
   ([db class-name all-idents class-block]
    (if-let [db-ident (get @all-idents (keyword class-name))]
      {:db/ident db-ident}
-     (let [m* (cond-> {:block/title class-name
-                       :block/name (common-util/page-name-sanity-lc class-name)}
-                (:block/namespace class-block)
-                (build-new-namespace-page))
-           m (db-class/build-new-class db m*)]
+     (let [m
+           (if (:block/namespace class-block)
+             ;; Give namespaced tags a unique ident so they don't conflict with other tags
+             (-> (db-class/build-new-class db {:block/title (build-class-ident-name class-name)})
+                 (merge {:block/title class-name
+                         :block/name (common-util/page-name-sanity-lc class-name)})
+                 (build-new-namespace-page))
+             (db-class/build-new-class db
+                                       {:block/title class-name
+                                        :block/name (common-util/page-name-sanity-lc class-name)}))]
        (swap! all-idents assoc (keyword class-name) (:db/ident m))
        (with-meta m {:new-class? true})))))
 
@@ -950,7 +959,8 @@
       ((fn [block']
          (merge (build-new-namespace-page block')
                 {;; save original name b/c it's still used for a few name lookups
-                 ::original-name (:block/name block')}))))))
+                 ::original-name (:block/name block')
+                 ::original-title (:block/title block')}))))))
 
 (defn- build-pages-tx
   "Given all the pages and blocks parsed from a file, return a map containing
@@ -976,18 +986,26 @@
                         :classes-tx (:classes-tx options)}
         all-pages-m (mapv #(handle-page-properties % @conn per-file-state all-pages options)
                           all-pages)
-        pages-tx (keep (fn [m]
-                         (if-let [page-uuid (if (::original-name m)
-                                              (all-existing-page-uuids (::original-name m))
-                                              (all-existing-page-uuids (:block/name m)))]
-                           (build-existing-page (dissoc m ::original-name) @conn page-uuid per-file-state options)
-                           (when (or (= "class" (:block/type m))
-                                     ;; Don't build a new page if it overwrites an existing class
-                                     (not (some-> (get @(:all-idents import-state) (keyword (:block/title m)))
-                                                  db-malli-schema/class?)))
-                             (build-new-page-or-class (dissoc m ::original-name)
-                                                      @conn per-file-state (:all-idents import-state) options))))
-                       (map :block all-pages-m))]
+        pages-tx (keep (fn [{m :block _properties-tx :properties-tx}]
+                         (let [page (if-let [page-uuid (if (::original-name m)
+                                                         (all-existing-page-uuids (::original-name m))
+                                                         (all-existing-page-uuids (:block/name m)))]
+                                      (build-existing-page (dissoc m ::original-name ::original-title) @conn page-uuid per-file-state options)
+                                      (when (or (= "class" (:block/type m))
+                                                ;; Don't build a new page if it overwrites an existing class
+                                                (not (some-> (get @(:all-idents import-state)
+                                                                  (some-> (or (::original-title m) (:block/title m))
+                                                                          build-class-ident-name
+                                                                          keyword))
+                                                             db-malli-schema/class?))
+                                                ;; TODO: Enable this when it's valid for all test graphs because
+                                                ;; pages with properties must be built or else properties-tx is invalid
+                                                #_(seq properties-tx))
+                                        (build-new-page-or-class (dissoc m ::original-name ::original-title)
+                                                                 @conn per-file-state (:all-idents import-state) options)))]
+                           ;;  (when-not ret (println "Skipped page tx for" (pr-str (:block/title m))))
+                           page))
+                       all-pages-m)]
     {:pages-tx pages-tx
      :page-properties-tx (mapcat :properties-tx all-pages-m)
      :existing-pages (select-keys all-existing-page-uuids (map :block/name all-pages*))

deps/graph-parser/test/logseq/graph_parser/exporter_test.cljs

@@ -119,18 +119,21 @@
         config-file (first (filter #(string/ends-with? (:path %) "logseq/config.edn") *files))
         _ (assert config-file "No 'logseq/config.edn' found for file graph dir")
         options' (merge default-export-options
-                        {:user-options (dissoc options :assets)
+                        {:user-options (dissoc options :assets :verbose)
                         ;; asset file options
-                         :<copy-asset #(swap! assets conj %)})]
+                         :<copy-asset #(swap! assets conj %)}
+                        (select-keys options [:verbose]))]
     (gp-exporter/export-file-graph conn conn config-file *files options')))
 
 (defn- import-files-to-db
   "Import specific doc files for dev purposes"
   [files conn options]
   (reset! gp-block/*export-to-db-graph? true)
-  (-> (p/let [doc-options (gp-exporter/build-doc-options (merge {:macros {}} (:user-config options))
+  (-> (p/let [doc-options (gp-exporter/build-doc-options (merge {:macros {} :file/name-format :triple-lowbar}
+                                                                (:user-config options))
                                                          (merge default-export-options
-                                                                {:user-options (dissoc options :user-config)}))
+                                                                {:user-options (dissoc options :user-config :verbose)}
+                                                                (select-keys options [:verbose])))
               files' (mapv #(hash-map :path %) files)
               _ (gp-exporter/export-doc-files conn files' <read-file doc-options)]
         {:import-state (:import-state doc-options)})
@@ -190,7 +193,7 @@
       (is (= 3 (count (d/q '[:find ?b :where [?b :block/tags :logseq.class/Query]] @conn))))
 
       ;; Don't count pages like url.md that have properties but no content
-      (is (= 8
+      (is (= 9
              (count (->> (d/q '[:find [(pull ?b [:block/title :block/type]) ...]
                                 :where [?b :block/title] [_ :block/page ?b] (not [?b :logseq.property/built-in?])] @conn)
                          (filter ldb/internal-page?))))
@@ -491,6 +494,10 @@
                    count))
         "Correct number of user classes")
 
+    (is (= :user.class/Quotes___life
+           (:db/ident (find-page-by-name @conn "life")))
+        "Namespaced tag's ident has hierarchy to make it unique")
+
     (is (= [{:block/type "class"}]
            (d/q '[:find [(pull ?b [:block/type]) ...] :where [?b :block/name "life"]] @conn))
         "When a class is used and referenced on the same page, there should only be one instance of it")
@@ -529,7 +536,9 @@
 
 (deftest-async export-files-with-property-classes-option
   (p/let [file-graph-dir "test/resources/exporter-test-graph"
-          files (mapv #(node-path/join file-graph-dir %) ["journals/2024_02_23.md" "pages/url.md"])
+          files (mapv #(node-path/join file-graph-dir %)
+                      ["journals/2024_02_23.md" "pages/url.md" "pages/Whiteboard___Tool.md"
+                       "pages/Whiteboard___Arrow_head_toggle.md"])
           conn (db-test/create-conn)
           _ (import-files-to-db files conn {:property-classes ["type"]})
           _ (@#'gp-exporter/export-class-properties conn conn)]
@@ -537,7 +546,7 @@
     (is (empty? (map :entity (:errors (db-validate/validate-db! @conn))))
         "Created graph has no validation errors")
 
-    (is (= #{:user.class/Property :user.class/Movie}
+    (is (= #{:user.class/Property :user.class/Movie :user.class/Class :user.class/Tool}
            (->> @conn
                 (d/q '[:find [?ident ...]
                        :where [?b :block/type "class"] [?b :db/ident ?ident] (not [?b :logseq.property/built-in?])])

deps/graph-parser/test/resources/exporter-test-graph/pages/Whiteboard___Arrow_head_toggle.md

@@ -0,0 +1 @@
+type:: [[Tool]]

deps/graph-parser/test/resources/exporter-test-graph/pages/Whiteboard___Tool.md

@@ -0,0 +1,4 @@
+alias:: Whiteboard tool, Tool, Tools
+type:: [[Class]]
+parent:: [[Feature]]
+description:: Tools on the [[Whiteboard/Toolbar]]