🐛 Absorb the libraries a move to another team leaves behind

Moving files or a project to another team unlinked every library that
ended up in another team than the file using it, but absorbed nothing:
the copies kept pointing at a library their file could no longer
reach. A move now does what deleting the library does (PENPOT-1540):
it absorbs each such library into the files using it, then unlinks
them; as on deletion, a failed absorption is logged and does not stop
the move. That covers a moved library and the files left behind using
it, a moved file and the libraries it used, and a moved project.

The links checked are those that start or end at a moved file. They
were those of the files in the source and destination projects, so a
file in another project of the old team kept its link to a moved
library across teams.

rpc-management-test: a moved library is absorbed into a file in its
project and into one in a project the move does not touch; a moved
file absorbs its library; a moved project absorbs the library it left
behind.

AI-assisted-by: claude-opus-5-5

backend/src/app/rpc/commands/management.clj

@@ -12,6 +12,7 @@
    [app.binfile.v3 :as bf.v3]
    [app.common.exceptions :as ex]
    [app.common.features :as cfeat]
+   [app.common.logging :as l]
    [app.common.schema :as sm]
    [app.common.time :as ct]
    [app.common.uuid :as uuid]
@@ -259,20 +260,34 @@
 (def sql:move-files
   "update file set project_id = ? where id = ANY(?)")
 
