From 87384aaccda61147085973f159874412434dc1bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Valderrama?= Date: Mon, 25 May 2026 13:07:45 +0200 Subject: [PATCH] :bug: Fix nitrate delete and leave org flow --- backend/src/app/rpc/commands/nitrate.clj | 116 ++++++++++++++---- backend/src/app/rpc/commands/profile.clj | 7 +- backend/src/app/rpc/commands/teams.clj | 8 +- backend/src/app/rpc/management/nitrate.clj | 77 +++++++----- backend/src/app/rpc/notifications.clj | 5 +- .../rpc_management_nitrate_test.clj | 41 ++++--- .../test/backend_tests/rpc_nitrate_test.clj | 65 ++++++++++ .../app/common/types/nitrate_permissions.cljc | 10 +- .../types/nitrate_permissions_test.cljc | 17 +-- frontend/src/app/main/data/dashboard.cljs | 7 +- frontend/src/app/main/data/nitrate.cljs | 43 +++++++ .../src/app/main/ui/dashboard/sidebar.cljs | 59 +++------ 12 files changed, 325 insertions(+), 130 deletions(-) diff --git a/backend/src/app/rpc/commands/nitrate.clj b/backend/src/app/rpc/commands/nitrate.clj index c0cd31c35b..bd00d52359 100644 --- a/backend/src/app/rpc/commands/nitrate.clj +++ b/backend/src/app/rpc/commands/nitrate.clj @@ -112,12 +112,48 @@ AND t.id = ANY(?) AND t.deleted_at IS NULL") -(def sql:get-team-files-count - "SELECT count(*) AS total +(def ^:private sql:get-teams-files-counts + "SELECT p.team_id, count(*) AS total FROM file AS f JOIN project AS p ON (p.id = f.project_id) - WHERE p.team_id = ? - AND f.deleted_at IS NULL") + WHERE p.team_id = ANY(?) + AND f.deleted_at IS NULL + GROUP BY p.team_id") + +(defn- get-team-files-counts + [conn team-ids] + (if (seq team-ids) + (let [ids-array (db/create-array conn "uuid" team-ids)] + (->> (db/exec! conn [sql:get-teams-files-counts ids-array]) + (reduce (fn [acc {:keys [team-id total]}] + (assoc acc team-id (long total))) + {}))) + {})) + +(defn- build-leave-org-plan + [{:keys [::db/conn]} default-team-id teams-to-delete keep-default-team-requested?] + (let [all-teams (cond-> (set teams-to-delete) default-team-id (conj default-team-id)) + files-counts (get-team-files-counts conn all-teams) + has-files? (fn [id] (pos? (long (get files-counts id 0)))) + deletable (remove has-files? teams-to-delete) + keep-default? (or keep-default-team-requested? + (and default-team-id (has-files? default-team-id))) + to-detach (cond-> (into [] (remove (set deletable) teams-to-delete)) + (and default-team-id keep-default?) (conj default-team-id))] + {:deletable-team-ids deletable + :keep-default-team? keep-default? + :delete-default-team? (boolean (and default-team-id (not keep-default?))) + :detach-from-org-team-ids to-detach})) + +(defn get-leave-org-summary + [cfg default-team-id teams-to-delete teams-to-transfer-count teams-to-exit-count] + (let [{:keys [deletable-team-ids delete-default-team? detach-from-org-team-ids]} + (build-leave-org-plan cfg default-team-id teams-to-delete nil)] + {:teams-to-delete (+ (count deletable-team-ids) + (if delete-default-team? 1 0)) + :teams-to-transfer teams-to-transfer-count + :teams-to-exit teams-to-exit-count + :teams-to-detach (count detach-from-org-team-ids)})) (def ^:private schema:leave-org [:map @@ -132,6 +168,18 @@ [:id ::sm/uuid] [:reassign-to {:optional true} ::sm/uuid]]]]]) +(def ^:private schema:get-leave-org-summary-result + [:map + [:teams-to-delete ::sm/int] + [:teams-to-transfer ::sm/int] + [:teams-to-exit ::sm/int] + [:teams-to-detach ::sm/int]]) + +(def ^:private schema:get-leave-org-summary + [:map + [:id ::sm/uuid] + [:default-team-id ::sm/uuid]]) + (defn- get-organization-teams-for-user [{:keys [::db/conn] :as cfg} org-summary profile-id] @@ -221,16 +269,14 @@ :code :not-valid-teams)))) + (defn leave-org - [{:keys [::db/conn] :as cfg} {:keys [profile-id id name default-team-id teams-to-delete teams-to-leave skip-validation] :as params}] - (let [org-prefix (str "[" (d/sanitize-string name) "] ") - - default-team-files-count (-> (db/exec-one! conn [sql:get-team-files-count default-team-id]) - :total) - delete-default-team? (= default-team-files-count 0)] - - - + [{:keys [::db/conn] :as cfg} + {:keys [profile-id id name default-team-id teams-to-delete teams-to-leave skip-validation keep-default-team-requested?]}] + (let [org-prefix (str "[" (d/sanitize-string name) "] ") + {:keys [deletable-team-ids + keep-default-team? + detach-from-org-team-ids]} (build-leave-org-plan cfg default-team-id teams-to-delete keep-default-team-requested?)] ;; assert that the received teams are valid, checking the different constraints (when-not skip-validation @@ -238,20 +284,27 @@ (assert-membership cfg profile-id id) - ;; delete the teams-to-delete - (doseq [id teams-to-delete] - (teams/delete-team cfg {:profile-id profile-id :team-id id})) + ;; delete only eligible teams (non-protected and without files) + (doseq [id deletable-team-ids] + (teams/delete-team cfg {:profile-id profile-id + :team-id id})) ;; leave the teams-to-leave (doseq [{:keys [id reassign-to]} teams-to-leave] (teams/leave-team cfg {:profile-id profile-id :id id :reassign-to reassign-to})) - ;; Delete default-team-id if empty; otherwise keep it and prefix the name. - (if delete-default-team? - (do - (db/update! conn :team {:is-default false} {:id default-team-id}) - (teams/delete-team cfg {:profile-id profile-id :team-id default-team-id})) - (db/exec! conn [sql:prefix-team-name-and-unset-default org-prefix default-team-id])) + ;; Process org "Your Penpot" team: keep with prefix if needed, otherwise delete. + (when default-team-id + (if keep-default-team? + (db/exec! conn [sql:prefix-team-name-and-unset-default org-prefix default-team-id]) + (teams/delete-team cfg {:profile-id profile-id + :team-id default-team-id}))) + + ;; Detach retained owned teams from the organization in Nitrate. + ;; Nitrate will rehome them to its fallback/default org. + (doseq [team-id detach-from-org-team-ids] + (nitrate/call cfg :remove-team-from-org {:team-id team-id + :organization-id id})) ;; Api call to nitrate (nitrate/call cfg :remove-profile-from-org {:profile-id profile-id :organization-id id}) @@ -268,6 +321,25 @@ (leave-org cfg (assoc params :profile-id profile-id))) +(sv/defmethod ::get-leave-org-summary + {::rpc/auth true + ::doc/added "2.18" + ::sm/params schema:get-leave-org-summary + ::sm/result schema:get-leave-org-summary-result + ::db/transaction true} + [cfg {:keys [::rpc/profile-id id default-team-id]}] + (let [{:keys [valid-teams-to-delete-ids + valid-teams-to-transfer + valid-teams-to-exit + valid-default-team]} (get-valid-teams cfg id profile-id default-team-id) + teams-to-transfer-count (count valid-teams-to-transfer) + teams-to-exit-count (count valid-teams-to-exit)] + (when-not valid-default-team + (ex/raise :type :validation + :code :not-valid-teams)) + (get-leave-org-summary cfg default-team-id valid-teams-to-delete-ids teams-to-transfer-count teams-to-exit-count))) + + (def ^:private schema:remove-team-from-org [:map [:team-id ::sm/uuid] diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 27cef605a6..2df3d68eeb 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -487,9 +487,10 @@ ;; Delete owned organizations on the fly (no grace period). ;; Nitrate iterates the user's owned orgs and, per org, calls - ;; Penpot back via ::notify-organization-deletion which renames - ;; the org's teams (prefixed with "[OrgName] ", including the - ;; user's "Your Penpot" team) and soft-deletes empty ones. + ;; Penpot back through two paths: ::notify-user-organizations-deletion + ;; (during delete-owned-orgs) and ::notify-organization-deletion. + ;; Both preserve org teams unchanged and only prefix or delete + ;; imported "Your Penpot" teams according to whether they still have files. (when (contains? cf/flags :nitrate) (nitrate/call cfg :delete-owned-orgs {:profile-id profile-id}) ;; Remove the user from any remaining org memberships. diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 542c77adbd..2ad040a04c 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -791,16 +791,16 @@ {:org-perms {:owner-id (dm/get-in team [:organization :owner-id]) :permissions (dm/get-in team [:organization :permissions])} :profile-id profile-id - :team-perms perms - ;; `onlyMe` is for a future org-level flow. - :allow-org-owner-delete? false}) + :team-perms perms}) (boolean (:is-owner perms)))] (when-not can-delete? (ex/raise :type :validation :code :only-owner-can-delete-team)) - (when (:is-default team) + ;; Protect the user's personal default team from deletion. + ;; Org-scoped default teams ("Your Penpot") are allowed to be deleted when they have no files. + (when (and (:is-default team) (not in-org?)) (ex/raise :type :validation :code :non-deletable-team :hint "impossible to delete default team")) diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index ffc62b1a3f..c241644662 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -296,46 +296,61 @@ RETURNING id, deleted_at;") nil) (defn manage-deleted-organization-teams - "For a list of teams, rename those with files and delete those without, then notify users." - [cfg {:keys [teams organization-name]}] - (let [teams (->> teams (filter uuid?) distinct (into []))] - (when (seq teams) + "For a deleted organization, preserve org teams unchanged and only prefix or + delete member Your Penpot teams depending on whether they still contain files." + [cfg {:keys [organization-id organization-name teams]}] + (let [all-team-ids (->> teams + (map :id) + (filter uuid?) + distinct + (into [])) + your-penpot-team-ids (->> teams + (filter :is-your-penpot) + (map :id) + (filter uuid?) + distinct + (into []))] + (when (seq all-team-ids) (let [org-prefix (str "[" (d/sanitize-string organization-name) "] ")] (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (let [teams-array (db/create-array conn "uuid" teams) - teams-with-files (->> (db/exec! conn [sql:get-teams-files-counts teams-array]) - (filter (fn [{:keys [total]}] (pos? total))) - (map :team-id) - (into #{})) - teams-to-keep (->> teams (filter teams-with-files) (into [])) - teams-to-delete (->> teams (remove teams-with-files) (into []))] + (let [teams-with-files (if (seq your-penpot-team-ids) + (->> (db/exec! conn [sql:get-teams-files-counts + (db/create-array conn "uuid" your-penpot-team-ids)]) + (filter (fn [{:keys [total]}] (pos? total))) + (map :team-id) + (into #{})) + #{}) + teams-to-prefix (->> your-penpot-team-ids (filter teams-with-files) (into [])) + teams-to-delete (->> your-penpot-team-ids (remove teams-with-files) (into []))] - ;; Rename teams that have files in one go - (when (seq teams-to-keep) + ;; Org teams move to the fallback org unchanged. Only imported + ;; Your Penpot teams keep the org prefix when they still have files. + (when (seq teams-to-prefix) (db/exec! conn [sql:prefix-teams-name-and-unset-default org-prefix - (db/create-array conn "uuid" teams-to-keep)])) + (db/create-array conn "uuid" teams-to-prefix)])) - ;; Soft-delete empty teams in one go + ;; Empty imported Your Penpot teams disappear entirely. (soft-delete-teams! cfg teams-to-delete) - (notifications/notify-organization-deletion cfg organization-name teams teams-to-delete) + (notifications/notify-organization-deletion cfg organization-id organization-name all-team-ids teams-to-delete) nil))))))) (sv/defmethod ::notify-organization-deletion - "For a list of teams, rename them with the name of the deleted org, and notify - of the deletion to the connected users" + "For a deleted organization, preserve org teams and only prefix or delete + imported Your Penpot teams before notifying connected users." {::doc/added "2.15" ::sm/params schema:notify-organization-deletion ::rpc/auth false} [cfg {:keys [organization-id]}] (let [org-summary (nitrate/call cfg :get-org-summary {:organization-id organization-id}) - teams (->> (:teams org-summary) - (map :id))] - (manage-deleted-organization-teams cfg {:teams teams :organization-name (:name org-summary)}) + teams (:teams org-summary)] + (manage-deleted-organization-teams cfg {:organization-name (:name org-summary) + :organization-id (:id org-summary) + :teams teams}) nil)) ;; ---- API: notify-user-organizations-deletion @@ -345,15 +360,18 @@ RETURNING id, deleted_at;") [:profile-id ::sm/uuid]]) (sv/defmethod ::notify-user-organizations-deletion - "For a given user, find all owned organizations and rename or delete their teams." + "For a given user, find all owned organizations and apply the deleted-org + transfer rules to their imported Your Penpot teams." {::doc/added "2.18" ::sm/params schema:notify-user-organizations-deletion} [cfg {:keys [profile-id]}] (let [owned-orgs (nitrate/call cfg :get-owned-orgs {:profile-id profile-id})] (doseq [org owned-orgs] (let [organization-name (:name org) - teams (map :id (:teams org))] - (manage-deleted-organization-teams cfg {:teams teams :organization-name organization-name})))) + teams (:teams org)] + (manage-deleted-organization-teams cfg {:organization-name organization-name + :organization-id (:id org) + :teams teams})))) nil) @@ -630,7 +648,8 @@ LEFT JOIN profile AS p [:map [:teams-to-delete ::sm/int] [:teams-to-transfer ::sm/int] - [:teams-to-exit ::sm/int]]) + [:teams-to-exit ::sm/int] + [:teams-to-detach ::sm/int]]) (sv/defmethod ::get-remove-from-org-summary "Get a summary of the teams that would be deleted, transferred, or exited @@ -650,9 +669,11 @@ LEFT JOIN profile AS p (when-not valid-default-team (ex/raise :type :validation :code :not-valid-teams)) - {:teams-to-delete (count valid-teams-to-delete-ids) - :teams-to-transfer (count valid-teams-to-transfer) - :teams-to-exit (count valid-teams-to-exit)})) + (cnit/get-leave-org-summary cfg + default-team-id + valid-teams-to-delete-ids + (count valid-teams-to-transfer) + (count valid-teams-to-exit)))) ;; API: cleanup-org-team-invitations diff --git a/backend/src/app/rpc/notifications.clj b/backend/src/app/rpc/notifications.clj index a439741092..8151088804 100644 --- a/backend/src/app/rpc/notifications.clj +++ b/backend/src/app/rpc/notifications.clj @@ -34,11 +34,12 @@ (defn notify-organization-deletion - [cfg organization-name teams deleted-teams] + [cfg organization-id organization-name teams deleted-teams] (let [msgbus (::mbus/msgbus cfg)] (mbus/pub! msgbus :topic uuid/zero :message {:type :organization-deleted + :organization-id organization-id :organization-name organization-name :teams teams - :deleted-teams deleted-teams}))) \ No newline at end of file + :deleted-teams deleted-teams}))) diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index 3069613bec..8e65e4bd65 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -186,8 +186,10 @@ expected-start (str "[" (d/sanitize-string organization-name) "] ") org-summary {:id organization-id :name organization-name - :teams [{:id (:id team-with-files)} - {:id (:id empty-team)}]} + :teams [{:id (:id team-with-files) + :is-your-penpot true} + {:id (:id empty-team) + :is-your-penpot true}]} calls (atom []) submitted (atom []) out (with-redefs [nitrate/call (fn [_cfg method params] @@ -222,6 +224,7 @@ (let [{:keys [topic message]} (first @calls)] (t/is (= uuid/zero topic)) (t/is (= :organization-deleted (:type message))) + (t/is (= organization-id (:organization-id message))) (t/is (= organization-name (:organization-name message))) (t/is (= #{(:id team-with-files) (:id empty-team)} (set (:teams message)))) @@ -254,12 +257,16 @@ org-2-prefix (str "[" (d/sanitize-string org-2-name) "] ") owned-orgs [{:id org-1-id :name org-1-name - :teams [{:id (:id org-1-team-files)} - {:id (:id org-1-team-empty)}]} + :teams [{:id (:id org-1-team-files) + :is-your-penpot true} + {:id (:id org-1-team-empty) + :is-your-penpot true}]} {:id org-2-id :name org-2-name - :teams [{:id (:id org-2-team-files)} - {:id (:id org-2-team-empty)}]}] + :teams [{:id (:id org-2-team-files) + :is-your-penpot true} + {:id (:id org-2-team-empty) + :is-your-penpot true}]}] calls (atom []) submitted (atom []) out (with-redefs [nitrate/call (fn [_cfg method params] @@ -313,6 +320,8 @@ m2 (org-msg org-2-name)] (t/is (some? m1)) (t/is (some? m2)) + (t/is (= org-1-id (:organization-id m1))) + (t/is (= org-2-id (:organization-id m2))) (t/is (= #{(:id org-1-team-files) (:id org-1-team-empty)} (set (:teams m1)))) (t/is (= #{(:id org-1-team-empty)} @@ -1016,9 +1025,10 @@ :organization-id organization-id :default-team-id (:id org-team)}))] (t/is (th/success? out)) - (t/is (= {:teams-to-delete 0 + (t/is (= {:teams-to-delete 1 :teams-to-transfer 0 - :teams-to-exit 0} + :teams-to-exit 0 + :teams-to-detach 0} (:result out))))) (t/deftest get-remove-from-org-summary-with-teams-to-delete @@ -1042,9 +1052,10 @@ :organization-id organization-id :default-team-id (:id org-team)}))] (t/is (th/success? out)) - (t/is (= {:teams-to-delete 1 + (t/is (= {:teams-to-delete 2 :teams-to-transfer 0 - :teams-to-exit 0} + :teams-to-exit 0 + :teams-to-detach 0} (:result out))))) (t/deftest get-remove-from-org-summary-with-teams-to-transfer @@ -1072,9 +1083,10 @@ :organization-id organization-id :default-team-id (:id org-team)}))] (t/is (th/success? out)) - (t/is (= {:teams-to-delete 0 + (t/is (= {:teams-to-delete 1 :teams-to-transfer 1 - :teams-to-exit 0} + :teams-to-exit 0 + :teams-to-detach 0} (:result out))))) (t/deftest get-remove-from-org-summary-with-teams-to-exit @@ -1101,9 +1113,10 @@ :organization-id organization-id :default-team-id (:id org-team)}))] (t/is (th/success? out)) - (t/is (= {:teams-to-delete 0 + (t/is (= {:teams-to-delete 1 :teams-to-transfer 0 - :teams-to-exit 1} + :teams-to-exit 1 + :teams-to-detach 0} (:result out))))) (t/deftest get-remove-from-org-summary-does-not-mutate diff --git a/backend/test/backend_tests/rpc_nitrate_test.clj b/backend/test/backend_tests/rpc_nitrate_test.clj index d8f4142a60..eeadccf687 100644 --- a/backend/test/backend_tests/rpc_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_nitrate_test.clj @@ -44,6 +44,13 @@ :organization-id (:id org-summary)} nil))) +(defn- nitrate-org-summary-only-mock + [org-summary] + (fn [_cfg method _params] + (case method + :get-org-summary org-summary + nil))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Tests ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -279,6 +286,64 @@ (let [team (th/db-get :team {:id (:id team1)})] (t/is (nil? (:deleted-at team)))))))) +(t/deftest get-leave-org-summary-counts-default-team-as-delete-when-empty + (let [profile-owner (th/create-profile* 1 {:is-active true}) + profile-user (th/create-profile* 2 {:is-active true}) + org-default-team (th/create-team* 97 {:profile-id (:id profile-user)}) + + organization-id (uuid/random) + your-penpot-id (:id org-default-team) + org-summary (make-org-summary + :organization-id organization-id + :organization-name "Test Org" + :owner-id (:id profile-owner) + :your-penpot-teams [your-penpot-id] + :org-teams [])] + + (with-redefs [nitrate/call (nitrate-org-summary-only-mock org-summary)] + (let [out (th/command! {::th/type :get-leave-org-summary + ::rpc/profile-id (:id profile-user) + :id organization-id + :default-team-id your-penpot-id})] + (t/is (th/success? out)) + (t/is (= {:teams-to-delete 1 + :teams-to-transfer 0 + :teams-to-exit 0 + :teams-to-detach 0} + (:result out))))))) + +(t/deftest get-leave-org-summary-counts-default-team-as-keep-when-has-files + (let [profile-owner (th/create-profile* 1 {:is-active true}) + profile-user (th/create-profile* 2 {:is-active true}) + org-default-team (th/create-team* 96 {:profile-id (:id profile-user)}) + project (th/create-project* 96 {:profile-id (:id profile-user) + :team-id (:id org-default-team)}) + _ (th/create-file* 96 {:profile-id (:id profile-user) + :project-id (:id project)}) + extra-team (th/create-team* 95 {:profile-id (:id profile-user)}) + + organization-id (uuid/random) + your-penpot-id (:id org-default-team) + org-summary (make-org-summary + :organization-id organization-id + :organization-name "Test Org" + :owner-id (:id profile-owner) + :your-penpot-teams [your-penpot-id] + :org-teams [(:id extra-team)])] + + (with-redefs [nitrate/call (nitrate-org-summary-only-mock org-summary)] + (let [out (th/command! {::th/type :get-leave-org-summary + ::rpc/profile-id (:id profile-user) + :id organization-id + :default-team-id your-penpot-id})] + (t/is (th/success? out)) + ;; extra-team is deletable, default team has files and is preserved. + (t/is (= {:teams-to-delete 1 + :teams-to-transfer 0 + :teams-to-exit 0 + :teams-to-detach 1} + (:result out))))))) + (t/deftest leave-org-error-org-owner-cannot-leave (let [profile-owner (th/create-profile* 1 {:is-active true}) org-default-team (th/create-team* 99 {:profile-id (:id profile-owner)}) diff --git a/common/src/app/common/types/nitrate_permissions.cljc b/common/src/app/common/types/nitrate_permissions.cljc index f21f8bff70..a395da6709 100644 --- a/common/src/app/common/types/nitrate_permissions.cljc +++ b/common/src/app/common/types/nitrate_permissions.cljc @@ -19,10 +19,11 @@ (= permission-value "any"))) (defn- can-delete-team? - [{:keys [is-org-owner? permission-value team-perms allow-org-owner-delete?]}] + [{:keys [is-org-owner? permission-value team-perms]}] (cond - (= permission-value "onlyMe") - (and allow-org-owner-delete? is-org-owner?) + ;; Org owners can always delete teams inside their organizations. + is-org-owner? + true (= permission-value "onlyOwners") (boolean (:is-owner team-perms)) :else false)) @@ -76,7 +77,7 @@ (defn allowed? "Returns true only for explicitly allowed actions (fail-closed)." - [action {:keys [org-perms profile-id team-perms allow-org-owner-delete? target-org-same-owner?]}] + [action {:keys [org-perms profile-id team-perms target-org-same-owner?]}] (let [{:keys [permission-key check-fn] :as rule} (get action-rules action) permissions (normalize-org-permissions org-perms) @@ -87,7 +88,6 @@ :else (boolean (check-fn {:is-org-owner? is-org-owner? :permission-value permission-value :team-perms team-perms - :allow-org-owner-delete? allow-org-owner-delete? :target-org-same-owner? target-org-same-owner?}))))) (defn can-send-invitations? diff --git a/common/test/common_tests/types/nitrate_permissions_test.cljc b/common/test/common_tests/types/nitrate_permissions_test.cljc index b5832b3793..795b94cf74 100644 --- a/common/test/common_tests/types/nitrate_permissions_test.cljc +++ b/common/test/common_tests/types/nitrate_permissions_test.cljc @@ -21,15 +21,15 @@ :profile-id :member :team-perms {:is-admin true}})))) -(t/deftest org-owner-is-allowed-for-create +(t/deftest org-owner-is-allowed-for-create-and-delete (t/is (true? (nitrate-perms/allowed? :create-team {:org-perms org-perms :profile-id :owner :team-perms {:is-admin false}}))) - (t/is (false? (nitrate-perms/allowed? :delete-team - {:org-perms org-perms - :profile-id :owner - :team-perms {:is-admin false}})))) + (t/is (true? (nitrate-perms/allowed? :delete-team + {:org-perms org-perms + :profile-id :owner + :team-perms {:is-admin false}})))) (t/deftest create-team-permission-rules (t/is (true? (nitrate-perms/allowed? :create-team @@ -57,16 +57,11 @@ :profile-id :member :team-perms {:is-admin true}})))) -(t/deftest delete-team-onlyme-is-gated-for-future-org-flow +(t/deftest delete-team-onlyme-still-allows-org-owner (let [only-me-org (assoc org-perms :permissions {:create-teams "any" :delete-teams "onlyMe"})] - (t/is (false? (nitrate-perms/allowed? :delete-team - {:org-perms only-me-org - :profile-id :owner - :team-perms {:is-owner false :is-admin false}}))) (t/is (true? (nitrate-perms/allowed? :delete-team {:org-perms only-me-org - :allow-org-owner-delete? true :profile-id :owner :team-perms {:is-owner false :is-admin false}}))) (t/is (false? (nitrate-perms/allowed? :delete-team diff --git a/frontend/src/app/main/data/dashboard.cljs b/frontend/src/app/main/data/dashboard.cljs index c31312d90e..65619fd205 100644 --- a/frontend/src/app/main/data/dashboard.cljs +++ b/frontend/src/app/main/data/dashboard.cljs @@ -731,16 +731,19 @@ (defn- handle-organization-deleted - [{:keys [organization-name teams deleted-teams]}] + [{:keys [organization-id organization-name teams deleted-teams]}] (ptk/reify ::handle-organization-deleted ptk/WatchEvent (watch [_ state _] (when (contains? cf/flags :nitrate) (let [team-id (:current-team-id state) + current-team (dm/get-in state [:teams team-id]) + current-org-id (dm/get-in current-team [:organization :id]) teams-set (set teams) notify? (contains? teams-set team-id) fetch? (some (:teams state) teams) - go-to-default? (some #{team-id} deleted-teams)] + go-to-default? (or (some #{team-id} deleted-teams) + (= organization-id current-org-id))] (rx/concat (when go-to-default? ;; If the user is currently on one of the deleted teams (rx/of (dcm/go-to-dashboard-recent {:team-id :default}))) diff --git a/frontend/src/app/main/data/nitrate.cljs b/frontend/src/app/main/data/nitrate.cljs index 1a245fa0d5..d08a8e90ce 100644 --- a/frontend/src/app/main/data/nitrate.cljs +++ b/frontend/src/app/main/data/nitrate.cljs @@ -163,6 +163,49 @@ :level :success})))) (rx/catch on-error)))))) +(defn show-leave-org-modal + [{:keys [organization profile default-team-id leave-fn teams-to-transfer on-error]}] + (ptk/reify ::show-leave-org-modal + ptk/WatchEvent + (watch [_ _ _] + (->> (rp/cmd! ::get-leave-org-summary {:id (:id organization) + :default-team-id default-team-id}) + (rx/mapcat + (fn [summary] + (let [num-teams-to-delete (:teams-to-delete summary) + num-teams-to-transfer (:teams-to-transfer summary) + num-teams-to-exit (:teams-to-exit summary) + num-teams-to-detach (:teams-to-detach summary)] + (cond + (pos? num-teams-to-transfer) + (rx/of + (modal/show + {:type :leave-and-reassign-org + :profile profile + :teams-to-transfer teams-to-transfer + :num-teams-to-delete num-teams-to-delete + :accept leave-fn})) + + (or (pos? num-teams-to-delete) + (pos? num-teams-to-exit) + (pos? num-teams-to-detach)) + (rx/of (modal/show + {:type :confirm + :title (tr "modals.before-leave-org.title" (:name organization)) + :message (tr "modals.before-leave-org.message") + :accept-label (tr "modals.leave-org-confirm.accept") + :on-accept leave-fn + :error-msg (tr "modals.before-leave-org.warning")})) + + :else + (rx/of (modal/show + {:type :confirm + :title (tr "modals.leave-org-confirm.title" (:name organization)) + :message (tr "modals.leave-org-confirm.message") + :accept-label (tr "modals.leave-org-confirm.accept") + :on-accept leave-fn})))))) + (rx/catch on-error))))) + (defn remove-team-from-org [{:keys [team-id organization-id organization-name] :as params}] diff --git a/frontend/src/app/main/ui/dashboard/sidebar.cljs b/frontend/src/app/main/ui/dashboard/sidebar.cljs index 72a42e2c33..648aa4c5c5 100644 --- a/frontend/src/app/main/ui/dashboard/sidebar.cljs +++ b/frontend/src/app/main/ui/dashboard/sidebar.cljs @@ -599,13 +599,11 @@ (filter #(dm/get-in % [:permissions :is-owner]) non-default-teams)) not-owned-teams (mf/with-memo [non-default-teams] (remove #(dm/get-in % [:permissions :is-owner]) non-default-teams)) - teams-to-delete (mf/with-memo [owned-teams] - (filter #(= (count (:members %)) 1) owned-teams)) + owned-teams-members-loaded? + (mf/with-memo [owned-teams] + (every? #(contains? % :members) owned-teams)) teams-to-transfer (mf/with-memo [owned-teams] (filter #(> (count (:members %)) 1) owned-teams)) - num-teams-to-leave (+ (count teams-to-transfer) (count not-owned-teams)) - num-teams-to-delete (count teams-to-delete) - num-teams-to-transfer (count teams-to-transfer) on-error (mf/use-fn @@ -627,14 +625,16 @@ leave-fn (mf/use-fn - (mf/deps on-error organization default-team-id not-owned-teams teams-to-delete) + (mf/deps on-error organization default-team-id not-owned-teams owned-teams) (fn [{:keys [teams-to-transfer]}] (let [teams-to-leave (cond->> not-owned-teams :always (map #(select-keys % [:id])) (seq teams-to-transfer) (concat teams-to-transfer)) - teams-to-delete (map :id teams-to-delete)] + teams-to-delete (->> owned-teams + (filter #(= (count (:members %)) 1)) + (map :id))] (st/emit! (dnt/leave-org {:id (:id organization) @@ -646,41 +646,22 @@ on-leave-clicked (mf/use-fn - (mf/deps leave-fn profile organization teams-to-transfer num-teams-to-leave num-teams-to-delete num-teams-to-transfer) + (mf/deps leave-fn profile organization default-team-id teams-to-transfer on-error owned-teams-members-loaded?) (fn [] - (cond - (and (pos? num-teams-to-delete) - (zero? num-teams-to-transfer)) - (st/emit! (modal/show - {:type :confirm - :title (tr "modals.before-leave-org.title" (:name organization)) - :message (tr "modals.before-leave-org.message") - :accept-label (tr "modals.leave-org-confirm.accept") - :on-accept leave-fn - :error-msg (tr "modals.before-leave-org.warning")})) - (pos? num-teams-to-transfer) - (st/emit! - (modal/show - {:type :leave-and-reassign-org - :profile profile - :teams-to-transfer teams-to-transfer - :num-teams-to-delete num-teams-to-delete - :accept leave-fn})) - - :else - (st/emit! (modal/show - {:type :confirm - :title (tr "modals.leave-org-confirm.title" (:name organization)) - :message (tr "modals.leave-org-confirm.message") - :accept-label (tr "modals.leave-org-confirm.accept") - :on-accept leave-fn})))))] + (when owned-teams-members-loaded? + (st/emit! (dnt/show-leave-org-modal {:organization organization + :profile profile + :default-team-id default-team-id + :leave-fn leave-fn + :teams-to-transfer teams-to-transfer + :on-error on-error})))))] (mf/use-effect + (mf/deps owned-teams) (fn [] - ;; We need all the team members of the owned teams - ;; TODO this will re-render once for each owned team, not very performance-wise - (do - (doseq [team owned-teams] - (st/emit! (dtm/fetch-members (:id team))))))) + ;; Fetch members for any owned team that doesn't have them yet. + (doseq [team owned-teams + :when (not (contains? team :members))] + (st/emit! (dtm/fetch-members (:id team)))))) [:> dropdown-menu* props [:> dropdown-menu-item* {:on-click on-leave-clicked