Browse Source

fix: importer failing and invalid on two page to class conversion cases

Failed for edge case when page converts to class via -p or -P. Page to
class conversions sometimes resulted in invalid :block/tags. Also
cleaned up last of class or page tx that were embedded in db attributes
Gabriel Horner 9 months ago
parent
commit
b8c569f1c4

+ 20 - 13
deps/graph-parser/src/logseq/graph_parser/exporter.cljs

@@ -731,7 +731,7 @@
 
 (defn- handle-page-properties
   "Adds page properties including special handling for :logseq.property/parent"
-  [{:block/keys [properties] :as block*} db {:keys [page-names-to-uuids]} refs
+  [{:block/keys [properties] :as block*} db {:keys [page-names-to-uuids classes-tx]} refs
    {:keys [user-options log-fn import-state] :as options}]
   (let [{:keys [block properties-tx]} (handle-page-and-block-properties block* db page-names-to-uuids refs options)
         block'
@@ -740,18 +740,18 @@
                                                      distinct
                                                      seq)]
           (let [_ (swap! (:classes-from-property-parents import-state) conj (:block/title block*))
-                ;; TODO: Mv new classes from these find-or-create-class to :classes-tx as they are the only ones
-                ;; that aren't conrolled by :classes-tx
                 class-m (find-or-create-class db ((some-fn ::original-title :block/title) block) (:all-idents import-state) block)
                 class-m' (-> block
                              (merge class-m)
                              (assoc :logseq.property/parent
                                     (let [new-class (first parent-classes-from-properties)
-                                          class-m (find-or-create-class db new-class (:all-idents import-state))]
+                                          class-m (find-or-create-class db new-class (:all-idents import-state))
+                                          class-m' (merge class-m
+                                                          {:block/uuid (find-or-gen-class-uuid page-names-to-uuids (common-util/page-name-sanity-lc new-class) (:db/ident class-m))})]
                                       (when (> (count parent-classes-from-properties) 1)
                                         (log-fn :skipped-parent-classes "Only one parent class is allowed so skipped ones after the first one" :classes parent-classes-from-properties))
-                                      (merge class-m
-                                             {:block/uuid (find-or-gen-class-uuid page-names-to-uuids (common-util/page-name-sanity-lc new-class) (:db/ident class-m))}))))]
+                                      (when (:new-class? (meta class-m)) (swap! classes-tx conj class-m'))
+                                      [:block/uuid (:block/uuid class-m')])))]
             class-m')
           block)
         block'' (replace-namespace-with-parent block' page-names-to-uuids)]
@@ -1267,7 +1267,7 @@
 (defn- clean-extra-invalid-tags
   "If a page/class tx is an existing property or a new or existing class, ensure that
   it only has one tag by removing :logseq.class/Page from its tx"
-  [db pages-tx' classes-tx]
+  [db pages-tx' classes-tx existing-pages]
   ;; TODO: Improve perf if we tracked all created classes in atom
   (let [existing-classes (->> (d/datoms db :avet :block/tags :logseq.class/Tag)
                               (map #(d/entity db (:e %)))
@@ -1278,7 +1278,13 @@
         existing-properties (->> (d/datoms db :avet :block/tags :logseq.class/Property)
                                  (map #(d/entity db (:e %)))
                                  (map :block/uuid)
-                                 set)]
+                                 set)
+        existing-pages' (set/map-invert existing-pages)
+        retract-page-tag-from-existing-pages
+        (->> pages-tx'
+             ;; Existing pages that have converted to property or class
+             (filter #(and (:db/ident %) (get existing-pages' (:block/uuid %))))
+             (mapv #(vector :db/retract [:block/uuid (:block/uuid %)] :block/tags :logseq.class/Page)))]
     {:pages-tx
      (mapv (fn [page]
              (if (or (contains? classes (:block/uuid page))
@@ -1286,9 +1292,10 @@
                (update page :block/tags (fn [tags] (vec (remove #(= % :logseq.class/Page) tags))))
                page))
            pages-tx')
-     :retract-page-tag-from-classes-tx
-     (mapv #(vector :db/retract [:block/uuid (:block/uuid %)] :block/tags :logseq.class/Page)
-           classes-tx)}))
+     :retract-page-tags-tx
+     (into (mapv #(vector :db/retract [:block/uuid (:block/uuid %)] :block/tags :logseq.class/Page)
+                 classes-tx)
+           retract-page-tag-from-existing-pages)}))
 
 (defn add-file-to-db-graph
   "Parse file and save parsed data to the given db graph. Options available:
@@ -1331,8 +1338,8 @@
         main-props-tx-report (d/transact! conn property-pages-tx {::new-graph? true})
 
         classes-tx @(:classes-tx tx-options)
-        {:keys [retract-page-tag-from-classes-tx] pages-tx'' :pages-tx} (clean-extra-invalid-tags @conn pages-tx' classes-tx)
-        classes-tx' (concat classes-tx retract-page-tag-from-classes-tx)
+        {:keys [retract-page-tags-tx] pages-tx'' :pages-tx} (clean-extra-invalid-tags @conn pages-tx' classes-tx existing-pages)
+        classes-tx' (concat classes-tx retract-page-tags-tx)
         ;; Build indices
         pages-index (->> (map #(select-keys % [:block/uuid]) pages-tx'')
                          (concat (map #(select-keys % [:block/uuid]) classes-tx))

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

@@ -195,7 +195,7 @@
                   #_(map #(select-keys % [:block/title :block/tags]))
                   count))
           "Correct number of pages with block content")
-      (is (= 11 (->> @conn
+      (is (= 12 (->> @conn
                      (d/q '[:find [?ident ...]
                             :where [?b :block/tags :logseq.class/Tag] [?b :db/ident ?ident] (not [?b :logseq.property/built-in?])])
                      count))
@@ -659,7 +659,8 @@
 
 (deftest-async export-files-with-property-parent-classes-option
   (p/let [file-graph-dir "test/resources/exporter-test-graph"
-          files (mapv #(node-path/join file-graph-dir %) ["pages/CreativeWork.md" "pages/Movie.md" "pages/type.md"
+          files (mapv #(node-path/join file-graph-dir %) ["journals/2024_11_26.md"
+                                                          "pages/CreativeWork.md" "pages/Movie.md" "pages/type.md"
                                                           "pages/Whiteboard___Tool.md" "pages/Whiteboard___Arrow_head_toggle.md"
                                                           "pages/Property.md" "pages/url.md"])
           conn (db-test/create-conn)
@@ -670,7 +671,7 @@
     (is (empty? (map :entity (:errors (db-validate/validate-db! @conn))))
         "Created graph has no validation errors")
 
-    (is (= #{:user.class/Movie :user.class/CreativeWork :user.class/Thing
+    (is (= #{:user.class/Movie :user.class/CreativeWork :user.class/Thing :user.class/Feature
              :user.class/Class :user.class/Tool :user.class/Whiteboard___Tool :user.class/Property}
            (->> @conn
                 (d/q '[:find [?ident ...]

+ 2 - 1
deps/graph-parser/test/resources/exporter-test-graph/journals/2024_11_26.md

@@ -6,4 +6,5 @@
   card-ease-factor:: 2.36
   card-last-reviewed:: 2024-11-26T20:27:33.373Z
 - card 2 with cloze {{cloze surprise!}} #card
-  card-next-schedule:: 2024-11-27T05:00:00.000Z
+  card-next-schedule:: 2024-11-27T05:00:00.000Z
+- This block references a page with a future parent class #Property