[#43677] Update notification groups to use dateAlert reason

https://community.openproject.org/work_packages/43677

app/models/queries/notifications/notification_query.rb

@@ -34,4 +34,8 @@ class Queries::Notifications::NotificationQuery < Queries::BaseQuery
   def default_scope
     Notification.visible(User.current).recipient(user)
   end
+
+  def group_values
+    ::API::V3::Notifications::PropertyFactory.groups_for(super)
+  end
 end

frontend/src/app/features/in-app-notifications/center/in-app-notification-center.component.ts

@@ -80,7 +80,7 @@ export class InAppNotificationCenterComponent implements OnInit {
       title: this.I18n.t('js.notifications.menu.watched'),
     },
     {
-      key: 'date_alert',
+      key: 'dateAlert',
       title: this.I18n.t('js.notifications.menu.date_alert'),
     },
   ];

frontend/src/app/features/in-app-notifications/center/menu/menu.component.ts

@@ -93,7 +93,7 @@ export class IanMenuComponent implements OnInit {
       ...getUiLinkForFilters({ filter: 'reason', name: 'watched' }),
     },
     {
-      key: 'date_alert',
+      key: 'dateAlert',
       title: this.I18n.t('js.notifications.menu.date_alert'),
       icon: 'date-alert',
       ...getUiLinkForFilters({ filter: 'reason', name: 'date_alert' }),

lib/api/v3/notifications/property_factory.rb

@@ -35,6 +35,8 @@ module API::V3::Notifications
       date_alert_due_date: "due_date"
     }.freeze
 
+    DATE_ALERT_REASONS = %w(date_alert_start_date date_alert_due_date).freeze
+
     module_function
 
     # Fetch the collection of details for a notification
@@ -53,11 +55,19 @@ module API::V3::Notifications
       ::API::V3::Values::Schemas::ValueSchemaFactory.all_for(detail_properties)
     end
 
+    # Returns the outward facing notification group attributes
+    def groups_for(values)
+      group_values = values.except(*DATE_ALERT_REASONS)
+      date_alert_values = values.slice(*DATE_ALERT_REASONS).values.reduce(:+)
+      group_values["dateAlert"] = date_alert_values if date_alert_values
+      group_values
+    end
+
     # Returns the outward facing reason e.g. `dateAlert` as opposed to `date_alert_start_date`.
     def reason_for(notification)
       reason = notification.reason
       case reason
-      when 'date_alert_start_date', 'date_alert_due_date'
+      when *DATE_ALERT_REASONS
         'dateAlert'
       else
         reason

spec/features/notifications/notification_center/notification_center_sidemenu_spec.rb

@@ -1,56 +1,65 @@
 require 'spec_helper'
 
-describe "Notification center sidemenu", type: :feature, js: true do
-  shared_let(:project) { create :project }
-  shared_let(:project2) { create :project }
-  shared_let(:project3) { create :project, parent: project2 }
+describe "Notification center sidemenu", js: true do
+  shared_let(:project) { create(:project) }
+  shared_let(:project2) { create(:project) }
+  shared_let(:project3) { create(:project, parent: project2) }
 
   shared_let(:recipient) do
-    create :user,
+    create(:user,
            member_in_projects: [project, project2, project3],
-           member_with_permissions: %i[view_work_packages]
+           member_with_permissions: %i[view_work_packages])
   end
   shared_let(:other_user) { create(:user) }
 
-  shared_let(:work_package) { create :work_package, project:, author: other_user }
-  shared_let(:work_package2) { create :work_package, project: project2, author: other_user }
-  shared_let(:work_package3) { create :work_package, project: project3, author: other_user }
-  shared_let(:work_package4) { create :work_package, project: project3, author: other_user }
+  shared_let(:work_package) { create(:work_package, project:, author: other_user) }
+  shared_let(:work_package2) { create(:work_package, project: project2, author: other_user) }
+  shared_let(:work_package3) { create(:work_package, project: project3, author: other_user) }
+  shared_let(:work_package4) { create(:work_package, project: project3, author: other_user) }
+  shared_let(:work_package5) { create(:work_package, project: project3, author: other_user) }
 
   let(:notification) do
-    create :notification,
+    create(:notification,
            recipient:,
            project:,
            resource: work_package,
-           reason: :watched
+           reason: :watched)
   end
 
   let(:notification2) do
