forge--update-{issue,pullreq}: Always try to save people and labels

When a topic is fetched individually, i.e., outside a request that also
fetches data about the repository itself, then it can happen that the
topic data contains references to new users and labels, which do not
exist in the local database yet.  Because we only record the object id
for users and labels in topic tables as foreign keys, that can lead to
foreign key errors.

In the past we did not even attempt to update these values when fetching
individual topics.  That was an okay approach, because the expectation
was that users would always fetch everything.  Support for only fetching
individual topics was only added later, and was only intended to be used
in special cases.  This is changing (but still work in progress).

More importantly though, when we modify a topic, we now follow that up
by only fetching that topic from the forge again.  Previously we would
have fetched the complete repository, which was slow and unnecessary,
because at that time all we care about is making sure that one topic is
in sync.

In that case we must update all slots.  If the user modified, e.g., the
labels, then they expect that to take effect locally, without having to
explicitly fetch the complete repository.  We can even be sure that the
chosen labels are known locally already, else the user would not have
been able to select them.

When fetching an individual topic explicitly, it can still happen that
it references an unknown label or person, but that is the exception.
We now silently ignore these rare cases, which is better than to not
even attempting to store this data for the vast majority of cases for
which it would have worked.

Closes #654.

Author: tarsius

lisp/forge-github.el

@@ -251,15 +251,16 @@
 
 (cl-defmethod forge--update-issue ((repo forge-github-repository) data
                                    &optional bump initial-pull)
-  (closql-with-transaction (forge-db)
+  (let (issue-id issue)
     (let-alist data
-      (let* ((issue-id (forge--object-id 'forge-issue repo .number))
-             (issue (or (forge-get-issue repo .number)
+      (closql-with-transaction (forge-db)
+        (setq issue-id (forge--object-id 'forge-issue repo .number))
+        (setq issue (or (forge-get-issue repo .number)
                         (closql-insert
                          (forge-db)
                          (forge-issue :id         issue-id
                                       :repository (oref repo id)
-                                      :number     .number)))))
+                                      :number     .number))))
         (oset issue their-id   .id)
         (oset issue slug       (format "#%s" .number))
         (oset issue state
@@ -291,12 +292,13 @@
               :updated .updatedAt
               :body    (forge--sanitize-string .body))
              t)))
-        (forge--update-status repo issue data bump initial-pull)
-        (when bump
-          (forge--set-id-slot repo issue 'assignees .assignees)
-          (unless (magit-get-boolean "forge.kludge-for-issue-294")
-            (forge--set-id-slot repo issue 'labels .labels)))
-        issue))))
+        (forge--update-status repo issue data bump initial-pull))
+      (ignore-errors
+        (forge--set-id-slot repo issue 'assignees .assignees))
+      (ignore-errors
+        (unless (magit-get-boolean "forge.kludge-for-issue-294")
+          (forge--set-id-slot repo issue 'labels .labels))))
+    issue))
 
 ;;;; Pullreqs
 
@@ -308,15 +310,16 @@
 
 (cl-defmethod forge--update-pullreq ((repo forge-github-repository) data
                                      &optional bump initial-pull)
-  (closql-with-transaction (forge-db)
+  (let (pullreq-id pullreq)
     (let-alist data
-      (let* ((pullreq-id (forge--object-id 'forge-pullreq repo .number))
-             (pullreq (or (forge-get-pullreq repo .number)
+      (closql-with-transaction (forge-db)
+        (setq pullreq-id (forge--object-id 'forge-pullreq repo .number))
+        (setq pullreq (or (forge-get-pullreq repo .number)
                           (closql-insert
                            (forge-db)
                            (forge-pullreq :id         pullreq-id
                                           :repository (oref repo id)
-                                          :number     .number)))))
+                                          :number     .number))))
         (oset pullreq their-id     .id)
         (oset pullreq slug         (format "#%s" .number))
         (oset pullreq state        (pcase-exhaustive .state
@@ -357,15 +360,17 @@
               :updated .updatedAt
               :body    (forge--sanitize-string .body))
              t)))
-        (forge--update-status repo pullreq data bump initial-pull)
-        (when bump
-          (forge--set-id-slot repo pullreq 'assignees .assignees)
-          (forge--set-id-slot repo pullreq 'review-requests
-                              (--map (cdr (cadr (car it)))
-                                     .reviewRequests))
-          (unless (magit-get-boolean "forge.kludge-for-issue-294")
-            (forge--set-id-slot repo pullreq 'labels .labels)))
-        pullreq))))
+        (forge--update-status repo pullreq data bump initial-pull))
+      (ignore-errors
+        (forge--set-id-slot repo pullreq 'assignees .assignees))
+      (ignore-errors
+        (forge--set-id-slot repo pullreq 'review-requests
+                            (--map (cdr (cadr (car it)))
+                                   .reviewRequests)))
+      (ignore-errors
+        (unless (magit-get-boolean "forge.kludge-for-issue-294")
+          (forge--set-id-slot repo pullreq 'labels .labels))))
+    pullreq))
 
 ;;;; Notifications