-(def sql:delete-broken-relations
-  "with broken as (
-     (select * from file_library_rel as flr
-       inner join file as f on (flr.file_id = f.id)
-       inner join project as p on (f.project_id = p.id)
-       inner join file as lf on (flr.library_file_id = lf.id)
-       inner join project as lp on (lf.project_id = lp.id)
-       where p.id = ANY(?)
-         and lp.team_id != p.team_id)
-   )
-   delete from file_library_rel as rel
-    using broken as br
-    where rel.file_id = br.file_id
-      and rel.library_file_id = br.library_file_id")
+(def ^:private sql:get-relations-across-teams
+  "SELECT flr.file_id, flr.library_file_id
+     FROM file_library_rel AS flr
+     JOIN file AS f ON (f.id = flr.file_id)
+     JOIN project AS p ON (p.id = f.project_id)
+     JOIN file AS lf ON (lf.id = flr.library_file_id)
+     JOIN project AS lp ON (lp.id = lf.project_id)
+    WHERE (flr.file_id = ANY(?) OR flr.library_file_id = ANY(?))
+      AND lp.team_id != p.team_id")
+
+(defn- absorb-libraries-across-teams!
+  "Absorb each library linked to or from the files `file-ids` that is now
+  in another team than the file using it into that file, and unlink them,
+  as deleting the library would. Run after the files move to another team."
+  [{:keys [::db/conn] :as cfg} file-ids]
+  (let [ids  (db/create-array conn "uuid" file-ids)
+        rels (db/exec! conn [sql:get-relations-across-teams ids ids])]
+    (doseq [[library-id rels] (group-by :library-file-id rels)]
+      ;; as on deletion, a failed absorption does not stop the move
+      (try
+        (db/tx-run! cfg files/absorb-library library-id (mapv :file-id rels))
+        (catch Throwable cause
+          (l/warn :hint "error on absorbing library"
+                  :library-id (str library-id)
+                  :cause cause))))
+    (doseq [{:keys [file-id library-file-id]} rels]
+      (db/delete! conn :file-library-rel {:file-id file-id
+                                          :library-file-id library-file-id}))))
 
 (defn move-files
   [{:keys [::db/conn] :as cfg} {:keys [profile-id ids project-id] :as params}]
@@ -280,9 +295,7 @@
   (let [fids    (db/create-array conn "uuid" ids)
         files   (->> (db/exec! conn [sql:get-files fids])
                      (map files/decode-row))
-        source  (into #{} (map :project-id) files)
-        pids    (->> (conj source project-id)
-                     (db/create-array conn "uuid"))]
+        source  (into #{} (map :project-id) files)]
 
     (when (contains? source project-id)
       (ex/raise :type :validation
@@ -309,8 +322,7 @@
     ;; move all files to the project
     (db/exec-one! conn [sql:move-files project-id fids])
 
-    ;; delete possible broken relations on moved files
-    (db/exec-one! conn [sql:delete-broken-relations pids])
+    (absorb-libraries-across-teams! cfg ids)
 
     ;; Update the modification date of the all affected projects
     ;; ensuring that the destination project is the most recent one.
@@ -351,17 +363,15 @@
 
   The operation also:
   - Updates the project's team assignment
-  - Cleans up any broken library relations after the move
+  - Absorbs the libraries now in another team than the files using them
+    into those files, and unlinks them
 
   Throws:
   - :cant-move-to-same-team if trying to move project to its current team
   - Permission exceptions if user lacks required permissions
   - Team compatibility exceptions if teams are incompatible"
   [{:keys [::db/conn] :as cfg} {:keys [profile-id team-id project-id] :as params}]
-  (let [project (db/get-by-id conn :project project-id {:columns [:id :team-id]})
-        pids    (->> (db/query conn :project {:team-id (:team-id project)} {:columns [:id]})
-                     (map :id)
-                     (db/create-array conn "uuid"))]
+  (let [project (db/get-by-id conn :project project-id {:columns [:id :team-id]})]
 
     (when (= team-id (:team-id project))
       (ex/raise :type :validation
@@ -389,8 +399,11 @@
                 {:team-id team-id}
                 {:id project-id})
 
-    ;; delete possible broken relations on moved files
-    (db/exec-one! conn [sql:delete-broken-relations pids])
+    (absorb-libraries-across-teams! cfg (->> (db/query conn :file
+                                                       {:project-id project-id}
+                                                       {:columns [:id]
+                                                        ::db/remove-deleted false})
+                                             (map :id)))
 
     nil))
 

backend/test/backend_tests/rpc_management_test.clj

@@ -637,6 +637,144 @@
         (t/is (= (:file-id item1) (:id file1)))
         (t/is (= (:library-file-id item1) (:id file2)))))))
 
+(defn- add-library-copy!
+  "Add a component to the shared `library` and a copy of it to `file`,
+  linked to it. Returns the ids `check-absorbed!` needs."
+  [profile library file]
+  (let [c-id    (uuid/random)
+        main-id (uuid/random)
+        inst-id (uuid/random)
+        l-page  (first (get-in library [:data :pages]))
+        f-page  (first (get-in file [:data :pages]))]
+    (th/link-file-to-library* {:file-id (:id file)
+                               :library-id (:id library)})
+    (update-file!
+     :file-id (:id library)
+     :profile-id (:id profile)
+     :changes
+     [{:type :add-obj
+       :page-id l-page
+       :id main-id
+       :parent-id uuid/zero
+       :frame-id uuid/zero
+       :obj (cts/setup-shape
+             {:id main-id
+              :name "Board"
+              :frame-id uuid/zero
+              :parent-id uuid/zero
+              :type :frame
+              :main-instance true
+              :component-root true
+              :component-file (:id library)
+              :component-id c-id})}
+      {:type :add-component
+       :path ""
+       :name "Board"
+       :main-instance-id main-id
+       :main-instance-page l-page
+       :id c-id
+       :anotation nil}])
+    (update-file!
+     :file-id (:id file)
+     :profile-id (:id profile)
+     :changes
+     [{:type :add-obj
+       :page-id f-page
+       :id inst-id
+       :parent-id uuid/zero
+       :frame-id uuid/zero
+       :obj (cts/setup-shape
+             {:id inst-id
+              :name "Board"
+              :frame-id uuid/zero
+              :parent-id uuid/zero
+              :type :frame
+              :main-instance false
+              :component-root true
+              :component-file (:id library)
+              :component-id c-id})}])
+    {:profile profile :file file :c-id c-id :inst-id inst-id :f-page f-page}))
+
+(defn- check-absorbed!
+  "Check that the file of `copy` holds the library component, with its copy
+  pointing at it, and is linked to no library."
+  [{:keys [profile file c-id inst-id f-page]}]
+  (let [out (th/command! {::th/type :get-file
+                          ::rpc/profile-id (:id profile)
+                          :id (:id file)})]
+    (t/is (th/success? out))
+    (t/is (some? (get-in out [:result :data :components c-id])))
+    (t/is (= (:id file)
+             (get-in out [:result :data :pages-index f-page :objects inst-id :component-file])))
+    (t/is (empty? (th/db-query :file-library-rel {:file-id (:id file)})))))
+
+(defn- move-files!
+  [profile project file]
+  (let [out (th/command! {::th/type :move-files
+                          ::rpc/profile-id (:id profile)
+                          :project-id (:id project)
+                          :ids #{(:id file)}})]
+    (t/is (nil? (:error out)))))
+
+(t/deftest move-library-to-other-team-absorbs-it-into-the-files-using-it
+  (let [profile  (th/create-profile* 1 {:is-active true})
+        team     (th/create-team* 1 {:profile-id (:id profile)})
+        project1 (th/create-project* 1 {:team-id (:default-team-id profile)
+                                        :profile-id (:id profile)})
+        project2 (th/create-project* 2 {:team-id (:id team)
+                                        :profile-id (:id profile)})
+        project3 (th/create-project* 3 {:team-id (:default-team-id profile)
+                                        :profile-id (:id profile)})
+        library  (th/create-file* 1 {:profile-id (:id profile)
+                                     :project-id (:id project1)
+                                     :is-shared true})
+        file1    (th/create-file* 2 {:profile-id (:id profile)
+                                     :project-id (:id project1)})
+        ;; in a project the move does not touch
+        file3    (th/create-file* 3 {:profile-id (:id profile)
+                                     :project-id (:id project3)})
+        copy1    (add-library-copy! profile library file1)
+        copy3    (add-library-copy! profile library file3)]
+    (move-files! profile project2 library)
+    (check-absorbed! copy1)
+    (check-absorbed! copy3)))
+
+(t/deftest move-file-to-other-team-absorbs-its-libraries
+  (let [profile  (th/create-profile* 1 {:is-active true})
+        team     (th/create-team* 1 {:profile-id (:id profile)})
+        project1 (th/create-project* 1 {:team-id (:default-team-id profile)
+                                        :profile-id (:id profile)})
+        project2 (th/create-project* 2 {:team-id (:id team)
+                                        :profile-id (:id profile)})
+        library  (th/create-file* 1 {:profile-id (:id profile)
+                                     :project-id (:id project1)
+                                     :is-shared true})
+        file     (th/create-file* 2 {:profile-id (:id profile)
+                                     :project-id (:id project1)})
+        copy     (add-library-copy! profile library file)]
+    (move-files! profile project2 file)
+    (check-absorbed! copy)))
+
+(t/deftest move-project-to-other-team-absorbs-the-libraries-left-behind
+  (let [profile  (th/create-profile* 1 {:is-active true})
+        team     (th/create-team* 1 {:profile-id (:id profile)})
+        project1 (th/create-project* 1 {:team-id (:default-team-id profile)
+                                        :profile-id (:id profile)})
+        project2 (th/create-project* 2 {:team-id (:default-team-id profile)
+                                        :profile-id (:id profile)})
+        library  (th/create-file* 1 {:profile-id (:id profile)
+                                     :project-id (:id project2)
+                                     :is-shared true})
+        file     (th/create-file* 2 {:profile-id (:id profile)
+                                     :project-id (:id project1)})
+        copy     (add-library-copy! profile library file)
+        out      (th/command! {::th/type :move-project
+                               ::rpc/profile-id (:id profile)
+                               :project-id (:id project1)
+                               :team-id (:id team)})]
+    (t/is (nil? (:error out)))
+    (check-absorbed! copy)))
+
 (t/deftest clone-template
   (let [prof    (th/create-profile* 1 {:is-active true})
         data    {::th/type :clone-template