-    create :notification,
+    create(:notification,
            recipient:,
            project: project2,
            resource: work_package2,
-           reason: :assigned
+           reason: :assigned)
   end
 
   let(:notification3) do
-    create :notification,
+    create(:notification,
            recipient:,
            project: project3,
            resource: work_package3,
-           reason: :responsible
+           reason: :responsible)
   end
 
   let(:notification4) do
-    create :notification,
+    create(:notification,
            recipient:,
            project: project3,
            resource: work_package4,
-           reason: :mentioned
+           reason: :mentioned)
+  end
+
+  let(:notification5) do
+    create(:notification,
+           recipient:,
+           project: project3,
+           resource: work_package5,
+           reason: :date_alert_start_date)
   end
 
   let(:notifications) do
-    [notification, notification2, notification3, notification4]
+    [notification, notification2, notification3, notification4, notification5]
   end
 
   let(:center) { ::Pages::Notifications::Center.new }
@@ -77,6 +86,7 @@ describe "Notification center sidemenu", type: :feature, js: true do
       side_menu.expect_item_with_no_count 'Mentioned'
       side_menu.expect_item_with_no_count 'Accountable'
       side_menu.expect_item_with_no_count 'Watcher'
+      side_menu.expect_item_with_no_count 'Date alert'
     end
   end
 
@@ -84,31 +94,33 @@ describe "Notification center sidemenu", type: :feature, js: true do
     side_menu.expect_open
 
     # Expect standard filters
-    side_menu.expect_item_with_count 'Inbox', 4
+    side_menu.expect_item_with_count 'Inbox', 5
     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
 
     # 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}", 2
+    side_menu.expect_item_with_count "... #{project3.name}", 3
 
     # Reading a notification...
     center.mark_notification_as_read notification
 
     # ...  will change the filter counts
-    side_menu.expect_item_with_count 'Inbox', 3
+    side_menu.expect_item_with_count 'Inbox', 4
     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_no_count 'Watcher'
 
     # ... and show only those projects with a notification
     side_menu.expect_item_not_visible project.name
     side_menu.expect_item_with_count project2.name, 1
-    side_menu.expect_item_with_count "... #{project3.name}", 2
+    side_menu.expect_item_with_count "... #{project3.name}", 3
 
     # Empty filter sets have a separate message
     side_menu.click_item 'Watcher'
@@ -124,6 +136,7 @@ describe "Notification center sidemenu", type: :feature, js: true do
     side_menu.expect_item_with_no_count 'Mentioned'
     side_menu.expect_item_with_no_count 'Accountable'
     side_menu.expect_item_with_no_count 'Watcher'
+    side_menu.expect_item_with_no_count 'Date alert'
 
     side_menu.expect_item_not_visible project.name
     side_menu.expect_item_not_visible project2.name
@@ -132,47 +145,53 @@ describe "Notification center sidemenu", type: :feature, js: true do
 
   it 'updates the content when a filter is clicked' do
     # All notifications are shown
-    center.expect_work_package_item notification, notification2, notification3, notification4
+    center.expect_work_package_item *notifications
 
     # Filter for "Watcher"
     side_menu.click_item 'Watcher'
     side_menu.finished_loading
     center.expect_work_package_item notification
-    center.expect_no_item notification2, notification3, notification4
+    center.expect_no_item notification2, notification3, notification4, notification5
 
     # Filter for "Assignee"
     side_menu.click_item 'Assignee'
     side_menu.finished_loading
     center.expect_work_package_item notification2
-    center.expect_no_item notification, notification3, notification4
+    center.expect_no_item notification, notification3, notification4, notification5
 
     # Filter for "Accountable"
     side_menu.click_item 'Accountable'
     side_menu.finished_loading
     center.expect_work_package_item notification3
-    center.expect_no_item notification, notification2, notification4
+    center.expect_no_item notification, notification2, notification4, notification5
 
     # Filter for "Mentioned"
     side_menu.click_item 'Mentioned'
     side_menu.finished_loading
     center.expect_work_package_item notification4
-    center.expect_no_item notification, notification2, notification3
+    center.expect_no_item notification, notification2, notification3, notification5
+
+    # Filter for "Date alert"
+    side_menu.click_item 'Date alert'
+    side_menu.finished_loading
+    center.expect_work_package_item notification5
+    center.expect_no_item notification, notification2, notification3, notification4
 
     # Filter for project1
     side_menu.click_item project.name
     side_menu.finished_loading
     center.expect_work_package_item notification
