diff --git a/frontend/src/app/core/state/collection-store.ts b/frontend/src/app/core/state/collection-store.ts index 3a7a817861..d837c15225 100644 --- a/frontend/src/app/core/state/collection-store.ts +++ b/frontend/src/app/core/state/collection-store.ts @@ -70,22 +70,15 @@ export function selectCollectionAsHrefs$(service:Colle * Retrieve the entities from the collection a given parameter set produces. * * @param service + * @param state * @param params */ -export function selectCollectionAsEntities$(service:CollectionService, params:Apiv3ListParameters):Observable { +export function selectCollectionAsEntities$(service:CollectionService, state:CollectionState, 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[]; } diff --git a/frontend/src/app/features/in-app-notifications/center/in-app-notification-center.component.ts b/frontend/src/app/features/in-app-notifications/center/in-app-notification-center.component.ts index 5b7a6b5698..46c2ead821 100644 --- a/frontend/src/app/features/in-app-notifications/center/in-app-notification-center.component.ts +++ b/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 { diff --git a/frontend/src/app/features/in-app-notifications/center/state/ian-center.query.ts b/frontend/src/app/features/in-app-notifications/center/state/ian-center.query.ts index ea6bdb609b..b0cd25ed31 100644 --- a/frontend/src/app/features/in-app-notifications/center/state/ian-center.query.ts +++ b/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 { paramsChanges$ = this.select(['params', 'activeFacet']); - selectNotifications$ = this - .paramsChanges$ - .pipe( - switchMap(() => selectCollectionAsEntities$(this.resourceService, this.params)), - ); + selectNotifications$ = combineLatest([ + this.paramsChanges$, + this.resourceService.query.select(), + ]).pipe( + map(([, state]) => selectCollectionAsEntities$(this.resourceService, state, this.params)), + ); aggregatedCenterNotifications$ = this .selectNotifications$ 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 aec31b913c..304c86be0a 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 @@ -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) { - 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 { @@ -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; + } + } + }); + } } diff --git a/frontend/src/app/features/in-app-notifications/entry/in-app-notification-entry.component.ts b/frontend/src/app/features/in-app-notifications/entry/in-app-notification-entry.component.ts index d3b0e7f0e6..169739f4b4 100644 --- a/frontend/src/app/features/in-app-notifications/entry/in-app-notification-entry.component.ts +++ b/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); }); } diff --git a/frontend/src/app/features/work-packages/routing/wp-view-base/state/wp-single-view.query.ts b/frontend/src/app/features/work-packages/routing/wp-view-base/state/wp-single-view.query.ts index 09dfcefd84..856be08025 100644 --- a/frontend/src/app/features/work-packages/routing/wp-view-base/state/wp-single-view.query.ts +++ b/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 { - selectNotifications$ = this - .select((state) => state.notifications.filters) - .pipe( - filter((filters) => filters.length > 0), - switchMap((filters) => selectCollectionAsEntities$(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$(this.resourceService, state, { filters })), + ); selectNotificationsCount$ = this .selectNotifications$ diff --git a/spec/features/notifications/notification_center/notification_center_spec.rb b/spec/features/notifications/notification_center/notification_center_spec.rb index 550344bc28..d4cdaf1181 100644 --- a/spec/features/notifications/notification_center/notification_center_spec.rb +++ b/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] diff --git a/spec/support/pages/work_packages/abstract_work_package.rb b/spec/support/pages/work_packages/abstract_work_package.rb index cac61bbcea..88b42b630a 100644 --- a/spec/support/pages/work_packages/abstract_work_package.rb +++ b/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)