Merge pull request #9696 from opf/automatically-open-next-notification

[38129]Automatically open the next notification after reading another notification

frontend/src/app/core/state/collection-store.ts

@@ -70,22 +70,15 @@ export function selectCollectionAsHrefs$<T extends CollectionItem>(service:Colle
  * Retrieve the entities from the collection a given parameter set produces.
  *
  * @param service
+ * @param state
  * @param params
  */
-export function selectCollectionAsEntities$<T extends CollectionItem>(service:CollectionService<T>, params:Apiv3ListParameters):Observable<T[]> {
+export function selectCollectionAsEntities$<T extends CollectionItem>(service:CollectionService<T>, state:CollectionState<T>, params:Apiv3ListParameters):T[] {
   const key = collectionKey(params);
+  const collection = state.collections[key];
+  const ids = collection?.ids || [];
 
-  return service
-    .query
-    .select()
-    .pipe(
-      map((state) => {
-        const collection = state.collections[key];
-        const ids = collection?.ids || [];
-
-        return ids
-          .map((id) => service.query.getEntity(id))
-          .filter((item) => !!item) as T[];
-      }),
-    );
+  return ids
+    .map((id:string) => service.query.getEntity(id))
+    .filter((item) => !!item) as T[];
 }

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

@@ -7,21 +7,15 @@ import {
 } from '@angular/core';
 import { I18nService } from 'core-app/core/i18n/i18n.service';
 import {
-  distinctUntilChanged,
   filter,
   map,
-  pluck,
-  share,
 } from 'rxjs/operators';
-import { StateService } from '@uirouter/angular';
-import { UIRouterGlobals } from '@uirouter/core';
 import { IanCenterService } from 'core-app/features/in-app-notifications/center/state/ian-center.service';
 import {
   InAppNotification,
   NOTIFICATIONS_MAX_SIZE,
 } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
-import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin';
-import { APIV3Service } from 'core-app/core/apiv3/api-v3.service';
+import { UIRouterGlobals } from '@uirouter/core';
 
 @Component({
   selector: 'op-in-app-notification-center',
@@ -29,7 +23,7 @@ import { APIV3Service } from 'core-app/core/apiv3/api-v3.service';
   styleUrls: ['./in-app-notification-center.component.sass'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class InAppNotificationCenterComponent extends UntilDestroyedMixin implements OnInit {
+export class InAppNotificationCenterComponent implements OnInit {
   maxSize = NOTIFICATIONS_MAX_SIZE;
 
   hasMoreThanPageSize$ = this.storeService.query.hasMoreThanPageSize$;
@@ -60,13 +54,7 @@ export class InAppNotificationCenterComponent extends UntilDestroyedMixin implem
       )),
     );
 
-  stateChanged$ = this.uiRouterGlobals.params$?.pipe(
-    this.untilDestroyed(),
-    pluck('workPackageId'),
-    distinctUntilChanged(),
-    map((workPackageId:string) => (workPackageId ? this.apiV3.work_packages.id(workPackageId).path : undefined)),
-    share(),
-  );
+  stateChanged$ = this.storeService.stateChanged$;
 
   originalOrder = ():number => 0;
 
@@ -83,18 +71,13 @@ export class InAppNotificationCenterComponent extends UntilDestroyedMixin implem
     },
   };
 
-  selectedNotification:InAppNotification|undefined;
-
   constructor(
     readonly cdRef:ChangeDetectorRef,
     readonly elementRef:ElementRef,
     readonly I18n:I18nService,
     readonly storeService:IanCenterService,
     readonly uiRouterGlobals:UIRouterGlobals,
-    readonly state:StateService,
-    readonly apiV3:APIV3Service,
   ) {
-    super();
   }
 
   ngOnInit():void {

frontend/src/app/features/in-app-notifications/center/state/ian-center.query.ts

@@ -1,8 +1,6 @@
 import { Query } from '@datorama/akita';
-import {
-  map,
-  switchMap,
-} from 'rxjs/operators';
+import { map } from 'rxjs/operators';
+import { combineLatest } from 'rxjs';
 import {
   IAN_FACET_FILTERS,
   IanCenterState,
@@ -23,11 +21,12 @@ export class IanCenterQuery extends Query<IanCenterState> {
 
   paramsChanges$ = this.select(['params', 'activeFacet']);
 
-  selectNotifications$ = this
-    .paramsChanges$
-    .pipe(
-      switchMap(() => selectCollectionAsEntities$<InAppNotification>(this.resourceService, this.params)),
-    );
+  selectNotifications$ = combineLatest([
+    this.paramsChanges$,
+    this.resourceService.query.select(),
+  ]).pipe(
+    map(([, state]) => selectCollectionAsEntities$<InAppNotification>(this.resourceService, state, this.params)),
+  );
 
   aggregatedCenterNotifications$ = this
     .selectNotifications$

frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts

@@ -3,7 +3,10 @@ import {
   Injector,
 } from '@angular/core';
 import {
+  distinctUntilChanged,
   map,
+  pluck,
+  share,
   switchMap,
   take,
 } from 'rxjs/operators';
@@ -32,22 +35,45 @@ import {
   IanCenterStore,
   InAppNotificationFacet,
 } from './ian-center.store';
+import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin';
+import { UIRouterGlobals } from '@uirouter/core';
+import { StateService } from '@uirouter/angular';
+import idFromLink from 'core-app/features/hal/helpers/id-from-link';
 
 @Injectable()
 @EffectHandler
-export class IanCenterService {
+export class IanCenterService extends UntilDestroyedMixin {
   readonly id = 'ian-center';
 
   readonly store = new IanCenterStore();
 
   readonly query = new IanCenterQuery(this.store, this.resourceService);
 
+  public selectedNotificationIndex = 0;
+
+  stateChanged$ = this.uiRouterGlobals.params$?.pipe(
+    this.untilDestroyed(),
+    pluck('workPackageId'),
+    distinctUntilChanged(),
+    map((workPackageId:string) => (workPackageId ? this.apiV3Service.work_packages.id(workPackageId).path : undefined)),
+    share(),
+  );
+
   constructor(
     readonly injector:Injector,
     readonly resourceService:InAppNotificationsResourceService,
     readonly actions$:ActionsService,
     readonly apiV3Service:APIV3Service,
+    readonly uiRouterGlobals:UIRouterGlobals,
+    readonly state:StateService,
   ) {
+    super();
+
+    if (this.stateChanged$) {
+      this.stateChanged$.subscribe(() => {
+        this.updateSelectedNotificationIndex();
+      });
+    }
   }
 
   setFilters(filters:INotificationPageQueryParameters):void {
@@ -76,30 +102,57 @@ export class IanCenterService {
       });
   }
 
+  openSplitScreen(wpId:string|null):void {
+    void this.state.go(
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/restrict-template-expressions
+      `${this.state.current.data.baseRoute}.details.tabs`,
+      { workPackageId: wpId, tabIdentifier: 'activity' },
+    );
+  }
+
+  showNextNotification():void {
+    void this
+      .query
+      .notifications$
+      .pipe(
+        take(1),
+      ).subscribe((notifications:InAppNotification[][]) => {
+        if (notifications.length <= 0) {
+          void this.state.go(
+            // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/restrict-template-expressions
+            `${this.state.current.data.baseRoute}`,
+          );
+          return;
+        }
+        if (notifications[0][0]._links.resource || notifications[this.selectedNotificationIndex][0]._links.resource) {
+          const wpId = idFromLink(notifications[this.selectedNotificationIndex >= notifications.length ? 0 : this.selectedNotificationIndex][0]._links.resource.href);
+          this.openSplitScreen(wpId);
+        }
+      });
+  }
+
   /**
    * Reload after notifications were successfully marked as read
    */
   @EffectCallback(notificationsMarkedRead)
   private reloadOnNotificationRead(action:ReturnType<typeof notificationsMarkedRead>) {
-    if (action.origin === this.id) {
-      this
-        .resourceService
-        .removeFromCollection(this.query.params, action.notifications);
-    } else {
-      this.debouncedReload();
-    }
+    this
+      .resourceService
+      .removeFromCollection(this.query.params, action.notifications);
+    this.showNextNotification();
   }
 
-  private debouncedReload = _.debounce(() => { this.reload(); });
+  private debouncedReload = _.debounce(() => { this.reload().subscribe(); });
 
   private reload() {
-    this.resourceService
+    return this.resourceService
       .fetchNotifications(this.query.params)
       .pipe(
         setLoading(this.store),
         switchMap((results) => from(this.sideLoadInvolvedWorkPackages(results._embedded.elements))),
-      )
-      .subscribe();
+        switchMap(() => this.query.notifications$),
+        take(1),
+      );
   }
 
   private sideLoadInvolvedWorkPackages(elements:InAppNotification[]):Promise<unknown> {
@@ -126,4 +179,21 @@ export class IanCenterService {
 
     return promise;
   }
+
+  private updateSelectedNotificationIndex() {
+    this
+      .query
+      .notifications$
+      .pipe(
+        take(1),
+      ).subscribe((notifications:InAppNotification[][]) => {
+        for (let i = 0; i < notifications.length; ++i) {
+          if (notifications[i][0]._links.resource
+            && idFromLink(notifications[i][0]._links.resource.href) === this.uiRouterGlobals.params.workPackageId) {
+            this.selectedNotificationIndex = i;
+            return;
+          }
+        }
+      });
+  }
 }

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

@@ -123,10 +123,7 @@ export class InAppNotificationEntryComponent implements OnInit {
         take(1),
       )
       .subscribe((wp) => {
-        void this.state.go(
-          `${(this.state.current.data as BackRouteOptions).baseRoute}.details.tabs`,
-          { workPackageId: wp.id, tabIdentifier: 'activity' },
-        );
+        this.storeService.openSplitScreen(wp.id);
       });
   }
 

frontend/src/app/features/work-packages/routing/wp-view-base/state/wp-single-view.query.ts

@@ -6,20 +6,21 @@ import {
 import {
   filter,
   map,
-  switchMap,
 } from 'rxjs/operators';
 import { selectCollectionAsEntities$ } from 'core-app/core/state/collection-store';
 import { InAppNotification } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
 import { InAppNotificationsResourceService } from 'core-app/core/state/in-app-notifications/in-app-notifications.service';
 import { Apiv3ListParameters } from 'core-app/core/apiv3/paths/apiv3-list-resource.interface';
+import { combineLatest } from 'rxjs';
 
 export class WpSingleViewQuery extends Query<WpSingleViewState> {
-  selectNotifications$ = this
-    .select((state) => state.notifications.filters)
-    .pipe(
-      filter((filters) => filters.length > 0),
-      switchMap((filters) => selectCollectionAsEntities$<InAppNotification>(this.resourceService, { filters })),
-    );
+  selectNotifications$ = combineLatest([
+    this.select((state) => state.notifications.filters),
+    this.resourceService.query.select(),
+  ]).pipe(
+    filter((filters) => filters.length > 0),
+    map(([filters, state]) => selectCollectionAsEntities$<InAppNotification>(this.resourceService, state, { filters })),
+  );
 
   selectNotificationsCount$ = this
     .selectNotifications$

spec/features/notifications/notification_center/notification_center_spec.rb

@@ -31,6 +31,7 @@ describe "Notification center", type: :feature, js: true, with_settings: { journ
   let(:center) { ::Pages::Notifications::Center.new }
   let(:activity_tab) { ::Components::WorkPackages::Activities.new(work_package) }
   let(:split_screen) { ::Pages::SplitWorkPackage.new work_package }
+  let(:split_screen2) { ::Pages::SplitWorkPackage.new work_package2 }
 
   let(:notifications) do
     [notification, notification2]
@@ -114,6 +115,39 @@ describe "Notification center", type: :feature, js: true, with_settings: { journ
       center.expect_work_package_item notification2
     end
 
+    it 'opens the next notification after marking one as read' do
+      visit home_path
+      center.expect_bell_count 2
+      center.open
+
+      center.click_item notification
+      split_screen.expect_open
+
+      # Marking the first notification as read (via icon on the notification row)
+      center.mark_notification_as_read notification
+      retry_block do
+        notification.reload
+        raise "Expected notification to be marked read" unless notification.read_ian
+      end
+
+      # The second is automatically opened in the split screen
+      split_screen2.expect_open
+
+      # When marking the second as closed (via the icon in the split screen)
+      # the empty state is shown
+      split_screen2.mark_notifications_as_read
+
+      retry_block do
+        notification.reload
+        raise "Expected notification to be marked read" unless notification.read_ian
+      end
+
+      center.expect_no_item notification
+      center.expect_no_item notification2
+
+      center.expect_empty
+    end
+
     context 'with multiple notifications per work package' do
       # In this context we have four notifications for two work packages.
       let(:notification3) do
@@ -130,7 +164,6 @@ describe "Notification center", type: :feature, js: true, with_settings: { journ
         # Will have been created via the JOURNAL_CREATED event listeners
         work_package.journals.last.notifications.first
       end
-      let(:split_screen2) { ::Pages::SplitWorkPackage.new work_package2 }
 
       let(:notifications) do
         [notification, notification2, notification3, notification4]

spec/support/pages/work_packages/abstract_work_package.rb

@@ -285,6 +285,10 @@ module Pages
       find('.work-packages-back-button').click
     end
 
+    def mark_notifications_as_read
+      find('[data-qa-selector="mark-notification-read-button"]').click
+    end
+
     private
 
     def create_page(_args)