From eaee546c818a6d182c0a7e59cb3d1122bd2546cf Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 31 Mar 2026 16:43:55 +0300 Subject: [PATCH] fix(reminders): align visible scope with instance-level authorization check (#22615) The class-level visible scope now verifies the user retains access to the associated remindable, consistent with the existing instance method. Updated specs to reflect the corrected authorization behavior. --- app/models/reminder.rb | 4 +- spec/models/reminder_spec.rb | 54 +++++++++++++++---- .../api/v3/reminders/reminders_api_spec.rb | 4 +- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/app/models/reminder.rb b/app/models/reminder.rb index af935f5708e..cb4d210c3b5 100644 --- a/app/models/reminder.rb +++ b/app/models/reminder.rb @@ -36,9 +36,11 @@ class Reminder < ApplicationRecord has_many :notifications, through: :reminder_notifications # Currently, reminders are personal, meaning - # they are only visible to the user who created them. + # they are only visible to the user who created them + # and who still has access to the remindable. def self.visible(user) where(creator: user) + .where(remindable_type: WorkPackage.name, remindable_id: WorkPackage.visible(user).select(:id)) end def self.upcoming_and_visible_to(user) diff --git a/spec/models/reminder_spec.rb b/spec/models/reminder_spec.rb index 6ea6b49f9cf..f53e5d90f9c 100644 --- a/spec/models/reminder_spec.rb +++ b/spec/models/reminder_spec.rb @@ -41,31 +41,65 @@ RSpec.describe Reminder do describe "Scopes" do describe ".visible" do it "returns reminders where the creator is the given user" do - user = create(:user) - other_user = create(:user) - reminder = create(:reminder, creator: user) - _other_reminder = create(:reminder, creator: other_user) + role = create(:project_role, permissions: %i[view_work_packages]) + project = create(:project) + user = create(:user, member_with_roles: { project => role }) + other_user = create(:user, member_with_roles: { project => role }) + work_package = create(:work_package, project:) + reminder = create(:reminder, creator: user, remindable: work_package) + _other_reminder = create(:reminder, creator: other_user, remindable: work_package) expect(described_class.visible(user)).to contain_exactly(reminder) end + + it "excludes reminders whose remindable is no longer visible to the user" do + role = create(:project_role, permissions: %i[view_work_packages]) + project = create(:project) + user = create(:user, member_with_roles: { project => role }) + work_package = create(:work_package, project:) + reminder = create(:reminder, creator: user, remindable: work_package) + + expect(described_class.visible(user)).to contain_exactly(reminder) + + Member.where(principal: user, project:).destroy_all + + expect(described_class.visible(user)).to be_empty + end end describe ".upcoming_and_visible_to" do it "returns reminders where the creator is the given user" \ "and there are no reminder_notifications for it yet" do - user = create(:user) - other_user = create(:user) - reminder = create(:reminder, creator: user) - _another_reminder_with_notification = create(:reminder, creator: user) do |r| + role = create(:project_role, permissions: %i[view_work_packages]) + project = create(:project) + user = create(:user, member_with_roles: { project => role }) + other_user = create(:user, member_with_roles: { project => role }) + work_package = create(:work_package, project:) + reminder = create(:reminder, creator: user, remindable: work_package) + _another_reminder_with_notification = create(:reminder, creator: user, remindable: work_package) do |r| create(:reminder_notification, reminder: r) end - _other_users_reminder_with_notification = create(:reminder, creator: other_user) do |r| + _other_users_reminder_with_notification = create(:reminder, creator: other_user, remindable: work_package) do |r| create(:reminder_notification, reminder: r) end - _other_users_reminder = create(:reminder, creator: other_user) + _other_users_reminder = create(:reminder, creator: other_user, remindable: work_package) expect(described_class.upcoming_and_visible_to(user)).to contain_exactly(reminder) end + + it "excludes reminders whose remindable is no longer visible to the user" do + role = create(:project_role, permissions: %i[view_work_packages]) + project = create(:project) + user = create(:user, member_with_roles: { project => role }) + work_package = create(:work_package, project:) + reminder = create(:reminder, creator: user, remindable: work_package) + + expect(described_class.upcoming_and_visible_to(user)).to contain_exactly(reminder) + + Member.where(principal: user, project:).destroy_all + + expect(described_class.upcoming_and_visible_to(user)).to be_empty + end end end diff --git a/spec/requests/api/v3/reminders/reminders_api_spec.rb b/spec/requests/api/v3/reminders/reminders_api_spec.rb index b6432d5bfce..5fa79aad464 100644 --- a/spec/requests/api/v3/reminders/reminders_api_spec.rb +++ b/spec/requests/api/v3/reminders/reminders_api_spec.rb @@ -130,7 +130,7 @@ RSpec.describe API::V3::Reminders::RemindersAPI do it_behaves_like "error response", 404, "NotFound", - "The requested resource could not be found." + "The reminder you are looking for cannot be found or has been deleted." end end @@ -191,7 +191,7 @@ RSpec.describe API::V3::Reminders::RemindersAPI do it_behaves_like "error response", 404, "NotFound", - "The requested resource could not be found." + "The reminder you are looking for cannot be found or has been deleted." end end end