Browse Source

fix: display more helpful notifications for invalid

properties and classes that can't build a db-ident. Part of LOG-3176
Gabriel Horner 1 year ago
parent
commit
4b365da70c

+ 6 - 4
deps/graph-parser/script/db_import.cljs

@@ -131,12 +131,14 @@
                         [(node-path/join (os/homedir) "logseq" "graphs") db-graph-dir])
         file-graph' (resolve-path file-graph)
         conn (outliner-cli/init-conn dir db-name {:classpath (cp/get-classpath)})
-        directory? (.isDirectory (fs/statSync file-graph'))]
+        directory? (.isDirectory (fs/statSync file-graph'))
+        ;; coerce option collection into strings
+        options' (if (:tag-classes options) (update options :tag-classes (partial mapv str)) options)]
     (p/do!
      (if directory?
-       (import-file-graph-to-db file-graph' (node-path/join dir db-name) conn (merge options {:graph-name db-name}))
-       (import-files-to-db file-graph' conn (merge options {:graph-name db-name})))
-     (when (:verbose options) (println "Transacted" (count (d/datoms @conn :eavt)) "datoms"))
+       (import-file-graph-to-db file-graph' (node-path/join dir db-name) conn (merge options' {:graph-name db-name}))
+       (import-files-to-db file-graph' conn (merge options' {:graph-name db-name})))
+     (when (:verbose options') (println "Transacted" (count (d/datoms @conn :eavt)) "datoms"))
      (println "Created graph" (str db-name "!")))))
 
 (when (= nbb/*file* (:file (meta #'-main)))

+ 17 - 5
deps/graph-parser/src/logseq/graph_parser/exporter.cljs

@@ -47,8 +47,14 @@
   [db class-name all-idents]
   (if-let [db-ident (get @all-idents (keyword class-name))]
     {:db/ident db-ident}
-    (let [m (db-class/build-new-class db {:block/original-name class-name
-                                          :block/name (common-util/page-name-sanity-lc class-name)})]
+    (let [m (try
+              (db-class/build-new-class db {:block/original-name class-name
+                                            :block/name (common-util/page-name-sanity-lc class-name)})
+              (catch :default e
+                ;; Notify with more helpful error message
+                (if (= :notification (:type (ex-data e)))
+                  (throw (ex-info (str "Class " (pr-str class-name) " is an invalid class name. Please rename it.") {}))
+                  (throw e))))]
       (swap! all-idents assoc (keyword class-name) (:db/ident m))
       m)))
 
@@ -235,9 +241,15 @@
     (some? non-ref-char)))
 
 (defn- create-property-ident [db all-idents property-name]
-  (let [db-ident (->> (db-property/create-user-property-ident-from-name (name property-name))
-                      ;; TODO: Detect new ident conflicts within same page
-                      (db-ident/ensure-unique-db-ident db))]
+  (let [db-ident (try
+                   (->> (db-property/create-user-property-ident-from-name (name property-name))
+                        ;; TODO: Detect new ident conflicts within same page
+                        (db-ident/ensure-unique-db-ident db))
+                   (catch :default e
+                     ;; Notify with more helpful error message
+                     (if (re-find #"name.*empty" (str (ex-message e)))
+                       (throw (ex-info (str "Property " (pr-str property-name) " is an invalid property name. Please rename it.") {}))
+                       (throw e))))]
     (swap! all-idents assoc property-name db-ident)))
 
 (defn- get-ident [all-idents kw]

+ 20 - 4
deps/graph-parser/test/logseq/graph_parser/exporter_test.cljs

@@ -99,8 +99,8 @@
   (let [*files (build-graph-files file-graph-dir)
         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 options
-                            default-export-options
+        options' (-> (merge default-export-options
+                            options
                             ;; asset file options
                             {:<copy-asset #(swap! assets conj %)})
                      (dissoc :assets))]
@@ -109,7 +109,7 @@
 (defn- import-files-to-db
   "Import specific doc files for dev purposes"
   [files conn options]
-  (p/let [doc-options (gp-exporter/build-doc-options {:macros {}} (merge options default-export-options))
+  (p/let [doc-options (gp-exporter/build-doc-options {:macros {}} (merge default-export-options options))
           files' (mapv #(hash-map :path %) files)
           _ (gp-exporter/export-doc-files conn files' <read-file doc-options)]
     {:import-state (:import-state doc-options)}))
@@ -159,7 +159,7 @@
 
       ;; Counts
       ;; Includes journals as property values e.g. :logseq.task/deadline
-      (is (= 16 (count (d/q '[:find ?b :where [?b :block/type "journal"]] @conn))))
+      (is (= 17 (count (d/q '[:find ?b :where [?b :block/type "journal"]] @conn))))
 
       ;; Don't count pages like url.md that have properties but no content
       (is (= 6
@@ -384,6 +384,22 @@
              (readable-properties @conn (find-page-by-name @conn "Interstellar")))
           "tagged page has configured tag imported as a class"))))
 
+(deftest-async export-files-with-invalid-class-and-property-display-notifications
+  (p/let [file-graph-dir "test/resources/exporter-test-graph"
+          files (mapv #(node-path/join file-graph-dir %) ["journals/2024_07_24.md" "ignored/invalid-property-page.md"])
+          conn (d/create-conn db-schema/schema-for-db-based-graph)
+          _ (d/transact! conn (sqlite-create-graph/build-db-initial-data "{}"))
+          notifications (atom [])
+          _ (import-files-to-db files conn {:tag-classes ["123"]
+                                            :notify-user #(swap! notifications conj %)})]
+
+    (is (= [:error :error] (map :level @notifications))
+        "Error notifications for both invalid property and class")
+    (is (some #(re-find #"invalid class" %) (map :msg @notifications))
+        "Helpful message for invalid class")
+    (is (some #(re-find #"invalid property" %) (map :msg @notifications))
+        "Helpful message for invalid property")))
+
 (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"])

+ 2 - 0
deps/graph-parser/test/resources/exporter-test-graph/ignored/invalid-property-page.md

@@ -0,0 +1,2 @@
+- b1
+123:: oops

+ 1 - 0
deps/graph-parser/test/resources/exporter-test-graph/journals/2024_07_24.md

@@ -0,0 +1 @@
+- invalid class example #123