From ee448fd5a11487788c3eb58a9857668d82d930b7 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 16 Oct 2026 10:56:53 +0200 Subject: [PATCH] :bug: 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 | 65 +++++---- .../backend_tests/rpc_management_test.clj | 138 ++++++++++++++++++ 2 files changed, 177 insertions(+), 26 deletions(-) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index b0ac390b57..87e0714170 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/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] @@ -276,20 +277,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}] @@ -297,9 +312,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 @@ -326,8 +339,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. @@ -368,17 +380,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 @@ -406,8 +416,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)) diff --git a/backend/test/backend_tests/rpc_management_test.clj b/backend/test/backend_tests/rpc_management_test.clj index a55dcf95a4..9f20edd3d3 100644 --- a/backend/test/backend_tests/rpc_management_test.clj +++ b/backend/test/backend_tests/rpc_management_test.clj @@ -639,6 +639,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