fix(cli): handle inline block properties atomically

2026-05-29 15:09:41 +00:00 · 2026-05-22 22:32:47 +08:00
parent 8c71f2cacf
commit f5d94ec993
3 changed files with 111 additions and 12 deletions
--- a/cli-e2e/spec/non_sync_cases.edn
+++ b/cli-e2e/spec/non_sync_cases.edn
@@ -795,6 +795,20 @@ PY"
     :show ["--page"]}},
   :tags [:upsert :show],
   :extends :non-sync/graph-json-env}
+  {:id "block-upsert-blocks-inline-property-missing-does-not-create-target-page-json",
+   :cmds
+   ["set +e; out=\"$({{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert block --graph {{graph-arg}} --target-page NewPage --blocks '[{:block/title \"Q\" \"Missing Prop\" \"x\"}]' 2>&1)\"; code=$?; set -e; printf '%s\n' \"$out\"; test \"$code\" -ne 0; printf '%s' \"$out\" | python3 -c 'import sys,json; data=json.load(sys.stdin); assert data[\"status\"] == \"error\"; assert data[\"error\"][\"code\"] == \"property-not-found\"'; page=\"$({{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json query --graph {{graph-arg}} --query '[:find ?e . :where [?e :block/title \"NewPage\"]]')\"; printf '%s\n' \"$page\"; printf '%s' \"$page\" | python3 -c 'import sys,json; data=json.load(sys.stdin); assert data[\"data\"][\"result\"] is None'"],
+   :expect
+   {:exit 0,
+    :stdout-contains ["property-not-found"]},
+   :covers
+   {:commands ["upsert block" "query"],
+    :options
+    {:global ["--config" "--graph" "--root-dir" "--output"],
+     :upsert ["--blocks" "--target-page"],
+     :query ["--query"]}},
+   :tags [:upsert],
+   :extends :non-sync/graph-json-env}
  {:id "block-upsert-blocks-inline-properties-per-block-json",
   :setup
   ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert page --graph {{graph-arg}} --page Home >/dev/null"
@@ -814,6 +828,25 @@ PY"
     :show ["--id"]}},
   :tags [:upsert :show],
   :extends :non-sync/graph-json-env}
+  {:id "block-upsert-blocks-inline-property-numeric-id-json",
+   :setup
+   ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert page --graph {{graph-arg}} --page Home >/dev/null"
+    "{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert property --graph {{graph-arg}} --name 'Numeric Prop' --type default --cardinality one --public true >/dev/null"],
+   :cmds
+   ["prop_id=\"$({{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json query --graph {{graph-arg}} --query '[:find ?e . :where [?e :block/title \"Numeric Prop\"]]' | python3 -c 'import sys,json; print(json.load(sys.stdin)[\"data\"][\"result\"])')\"; {{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert block --graph {{graph-arg}} --target-page Home --blocks \"[{:block/title \\\"Inline numeric\\\" $prop_id \\\"inline\\\"}]\" >/dev/null"
+    "{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output human show --graph {{graph-arg}} --page Home"],
+   :expect
+   {:exit 0,
+    :stdout-contains ["Inline numeric" "Numeric Prop: inline"]},
+   :covers
+   {:commands ["upsert block" "upsert property" "query" "show"],
+    :options
+    {:global ["--config" "--graph" "--root-dir" "--output"],
+     :upsert ["--blocks" "--target-page" "--name" "--type" "--cardinality" "--public"],
+     :query ["--query"],
+     :show ["--page"]}},
+   :tags [:upsert :show],
+   :extends :non-sync/graph-json-env}
  {:id "block-upsert-target-uuid-json",
   :setup
   ["{{cli}} --root-dir {{root-dir-arg}} --config {{config-path-arg}} --output json upsert page --graph {{graph-arg}} --page Home >/dev/null"
--- a/src/main/logseq/cli/command/add.cljs
+++ b/src/main/logseq/cli/command/add.cljs
@@ -192,10 +192,16 @@
    (string? k)
    (seq (string/trim k))

+    (or (number? k) (uuid? k))
+    true
+
    (qualified-keyword? k)
    (and (inline-property-key-namespace? (namespace k))
         (public-built-in-property-key? k))

+    (keyword? k)
+    true
+
    :else
    false))