-    center.expect_no_item notification2, notification3, notification4
+    center.expect_no_item notification2, notification3, notification4, notification5
 
     # Filter for project3
     side_menu.click_item "... #{project3.name}"
     side_menu.finished_loading
-    center.expect_work_package_item notification3, notification4
+    center.expect_work_package_item notification3, notification4, notification5
     center.expect_no_item notification, notification2
 
     # Reset by clicking on the Inbox
     side_menu.click_item 'Inbox'
     side_menu.finished_loading
-    center.expect_work_package_item notification, notification2, notification3, notification4
+    center.expect_work_package_item *notifications
   end
 end

spec/lib/api/v3/notifications/property_factory_spec.rb

@@ -167,4 +167,44 @@ describe ::API::V3::Notifications::PropertyFactory do
       end
     end
   end
+
+  describe '.groups_for' do
+    context 'with a date_alert_start_date notification' do
+      let(:group_values) { { "date_alert_start_date" => 1 } }
+
+      it 'returns as a date alert' do
+        expect(described_class.groups_for(group_values))
+          .to eq({ "dateAlert" => 1 })
+      end
+    end
+
+    context 'for a date_alert_due_date notification' do
+      let(:group_values) { { "date_alert_due_date" => 1 } }
+
+      it 'returns as a date alert' do
+        expect(described_class.groups_for(group_values))
+          .to eq({ "dateAlert" => 1 })
+      end
+    end
+
+    context 'for multiple notifications' do
+      let(:group_values) { { "mentioned" => 1, "date_alert_due_date" => 2, "date_alert_start_date" => 1 } }
+
+      it 'sums up date alerts' do
+        expect(described_class.groups_for(group_values))
+          .to eq({ "mentioned" => 1, "dateAlert" => 3 })
+      end
+    end
+
+    (Notification::REASONS.keys - %i[date_alert_start_date date_alert_due_date]).each do |possible_reason|
+      context "for a #{possible_reason} notification" do
+        let(:group_values) { { possible_reason => 1 } }
+
+        it 'returns the unaltered reasons' do
+          expect(described_class.groups_for(group_values))
+            .to eq group_values
+        end
+      end
+    end
+  end
 end

spec/requests/api/v3/notifications/index_resource_spec.rb

@@ -29,30 +29,28 @@ require 'spec_helper'
 require 'rack/test'
 
 describe ::API::V3::Notifications::NotificationsAPI,
-         'index',
-         type: :request,
-         content_type: :json do
+         'index', content_type: :json do
   include API::V3::Utilities::PathHelper
 
-  shared_let(:work_package) { create :work_package }
+  shared_let(:work_package) { create(:work_package) }
   shared_let(:recipient) do
-    create :user,
+    create(:user,
            member_in_project: work_package.project,
-           member_with_permissions: %i[view_work_packages]
+           member_with_permissions: %i[view_work_packages])
   end
   shared_let(:notification1) do
-    create :notification,
+    create(:notification,
            recipient:,
            resource: work_package,
            project: work_package.project,
-           journal: work_package.journals.first
+           journal: work_package.journals.first)
   end
   shared_let(:notification2) do
-    create :notification,
+    create(:notification,
            recipient:,
            resource: work_package,
            project: work_package.project,
-           journal: work_package.journals.first
+           journal: work_package.journals.first)
   end
 
   let(:notifications) { [notification1, notification2] }
@@ -83,7 +81,7 @@ describe ::API::V3::Notifications::NotificationsAPI,
     it_behaves_like 'API V3 collection response', 2, 2, 'Notification'
 
     context 'with a readIAN filter' do
-      let(:nil_notification) { create :notification, recipient:, read_ian: nil }
+      let(:nil_notification) { create(:notification, recipient:, read_ian: nil) }
 
       let(:notifications) { [notification1, notification2, nil_notification] }
 
@@ -107,7 +105,7 @@ describe ::API::V3::Notifications::NotificationsAPI,
     end
 
     context 'with a resource filter' do
-      let(:notification3) { create :notification, recipient: }
+      let(:notification3) { create(:notification, recipient:) }
       let(:notifications) { [notification1, notification2, notification3] }
 
       let(:filters) do
