From c538a9aa92b80aec8c619ed6db9176fe602c51b9 Mon Sep 17 00:00:00 2001 From: Markku Rontu Date: Mon, 15 Jan 2024 12:12:11 +0200 Subject: [PATCH 1/3] fix: user role after deletion There was a problem in calculating a user's roles, because only the just updated applications and their roles were considered. The code was supposed to calculate the new roles for all updated users based on all of their applications. This fixes the above and adds a test to catch it in the future. It also makes sense to base the users by roles on the result of the previous logic. The future cache logic will not be based on this but can take advantage of the test anyway. --- src/clj/rems/db/applications.clj | 20 ++++++++++++---- test/clj/rems/db/test_applications.clj | 32 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/clj/rems/db/applications.clj b/src/clj/rems/db/applications.clj index 6c435085c..90844e74e 100644 --- a/src/clj/rems/db/applications.clj +++ b/src/clj/rems/db/applications.clj @@ -280,19 +280,29 @@ updated-users)) ;; update all the users that are in this round - updated-roles-by-user (->> updated-users - (mapcat new-updated-apps-by-user) - (distinct-by :application/id) - group-roles-by-user) + updated-roles-by-user (into {} + (for [userid updated-users + :let [apps (->> userid + new-personalized-apps-by-user + (distinct-by :application/id)) + roles (->> apps + (mapcat :application/roles) + set)]] + [userid roles])) new-roles-by-user (doall (merge (apply dissoc old-roles-by-user updated-users) updated-roles-by-user)) + ;; now calculate the reverse, i.e. users by role + new-updated-users-by-role (->> (for [user updated-users + role (updated-roles-by-user user)] + {role #{user}}) + (apply merge-with set/union)) ;; update all the users that are in this round new-users-by-role (->> old-users-by-role ;; remove users updated in this round (map-vals (fn [users] (apply disj users updated-users))) ;; add users back to correct groups - (merge-with set/union (group-users-by-role (vals new-updated-enriched-apps))) + (merge-with set/union new-updated-users-by-role) doall)] {::raw-apps new-raw-apps diff --git a/test/clj/rems/db/test_applications.clj b/test/clj/rems/db/test_applications.clj index e4279d170..18eb97b08 100644 --- a/test/clj/rems/db/test_applications.clj +++ b/test/clj/rems/db/test_applications.clj @@ -121,6 +121,38 @@ (testing "db entry for application is gone" (is (not (contains? (set (map :id (db/get-application-ids {}))) app-id1))))) + (testing "with two applications" + (let [app-id1 (test-helpers/create-application! {:actor "applicant1"}) + app-id2 (test-helpers/create-application! {:actor "applicant1"})] + (test-helpers/command! {:application-id app-id2 + :type :application.command/submit + :actor "applicant1"}) + (is (applications/get-application app-id1)) + (is (applications/get-application app-id2)) + (is (= [app-id1 app-id2] (sort (map :application/id (applications/get-my-applications "applicant1"))))) + (is (= #{:applicant} (applications/get-all-application-roles "applicant1"))) + (is (= #{"applicant1" "applicant2"} (applications/get-users-with-role :applicant))) + + (applications/delete-application! app-id1) + + (testing "application disappears from my applications" + (is (= [app-id2] (map :application/id (applications/get-my-applications "applicant1"))))) + (testing "application disappears from all-applications-cache (apps-by-user)" + (is (= [app-id2] (map :application/id (applications/get-all-applications "applicant1"))))) + (testing "role persists in roles-by-user" + (is (= #{:applicant} (applications/get-all-application-roles "applicant1")))) + (testing "role persists in users-by-role" + (is (= #{"applicant1" "applicant2"} (applications/get-users-with-role :applicant)))) + (testing "deleted draft is gone" + (is (not (applications/get-application app-id1)))) + (is (applications/get-application app-id2)) + (testing "events are gone from event cache" + (is (empty? (db-events/get-application-events app-id1)))) + (testing "events are gone from DB" + (is (empty? (db/get-application-events {:application app-id1})))) + (testing "db entry for application is gone" + (is (not (contains? (set (map :id (db/get-application-ids {}))) app-id1)))))) + (let [app-id1 (test-helpers/create-application! {:actor "applicant1"})] (test-helpers/command! {:application-id app-id1 :type :application.command/submit From d20530737d0d661371a07a93a2db68854da4e2d6 Mon Sep 17 00:00:00 2001 From: Markku Rontu Date: Mon, 15 Jan 2024 12:15:14 +0200 Subject: [PATCH 2/3] doc: update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abc8f3718..4449a25a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ have notable changes. Changes since v2.35 +### Fixes +- Fix users roles after deletion. This was introduced in the previous release that optimizes cache updates. (#3243) + ## v2.35 "Selkämerenkatu" 2023-12-13 **NB: This release removes the experimental application PDF export API. The non-experimental PDF export API is preferred instead. (#3098)** From ecc76d6cc28670bc146e8a00d1c24b6281f21f36 Mon Sep 17 00:00:00 2001 From: Markku Rontu Date: Mon, 15 Jan 2024 15:36:41 +0200 Subject: [PATCH 3/3] doc: update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4449a25a0..788b4e2e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ have notable changes. Changes since v2.35 ### Fixes -- Fix users roles after deletion. This was introduced in the previous release that optimizes cache updates. (#3243) +- Fix issue with user roles after deletion. This was introduced in the previous release that optimizes cache updates. (#3243) ## v2.35 "Selkämerenkatu" 2023-12-13