@@ -1252,22 +1258,22 @@
 (defn execute-add-block
  [action config]
  (-> (p/let [cfg (cli-server/ensure-server! config (:repo action))
-              target-block-uuid (resolve-add-target cfg action)
              action-blocks (normalize-block-content-keys (:blocks action))
-              ref-values (collect-page-refs action-blocks)
+              {blocks-without-inline-properties :blocks
+               inline-property-assignments :property-assignments} (extract-inline-properties action-blocks)
+              inline-property-assignments (resolve-inline-property-assignments cfg (:repo action)
+                                                                               inline-property-assignments)
+              target-block-uuid (resolve-add-target cfg action)
+              ref-values (collect-page-refs blocks-without-inline-properties)
              {:keys [uuid-refs page-refs id-refs]} (partition-ref-values ref-values)
              _ (ensure-block-refs-exist! cfg (:repo action) uuid-refs)
              page-refs' (or (resolve-page-ref-entities cfg (:repo action) page-refs) [])
              id-refs' (or (resolve-id-ref-entities cfg (:repo action) id-refs) [])
              refs (into page-refs' id-refs')
              blocks (if (seq refs)
-                       (normalize-block-title-refs action-blocks refs)
-                       action-blocks)
-              {blocks-without-inline-properties :blocks
-               inline-property-assignments :property-assignments} (extract-inline-properties blocks)
-              inline-property-assignments (resolve-inline-property-assignments cfg (:repo action)
-                                                                               inline-property-assignments)
-              blocks-for-insert (flatten-block-tree blocks-without-inline-properties)
+                       (normalize-block-title-refs blocks-without-inline-properties refs)
+                       blocks-without-inline-properties)
+              blocks-for-insert (flatten-block-tree blocks)
              status (:status action)
              tags (if (contains? action :resolved-tags)
                     (:resolved-tags action)
--- a/src/test/logseq/cli/command/add_test.cljs
+++ b/src/test/logseq/cli/command/add_test.cljs
@@ -128,8 +128,9 @@
             (:property-assignments result))))))

 (deftest test-extract-inline-properties-keeps-non-property-attrs
-  (testing "only property namespace keywords and string property names are extracted"
+  (testing "property selectors are extracted without treating normal block attrs as properties"
    (let [block-uuid (uuid "00000000-0000-0000-0000-000000000103")
+          property-uuid (uuid "00000000-0000-0000-0000-000000000104")
          result (#'add-command/extract-inline-properties
                  [{:block/title "Block"
                    :block/uuid block-uuid
@@ -137,7 +138,10 @@
                    :build/keep-uuid? true
                    :plugin/option "kept"
                    :logseq.property.asset/type "png"
-                    :plugin.property._test_plugin/rating "5"}])]
+                    :plugin.property._test_plugin/rating "5"
+                    801 "by id"
+                    property-uuid "by uuid"
+                    :plain-title "by keyword"}])]
      (is (= [{:block/title "Block"
               :block/uuid block-uuid
               :db/id 101
@@ -146,7 +150,10 @@
               :logseq.property.asset/type "png"}]
             (:blocks result)))
      (is (= [{:block-uuid block-uuid
-               :properties {:plugin.property._test_plugin/rating "5"}}]
+               :properties {:plugin.property._test_plugin/rating "5"
+                            801 "by id"
+                            property-uuid "by uuid"
+                            :plain-title "by keyword"}}]
             (:property-assignments result))))))

 (def ^:private mock-transport-invoke
@@ -442,3 +449,56 @@
               (p/catch (fn [e]
                          (is false (str "unexpected error: " e))))
               (p/finally done)))))
+
+(deftest test-execute-add-block-does-not-create-target-page-when-inline-property-resolution-fails
+  (async done
+         (let [created-page-uuid (uuid "00000000-0000-0000-0000-000000000203")
+               ops* (atom [])]
+           (-> (p/with-redefs [cli-server/ensure-server! (fn [_ _] {:base-url "http://example"})
+                               add-command/resolve-properties
+                               (fn [_ _ properties & _]
+                                 (if (= {"Missing Prop" "x"} properties)
+                                   (p/rejected (ex-info "property not found: \"Missing Prop\""
+                                                        {:code :property-not-found
+                                                         :property "Missing Prop"}))
+                                   (p/resolved properties)))
+                               transport/invoke
+                               (fn [_ method args]
+                                 (case method
+                                   :thread-api/q
+                                   (p/resolved [])
+
+                                   :thread-api/pull
+                                   (let [[_ _ lookup] args]
+                                     (p/resolved
+                                      (if (= lookup [:block/name "newpage"])
+                                        {:db/id 900
+                                         :block/uuid created-page-uuid
+                                         :block/name "newpage"
+                                         :block/title "NewPage"}
+                                        {})))
+
+                                   :thread-api/apply-outliner-ops
+                                   (let [[_ ops _] args]
+                                     (swap! ops* conj ops)
+                                     (p/resolved {:result :ok}))
+
+                                   (p/rejected (ex-info "unexpected invoke" {:method method :args args}))))]
+                 (-> (add-command/execute-add-block
+                      {:type :add-block
+                       :repo "demo"
+                       :target-page-name "NewPage"
+                       :pos "last-child"
+                       :blocks [{:block/title "Q"
+                                 :block/uuid (uuid "00000000-0000-0000-0000-000000000204")
+                                 "Missing Prop" "x"}]}
+                      {})
+                     (p/then (fn [_]
+                               (is false "expected inline property resolution error")))
+                     (p/catch (fn [e]
+                                (is (= :property-not-found (-> e ex-data :code)))
+                                (is (empty? @ops*)
+                                    "target page creation must not run before inline properties validate")))))
+               (p/catch (fn [e]
+                          (is false (str "unexpected error: " e))))
+               (p/finally done)))))