(feat!)capture: change id creation to headline on entry-type capture-templates

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 2.3.2
+- [#2056](https://github.com/org-roam/org-roam/pull/2056) capture: IDs are now created for entries in capture templates
+
 ## 2.3.1 (2025-06-26)
 
 * (fix): Use correct type specifications to suppress warnings by @okomestudio in https://github.com/org-roam/org-roam/pull/2522

extensions/org-roam-dailies.el

@@ -339,11 +339,12 @@ In this case, interactive selection will be bypassed."
   (when goto (run-hooks 'org-roam-dailies-find-file-hook)))
 
 (add-hook 'org-roam-capture-preface-hook #'org-roam-dailies--override-capture-time-h)
+
 (defun org-roam-dailies--override-capture-time-h ()
   "Override the `:default-time' with the time from `:override-default-time'."
-  (prog1 nil
-    (when (org-roam-capture--get :override-default-time)
-      (org-capture-put :default-time (org-roam-capture--get :override-default-time)))))
+  (when (org-roam-capture--get :override-default-time)
+    (org-capture-put :default-time (org-roam-capture--get :override-default-time)))
+  nil)
 
 ;;; Bindings
 (defvar org-roam-dailies-map (make-sparse-keymap)

org-roam-capture.el

@@ -466,22 +466,21 @@ processing by `org-capture'.
 Note: During the capture process this function is run by
 `org-capture-set-target-location', as a (function ...) based
 capture target."
-  (let ((id (cond ((run-hook-with-args-until-success 'org-roam-capture-preface-hook))
-                  (t (org-roam-capture--setup-target-location)))))
-    (org-roam-capture--adjust-point-for-capture-type)
-    (let ((template (org-capture-get :template)))
-      (when (stringp template)
-        (org-capture-put
-         :template
-         (org-roam-capture--fill-template template))))
-    (org-roam-capture--put :id id)
-    (org-roam-capture--put :finalize (or (org-capture-get :finalize)
-                                         (org-roam-capture--get :finalize)))))
+  (if-let ((id (run-hook-with-args-until-success 'org-roam-capture-preface-hook)))
+      (org-roam-capture--put :id id)
+    (org-roam-capture--setup-target-location))
+  (let ((template (org-capture-get :template)))
+    (when (stringp template)
+      (org-capture-put
+       :template
+       (org-roam-capture--fill-template template))))
+  (org-roam-capture--put :finalize (or (org-capture-get :finalize)
+                                       (org-roam-capture--get :finalize))))
 
 (defun org-roam-capture--setup-target-location ()
-  "Initialize the buffer, and goto the location of the new capture.
-Return the ID of the location."
-  (let (p new-file-p)
+  "Initialize the buffer, and goto the location of the new capture."
+  (let ((target-entry-p t)
+        p new-file-p id)
     (pcase (org-roam-capture--get-target)
       (`(file ,path)
        (setq path (org-roam-capture--target-truepath path)
@@ -489,7 +488,8 @@ Return the ID of the location."
        (when new-file-p (org-roam-capture--put :new-file path))
        (set-buffer (org-capture-target-buffer path))
        (widen)
-       (setq p (goto-char (point-min))))
+       (setq p (goto-char (point-min))
+             target-entry-p nil))
       (`(file+olp ,path ,olp)
        (setq path (org-roam-capture--target-truepath path)
              new-file-p (org-roam-capture--new-file-p path))
@@ -507,7 +507,8 @@ Return the ID of the location."
          (org-roam-capture--put :new-file path)
          (insert (org-roam-capture--fill-template head 'ensure-newline)))
        (widen)
-       (setq p (goto-char (point-min))))
+       (setq p (goto-char (point-min))
+             target-entry-p nil))
       (`(file+head+olp ,path ,head ,olp)
        (setq path (org-roam-capture--target-truepath path)
              new-file-p (org-roam-capture--new-file-p path))
@@ -569,17 +570,45 @@ Return the ID of the location."
                        (user-error "No node with title or id \"%s\"" title-or-id))))
          (set-buffer (org-capture-target-buffer (org-roam-node-file node)))
          (goto-char (org-roam-node-point node))
-         (setq p (org-roam-node-point node)))))
+         (setq p (org-roam-node-point node)
+               target-entry-p (and (derived-mode-p 'org-mode) (org-at-heading-p))))))
     ;; Setup `org-id' for the current capture target and return it back to the
     ;; caller.
-    (save-excursion
-      (goto-char p)
-      (if-let ((id (org-entry-get p "ID")))
-          (setf (org-roam-node-id org-roam-capture--node) id)
-        (org-entry-put p "ID" (org-roam-node-id org-roam-capture--node)))
-      (prog1
-          (org-id-get)
-        (run-hooks 'org-roam-capture-new-node-hook)))))
+    ;; Unless it's an entry type, then we want to create an ID for the entry instead
+    (pcase (org-capture-get :type)
+      ('entry
+       (advice-add #'org-capture-place-entry :after #'org-roam-capture--create-id-for-entry)
+       (org-roam-capture--put :new-node-p t)
+       (setq id (org-roam-node-id org-roam-capture--node)))
+      (_
+       (save-excursion
+         (goto-char p)
+         (unless (org-entry-get p "ID")
+           (org-roam-capture--put :new-node-p t))
+         (setq id (or (org-entry-get p "ID")
+                      (org-roam-node-id org-roam-capture--node)))
+         (setf (org-roam-node-id org-roam-capture--node) id)
+         (org-entry-put p "ID" id))))
+    (org-roam-capture--put :id id)
+    (org-roam-capture--put :target-entry-p target-entry-p)
+    (advice-add #'org-capture-place-template :before #'org-roam-capture--set-target-entry-p-a)
+    (advice-add #'org-capture-place-template :after #'org-roam-capture-run-new-node-hook-a)))
+
+(defun org-roam-capture--set-target-entry-p-a (_)
+  "Correct `:target-entry-p' in Org-capture template based on `:target.'."
+  (org-capture-put :target-entry-p (org-roam-capture--get :target-entry-p))
+  (advice-remove #'org-capture-place-template #'org-roam-capture--set-target-entry-p-a))
+
+(defun org-roam-capture-run-new-node-hook-a (_)
+  "Advice to run after the Org-capture template is placed."
+  (when (org-roam-capture--get :new-node-p)
+    (run-hooks 'org-roam-capture-new-node-hook))
+  (advice-remove #'org-capture-place-template #'org-roam-capture--place-template-a))
+
+(defun org-roam-capture--create-id-for-entry ()
+  "Create the ID for the new entry."
+  (org-entry-put (point) "ID" (org-roam-capture--get :id))
+  (advice-remove #'org-capture-place-entry #'org-roam-capture--create-id-for-entry))
 
 (defun org-roam-capture--get-target ()
   "Get the current capture :target for the capture template in use."

tests/test-org-roam-capture.el

@@ -58,6 +58,193 @@
      (org-roam-capture--fill-template (lambda () "foo"))
      :to-equal "foo")))
 
+(describe "org-roam-capture entry-type ID creation"
+  (it "creates ID for entry-type captures"
+    (let* ((temp-dir (make-temp-file "org-roam-test" t))
+           (test-file (expand-file-name "test.org" temp-dir))
+           (org-roam-directory temp-dir)
+           (org-roam-capture--node (org-roam-node-create :id (org-id-new)))
+           (org-roam-capture--info (make-hash-table :test 'equal))
+           capture-id)
+      (unwind-protect
+          (progn
+            ;; Create initial file content
+            (with-temp-file test-file
+              (insert "#+TITLE: Test File\n\n* Parent Heading\n:PROPERTIES:\n:ID: parent-id\n:END:\n"))
+
+            ;; Mock org-capture context and get-target
+            (cl-letf* (((symbol-function 'org-capture-get)
+                        (lambda (prop)
+                          (pcase prop
+                            (:type 'entry)
+                            (_ nil))))
+                       ((symbol-function 'org-roam-capture--get-target)
+                        (lambda () `(file ,test-file))))
+
+              ;; Call the setup function
+              (with-current-buffer (find-file-noselect test-file)
+                (org-roam-capture--setup-target-location)
+                (setq capture-id (org-roam-capture--get :id)))
+
+              ;; Verify ID was created and stored for entry type
+              (expect capture-id :not :to-be nil)
+              (expect (org-roam-capture--get :new-node-p) :to-be t)))
+        (delete-directory temp-dir t))))
+
+  (it "creates ID at target for non-entry-type captures"
+    (let* ((temp-dir (make-temp-file "org-roam-test" t))
+           (test-file (expand-file-name "test-plain.org" temp-dir))
+           (org-roam-directory temp-dir)
+           (org-roam-capture--node (org-roam-node-create :id (org-id-new)))
+           (org-roam-capture--info (make-hash-table :test 'equal))
+           capture-id)
+      (unwind-protect
+          (progn
+            ;; Create initial empty file
+            (with-temp-file test-file
+              (insert "#+TITLE: Test\n"))
+
+            ;; Mock org-capture context for plain type
+            (cl-letf* (((symbol-function 'org-capture-get)
+                        (lambda (prop)
+                          (pcase prop
+                            (:type 'plain)
+                            (_ nil))))
+                       ((symbol-function 'org-roam-capture--get-target)
+                        (lambda () `(file ,test-file))))
+
+              ;; Call the setup function
+              (with-current-buffer (find-file-noselect test-file)
+                (org-roam-capture--setup-target-location)
+                (setq capture-id (org-roam-capture--get :id)))
+
+              ;; For non-entry types, ID should be created at the target location
+              (expect capture-id :not :to-be nil)
+              (expect (org-roam-capture--get :new-node-p) :to-be t)))
+        (delete-directory temp-dir t)))))
+
+(describe "org-roam-capture advice functions"
+  :var ((org-roam-capture--info))
+
+  (before-each
+    (setq org-roam-capture--info (make-hash-table :test 'equal)))
+
+  (it "org-roam-capture--create-id-for-entry creates and removes advice"
+    (cl-letf* ((entry-id nil)
+               ((symbol-function 'org-entry-put)
+                (lambda (pom prop val)
+                  (when (string= prop "ID")
+                    (setq entry-id val))))
+               ((symbol-function 'org-roam-capture--get)
+                (lambda (prop)
+                  (if (eq prop :id)
+                      "test-id-123"
+                    nil))))
+
+      ;; Add the advice
+      (advice-add #'org-capture-place-entry :after #'org-roam-capture--create-id-for-entry)
+
+      ;; Call the function
+      (org-roam-capture--create-id-for-entry)
+
+      ;; Verify ID was set and advice removed
+      (expect entry-id :to-equal "test-id-123")
+      (expect (advice-member-p #'org-roam-capture--create-id-for-entry
+                              #'org-capture-place-entry)
+              :to-be nil)))
+
+  (it "org-roam-capture--set-target-entry-p-a sets and removes advice"
+    (cl-letf* ((captured-value nil)
+               ((symbol-function 'org-capture-put)
+                (lambda (prop val)
+                  (when (eq prop :target-entry-p)
+                    (setq captured-value val))))
+               ((symbol-function 'org-roam-capture--get)
+                (lambda (prop)
+                  (if (eq prop :target-entry-p)
+                      t
+                    nil))))
+
+      ;; Add the advice
+      (advice-add #'org-capture-place-template :before #'org-roam-capture--set-target-entry-p-a)
+
+      ;; Call the function
+      (org-roam-capture--set-target-entry-p-a nil)
+
+      ;; Verify value was set and advice removed
+      (expect captured-value :to-be t)
+      (expect (advice-member-p #'org-roam-capture--set-target-entry-p-a
+                              #'org-capture-place-template)
+              :to-be nil)))
+
+  (it "org-roam-capture-run-new-node-hook-a runs hook when new node"
+    (let ((hook-ran nil))
+      (cl-letf* (((symbol-function 'org-roam-capture--get)
+                  (lambda (prop)
+                    (if (eq prop :new-node-p)
+                        t
+                      nil))))
+
+        ;; Add test hook
+        (add-hook 'org-roam-capture-new-node-hook
+                  (lambda () (setq hook-ran t)))
+
+        ;; Add the advice
+        (advice-add #'org-capture-place-template :after #'org-roam-capture-run-new-node-hook-a)
+
+        ;; Call the function
+        (org-roam-capture-run-new-node-hook-a nil)
+
+        ;; Verify hook ran
+        (expect hook-ran :to-be t)
+
+        ;; Clean up
+        (remove-hook 'org-roam-capture-new-node-hook
+                     (lambda () (setq hook-ran t)))))))
+
+(describe "org-roam-capture target-entry-p detection"
+  (it "detects entry target for file+olp"
+    (let* ((temp-dir (make-temp-file "org-roam-test" t))
+           (test-file (expand-file-name "test-olp.org" temp-dir))
+           (org-roam-directory temp-dir)
+           (org-roam-capture--node (org-roam-node-create :id (org-id-new)))
+           (org-roam-capture--info (make-hash-table :test 'equal)))
+      (unwind-protect
+          (progn
+            (with-temp-file test-file
+              (insert "* Level 1\n** Level 2\n"))
+
+            (cl-letf* (((symbol-function 'org-roam-capture--get-target)
+                        (lambda () `(file+olp ,test-file ("Level 1" "Level 2"))))
+                       ((symbol-function 'org-capture-get)
+                        (lambda (prop) nil)))
+
+              (with-current-buffer (find-file-noselect test-file)
+                (org-roam-capture--setup-target-location)
+                (expect (org-roam-capture--get :target-entry-p) :to-be t))))
+        (delete-directory temp-dir t))))
+
+  (it "detects non-entry target for file"
+    (let* ((temp-dir (make-temp-file "org-roam-test" t))
+           (test-file (expand-file-name "test-file.org" temp-dir))
+           (org-roam-directory temp-dir)
+           (org-roam-capture--node (org-roam-node-create :id (org-id-new)))
+           (org-roam-capture--info (make-hash-table :test 'equal)))
+      (unwind-protect
+          (progn
+            (with-temp-file test-file
+              (insert "#+TITLE: Test\n"))
+
+            (cl-letf* (((symbol-function 'org-roam-capture--get-target)
+                        (lambda () `(file ,test-file)))
+                       ((symbol-function 'org-capture-get)
+                        (lambda (prop) nil)))
+
+              (with-current-buffer (find-file-noselect test-file)
+                (org-roam-capture--setup-target-location)
+                (expect (org-roam-capture--get :target-entry-p) :to-be nil))))
+        (delete-directory temp-dir t)))))
+
 (provide 'test-org-roam-capture)
 
 ;;; test-org-roam-capture.el ends here