@@ -137,10 +135,10 @@ describe ::API::V3::Notifications::NotificationsAPI,
     context 'with a project filter' do
       let(:other_work_package) { create(:work_package) }
       let(:notification3) do
-        create :notification,
+        create(:notification,
                recipient:,
                resource: other_work_package,
-               project: other_work_package.project
+               project: other_work_package.project)
       end
       let(:notifications) { [notification1, notification2, notification3] }
 
@@ -162,20 +160,20 @@ describe ::API::V3::Notifications::NotificationsAPI,
 
     context 'with a reason filter' do
       let(:notification3) do
-        create :notification,
+        create(:notification,
                reason: :assigned,
                recipient:,
                resource: work_package,
                project: work_package.project,
-               journal: work_package.journals.first
+               journal: work_package.journals.first)
       end
       let(:notification4) do
-        create :notification,
+        create(:notification,
                reason: :responsible,
                recipient:,
                resource: work_package,
                project: work_package.project,
-               journal: work_package.journals.first
+               journal: work_package.journals.first)
       end
       let(:notifications) { [notification1, notification2, notification3, notification4] }
 
@@ -221,12 +219,12 @@ describe ::API::V3::Notifications::NotificationsAPI,
       let(:wiki_page) { create(:wiki_page_with_content) }
 
       let(:non_ian_notification) do
-        create :notification,
+        create(:notification,
                read_ian: nil,
                recipient:,
                resource: wiki_page,
                project: wiki_page.wiki.project,
-               journal: wiki_page.content.journals.first
+               journal: wiki_page.content.journals.first)
       end
 
       let(:notifications) { [notification2, notification1, non_ian_notification] }
@@ -238,15 +236,33 @@ describe ::API::V3::Notifications::NotificationsAPI,
 
     context 'with a reason groupBy' do
       let(:responsible_notification) do
-        create :notification,
+        create(:notification,
                recipient:,
                reason: :responsible,
                resource: work_package,
                project: work_package.project,
-               journal: work_package.journals.first
+               journal: work_package.journals.first)
       end
 
-      let(:notifications) { [notification1, notification2, responsible_notification] }
+      let(:start_date_notification) do
+        create(:notification,
+               recipient:,
+               reason: :date_alert_start_date,
+               resource: work_package,
+               project: work_package.project)
+      end
+
+      let(:due_date_notification) do
+        create(:notification,
+               recipient:,
+               reason: :date_alert_due_date,
+               resource: work_package,
+               project: work_package.project)
+      end
+
+      let(:notifications) do
+        [notification1, notification2, responsible_notification, start_date_notification, due_date_notification]
+      end
 
       let(:send_request) do
         get api_v3_paths.path_for :notifications, group_by: :reason
@@ -254,16 +270,17 @@ describe ::API::V3::Notifications::NotificationsAPI,
 
       let(:groups) { parsed_response['groups'] }
 
-      it_behaves_like 'API V3 collection response', 3, 3, 'Notification'
+      it_behaves_like 'API V3 collection response', 5, 5, 'Notification'
 
       it 'contains the reason groups', :aggregate_failures do
         expect(groups).to be_a Array
-        expect(groups.count).to eq 2
+        expect(groups.count).to eq 3
 
         keyed = groups.index_by { |el| el['value'] }
-        expect(keyed.keys).to contain_exactly 'mentioned', 'responsible'
+        expect(keyed.keys).to contain_exactly 'mentioned', 'responsible', 'dateAlert'
         expect(keyed['mentioned']['count']).to eq 2
         expect(keyed['responsible']['count']).to eq 1
+        expect(keyed['dateAlert']['count']).to eq 2
       end
     end
 
@@ -272,14 +289,14 @@ describe ::API::V3::Notifications::NotificationsAPI,
         create(:project,
                members: { recipient => recipient.members.first.roles })
       end
-      let(:work_package2) { create :work_package, project: other_project }
+      let(:work_package2) { create(:work_package, project: other_project) }
       let(:other_project_notification) do
-        create :notification,
+        create(:notification,
                resource: work_package2,
                project: other_project,
                recipient:,
                reason: :responsible,
-               journal: work_package2.journals.first
+               journal: work_package2.journals.first)
       end
 
       let(:notifications) { [notification1, notification2, other_project_notification] }