From 3ff8c5f0879742a845736f9c722352e8116fdec3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 9 Dec 2024 17:40:57 +0300 Subject: [PATCH 1/5] [#59967] Remove reminder notifications from aggregation and have them as standalone https://community.openproject.org/work_packages/59967 --- .../center/state/ian-center.service.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts index 0979826f6503..fb986dca3cc7 100644 --- a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts +++ b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts @@ -112,6 +112,22 @@ export class IanCenterService extends UntilDestroyedMixin { .aggregatedCenterNotifications$ .pipe( map((items) => Object.values(items)), + map((notificationGroups) => { + return notificationGroups.reduce((acc, group) => { + const reminders = group.filter((notification) => notification.reason === 'reminder'); + const others = group.filter((notification) => notification.reason !== 'reminder'); + + // Extract reminders into standalone groups so they can be displayed individually + if (reminders.length > 0) { + reminders.forEach((reminder) => acc.push([reminder])); + } + if (others.length > 0) { + acc.push(others); + } + + return acc; + }, [] as INotification[][]); + }), distinctUntilChanged(), ); From c773f2e4fde92b10e7e2a6b3695fb8bb16172146 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 10 Dec 2024 20:35:52 +0300 Subject: [PATCH 2/5] Refactor notifications filter into one --- .../center/state/ian-center.service.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts index fb986dca3cc7..e816ae97a7db 100644 --- a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts +++ b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts @@ -114,8 +114,14 @@ export class IanCenterService extends UntilDestroyedMixin { map((items) => Object.values(items)), map((notificationGroups) => { return notificationGroups.reduce((acc, group) => { - const reminders = group.filter((notification) => notification.reason === 'reminder'); - const others = group.filter((notification) => notification.reason !== 'reminder'); + const { reminders, others } = group.reduce((result, notification) => { + if (notification.reason === 'reminder') { + result.reminders.push(notification); + } else { + result.others.push(notification); + } + return result; + }, { reminders: [] as INotification[], others: [] as INotification[] }); // Extract reminders into standalone groups so they can be displayed individually if (reminders.length > 0) { From a846cf99663d7f3446275482ab8fb07123e11466 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 10 Dec 2024 20:47:59 +0300 Subject: [PATCH 3/5] refactor to use single map --- .../center/state/ian-center.service.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts index e816ae97a7db..9c41f1a46587 100644 --- a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts +++ b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts @@ -111,10 +111,9 @@ export class IanCenterService extends UntilDestroyedMixin { notifications$ = this .aggregatedCenterNotifications$ .pipe( - map((items) => Object.values(items)), - map((notificationGroups) => { - return notificationGroups.reduce((acc, group) => { - const { reminders, others } = group.reduce((result, notification) => { + map((items) => { + return Object.values(items).reduce((acc, workPackageNotificationGroup) => { + const { reminders, others } = workPackageNotificationGroup.reduce((result, notification) => { if (notification.reason === 'reminder') { result.reminders.push(notification); } else { From 2549a15b67c6973dc0b49e912f1064e1523acebf Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 10 Dec 2024 21:02:40 +0300 Subject: [PATCH 4/5] Update notification reminder spec remove it from aggregation --- .../notification_center_reminder_spec.rb | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/features/notifications/notification_center/notification_center_reminder_spec.rb b/spec/features/notifications/notification_center/notification_center_reminder_spec.rb index 3b9f45e28aec..015aa20a36f1 100644 --- a/spec/features/notifications/notification_center/notification_center_reminder_spec.rb +++ b/spec/features/notifications/notification_center/notification_center_reminder_spec.rb @@ -4,6 +4,7 @@ RSpec.describe "Notification center reminder, mention and date alert", :js, :with_cuprite, + with_ee: %i[date_alerts], with_settings: { journal_aggregation_time_minutes: 0 } do shared_let(:project) { create(:project) } shared_let(:actor) { create(:user, firstname: "Actor", lastname: "User") } @@ -46,13 +47,18 @@ wait_for_reload end - context "with reminders", with_ee: %i[date_alerts] do - it "shows only the reminder alert time and note" do - center.within_item(notification_reminder) do - expect(page).to have_text("Date alert, Mentioned, Reminder") - expect(page).to have_no_text("Actor user") - expect(page).to have_text("a few seconds ago.\nNote: “This is an important reminder”") - end + it "shows the reminder alert in own entry" do + center.within_item(notification_reminder) do + expect(page).to have_text("##{work_package.id}\n- #{project.name} -\nReminder") + expect(page).to have_no_text("Actor user") + expect(page).to have_text("a few seconds ago.\nNote: “This is an important reminder”") + end + end + + it "shows other notification reasons aggregated" do + center.within_item(notification_date_alert) do + expect(page).to have_text("##{work_package.id}\n- #{project.name} -\nDate alert, Mentioned") + expect(page).to have_no_text("Actor user") end end end From d35aec72a190878b6a09f2ce13548dc14d3b9a16 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 10 Dec 2024 21:29:27 +0300 Subject: [PATCH 5/5] add notification center sidemenu feature specs --- .../notification_center_sidemenu_spec.rb | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/spec/features/notifications/notification_center/notification_center_sidemenu_spec.rb b/spec/features/notifications/notification_center/notification_center_sidemenu_spec.rb index faf5a6209aa5..c2cd4a832484 100644 --- a/spec/features/notifications/notification_center/notification_center_sidemenu_spec.rb +++ b/spec/features/notifications/notification_center/notification_center_sidemenu_spec.rb @@ -24,6 +24,7 @@ shared_let(:work_package4) { create(:work_package, project: project3, author: other_user) } shared_let(:work_package5) { create(:work_package, :is_milestone, project: project3, author: other_user) } shared_let(:work_package6) { create(:work_package, :is_milestone, project: project3, author: other_user) } + shared_let(:work_package7) { create(:work_package, project: project3, author: other_user) } let(:notification_watched) do create(:notification, @@ -67,9 +68,19 @@ reason: :shared) end + let(:notification_reminder) do + reminder = create(:reminder, remindable: work_package7, creator: other_user, note: "This is an important reminder") + notification = create(:notification, + recipient:, + resource: work_package7, + reason: :reminder) + create(:reminder_notification, reminder:, notification:) + notification + end + let(:notifications) do [notification_watched, notification_assigned, notification_responsible, notification_mentioned, notification_date, - notification_shared] + notification_shared, notification_reminder] end let(:center) { Pages::Notifications::Center.new } @@ -99,6 +110,7 @@ side_menu.expect_item_with_no_count "Watcher" side_menu.expect_item_with_no_count "Date alert" side_menu.expect_item_with_no_count "Shared" + side_menu.expect_item_with_no_count "Reminder" end end @@ -106,35 +118,37 @@ side_menu.expect_open # Expect standard filters - side_menu.expect_item_with_count "Inbox", 6 + side_menu.expect_item_with_count "Inbox", 7 side_menu.expect_item_with_count "Assignee", 1 side_menu.expect_item_with_count "Mentioned", 1 side_menu.expect_item_with_count "Accountable", 1 side_menu.expect_item_with_count "Watcher", 1 side_menu.expect_item_with_count "Date alert", 1 side_menu.expect_item_with_count "Shared", 1 + side_menu.expect_item_with_count "Reminder", 1 # Expect project filters side_menu.expect_item_with_count project.name, 1 side_menu.expect_item_with_count project2.name, 1 - side_menu.expect_item_with_count "... #{project3.name}", 4 + side_menu.expect_item_with_count "... #{project3.name}", 5 # Reading a notification... center.mark_notification_as_read notification_watched # ... will change the filter counts - side_menu.expect_item_with_count "Inbox", 5 + side_menu.expect_item_with_count "Inbox", 6 side_menu.expect_item_with_count "Assignee", 1 side_menu.expect_item_with_count "Mentioned", 1 side_menu.expect_item_with_count "Accountable", 1 side_menu.expect_item_with_count "Date alert", 1 side_menu.expect_item_with_count "Shared", 1 + side_menu.expect_item_with_count "Reminder", 1 side_menu.expect_item_with_no_count "Watcher" # ... and show only those projects with a notification side_menu.expect_no_item project.name side_menu.expect_item_with_count project2.name, 1 - side_menu.expect_item_with_count "... #{project3.name}", 4 + side_menu.expect_item_with_count "... #{project3.name}", 5 # Empty filter sets have a separate message side_menu.click_item "Watcher" @@ -152,6 +166,7 @@ side_menu.expect_item_with_no_count "Watcher" side_menu.expect_item_with_no_count "Date alert" side_menu.expect_item_with_no_count "Shared" + side_menu.expect_item_with_no_count "Reminder" side_menu.expect_no_item project.name side_menu.expect_no_item project2.name @@ -160,66 +175,74 @@ it "updates the content when a filter is clicked" do # All notifications are shown - center.expect_work_package_item *notifications + center.expect_work_package_item(*notifications) # Filter for "Watcher" side_menu.click_item "Watcher" side_menu.finished_loading center.expect_work_package_item notification_watched center.expect_no_item notification_assigned, notification_responsible, notification_mentioned, notification_date, - notification_shared + notification_shared, notification_reminder # Filter for "Assignee" side_menu.click_item "Assignee" side_menu.finished_loading center.expect_work_package_item notification_assigned center.expect_no_item notification_watched, notification_responsible, notification_mentioned, notification_date, - notification_shared + notification_shared, notification_reminder # Filter for "Accountable" side_menu.click_item "Accountable" side_menu.finished_loading center.expect_work_package_item notification_responsible center.expect_no_item notification_watched, notification_assigned, notification_mentioned, notification_date, - notification_shared + notification_shared, notification_reminder # Filter for "Mentioned" side_menu.click_item "Mentioned" side_menu.finished_loading center.expect_work_package_item notification_mentioned center.expect_no_item notification_watched, notification_assigned, notification_responsible, notification_date, - notification_shared + notification_shared, notification_reminder # Filter for "Date alert" side_menu.click_item "Date alert" side_menu.finished_loading center.expect_work_package_item notification_date center.expect_no_item notification_watched, notification_assigned, notification_responsible, notification_mentioned, - notification_shared + notification_shared, notification_reminder # Filter for "Shared" side_menu.click_item "Shared" side_menu.finished_loading center.expect_work_package_item notification_shared center.expect_no_item notification_watched, notification_assigned, notification_responsible, notification_mentioned, - notification_date + notification_date, notification_reminder + + # Filter for "Reminder" + side_menu.click_item "Reminder" + side_menu.finished_loading + center.expect_work_package_item notification_reminder + center.expect_no_item notification_watched, notification_assigned, notification_responsible, notification_mentioned, + notification_date, notification_shared # Filter for project1 side_menu.click_item project.name side_menu.finished_loading center.expect_work_package_item notification_watched center.expect_no_item notification_assigned, notification_responsible, notification_mentioned, notification_date, - notification_shared + notification_shared, notification_reminder # Filter for project3 side_menu.click_item "... #{project3.name}" side_menu.finished_loading - center.expect_work_package_item notification_responsible, notification_mentioned, notification_date, notification_shared + center.expect_work_package_item notification_responsible, notification_mentioned, notification_date, notification_shared, + notification_reminder center.expect_no_item notification_watched, notification_assigned # Reset by clicking on the Inbox side_menu.click_item "Inbox" side_menu.finished_loading - center.expect_work_package_item *notifications + center.expect_work_package_item(*notifications) end end