From 537bf240f12e1ae1cebcae84322785b53c3456c1 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Wed, 12 Nov 2025 23:01:16 -0500 Subject: [PATCH 1/4] fix(notifications): change settings page to default to correct notif --- .../notifications/notifications.component.ts | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/src/app/features/settings/notifications/notifications.component.ts b/src/app/features/settings/notifications/notifications.component.ts index 6a38717e6..aa0194687 100644 --- a/src/app/features/settings/notifications/notifications.component.ts +++ b/src/app/features/settings/notifications/notifications.component.ts @@ -77,13 +77,25 @@ export class NotificationsComponent implements OnInit { notificationSubscriptionsForm = this.fb.group( SUBSCRIPTION_EVENTS.reduce( (control, { event }) => { - control[event] = this.fb.control(SubscriptionFrequency.Never, { nonNullable: true }); + control[event as string] = this.fb.control(SubscriptionFrequency.Never, { + nonNullable: true, + }); return control; }, {} as Record> ) ); + private readonly API_EVENT_TO_FORM_EVENT: Record = { + new_pending_submissions: 'global_reviews', + files_updated: 'global_file_updated', + }; + + private readonly FORM_EVENT_TO_API_EVENT: Record = { + global_reviews: 'new_pending_submissions', + global_file_updated: 'files_updated', + }; + constructor() { effect(() => { if (this.emailPreferences()) { @@ -125,7 +137,24 @@ export class NotificationsComponent implements OnInit { onSubscriptionChange(event: SubscriptionEvent, frequency: SubscriptionFrequency) { const user = this.currentUser(); if (!user) return; - const id = `${user.id}_${event}`; + + const eventKey = event as unknown as string; + + const apiEventName = this.FORM_EVENT_TO_API_EVENT[eventKey] ?? eventKey; + + let id: string | undefined; + + if (event === SubscriptionEvent.GlobalReviews) { + const subs = this.notificationSubscriptions(); + const match = subs.find((s) => (s.event as string) === 'new_pending_submissions'); + if (match) { + id = match.id; + } else { + return; + } + } else { + id = `${user.id}_${apiEventName}`; + } this.loaderService.show(); this.actions.updateNotificationSubscription({ id, frequency }).subscribe(() => { @@ -142,10 +171,24 @@ export class NotificationsComponent implements OnInit { } private updateNotificationSubscriptionsForm() { + const subs = this.notificationSubscriptions(); + if (!subs?.length) { + return; + } + const patch: Record = {}; - for (const sub of this.notificationSubscriptions()) { - patch[sub.event] = sub.frequency; + for (const sub of subs) { + const apiEvent = sub.event as string | null; + if (!apiEvent) { + continue; + } + + const formEventKey = this.API_EVENT_TO_FORM_EVENT[apiEvent]; + + if (formEventKey) { + patch[formEventKey] = sub.frequency; + } } this.notificationSubscriptionsForm.patchValue(patch); From 46b178746dd89a9c97ada64198f3e7614903fb9e Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 08:39:17 -0500 Subject: [PATCH 2/4] fix(tests): add new tests and changes for existing behavior --- .../notifications.component.spec.ts | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/app/features/settings/notifications/notifications.component.spec.ts b/src/app/features/settings/notifications/notifications.component.spec.ts index f05761011..b47bc6cb9 100644 --- a/src/app/features/settings/notifications/notifications.component.spec.ts +++ b/src/app/features/settings/notifications/notifications.component.spec.ts @@ -1,7 +1,7 @@ import { Store } from '@ngxs/store'; import { TranslatePipe } from '@ngx-translate/core'; -import { MockPipe, MockProvider } from 'ng-mocks'; +import { MockComponents, MockPipe, MockProvider } from 'ng-mocks'; import { of } from 'rxjs'; @@ -9,11 +9,15 @@ import { provideHttpClient } from '@angular/common/http'; import { provideHttpClientTesting } from '@angular/common/http/testing'; import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { FormBuilder, ReactiveFormsModule } from '@angular/forms'; +import { FormBuilder } from '@angular/forms'; import { UserSelectors } from '@osf/core/store/user'; -import { LoaderService, ToastService } from '@osf/shared/services'; -import { SubscriptionEvent, SubscriptionFrequency } from '@shared/enums'; +import { InfoIconComponent } from '@osf/shared/components/info-icon/info-icon.component'; +import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; +import { SubscriptionEvent } from '@osf/shared/enums/subscriptions/subscription-event.enum'; +import { SubscriptionFrequency } from '@osf/shared/enums/subscriptions/subscription-frequency.enum'; +import { LoaderService } from '@osf/shared/services/loader.service'; +import { ToastService } from '@osf/shared/services/toast.service'; import { AccountSettings } from '../account-settings/models'; import { AccountSettingsSelectors } from '../account-settings/store'; @@ -21,7 +25,9 @@ import { AccountSettingsSelectors } from '../account-settings/store'; import { NotificationsComponent } from './notifications.component'; import { NotificationSubscriptionSelectors } from './store'; -import { MOCK_STORE, MOCK_USER, TranslateServiceMock } from '@testing/mocks'; +import { MOCK_USER } from '@testing/mocks/data.mock'; +import { MOCK_STORE } from '@testing/mocks/mock-store.mock'; +import { TranslateServiceMock } from '@testing/mocks/translate.service.mock'; import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; describe('NotificationsComponent', () => { @@ -35,13 +41,19 @@ describe('NotificationsComponent', () => { subscribeOsfHelpEmail: false, }; + // new_pending_submissions → global_reviews + // files_updated → global_file_updated const mockNotificationSubscriptions = [ - { id: 'id1', event: SubscriptionEvent.GlobalMentions, frequency: SubscriptionFrequency.Daily }, { - id: 'id2', - event: SubscriptionEvent.GlobalMentions, + id: 'osf_new_pending_submissions', + event: 'new_pending_submissions', frequency: SubscriptionFrequency.Instant, }, + { + id: 'cuzg4_global_file_updated', + event: 'files_updated', + frequency: SubscriptionFrequency.Daily, + }, ]; beforeEach(async () => { @@ -71,10 +83,14 @@ describe('NotificationsComponent', () => { return signal(null); }); - MOCK_STORE.dispatch.mockImplementation(() => of()); + MOCK_STORE.dispatch.mockReturnValue(of({})); await TestBed.configureTestingModule({ - imports: [NotificationsComponent, MockPipe(TranslatePipe), ReactiveFormsModule], + imports: [ + NotificationsComponent, + ...MockComponents(InfoIconComponent, SubHeaderComponent), + MockPipe(TranslatePipe), + ], providers: [ provideHttpClient(), provideHttpClientTesting(), @@ -106,9 +122,9 @@ describe('NotificationsComponent', () => { return signal(null); }); - component.emailPreferencesFormSubmit(); - expect(loaderService.hide).not.toHaveBeenCalled(); + component.emailPreferencesFormSubmit(); + expect(loaderService.hide).toHaveBeenCalledTimes(1); }); it('should handle subscription completion correctly', () => { @@ -126,11 +142,17 @@ describe('NotificationsComponent', () => { it('should call dispatch only once per subscription change', () => { const mockDispatch = jest.fn().mockReturnValue(of({})); MOCK_STORE.dispatch.mockImplementation(mockDispatch); - const event = SubscriptionEvent.GlobalMentions; - const frequency = SubscriptionFrequency.Daily; - component.onSubscriptionChange(event, frequency); + component.onSubscriptionChange(SubscriptionEvent.GlobalFileUpdated, SubscriptionFrequency.Daily); expect(mockDispatch).toHaveBeenCalledTimes(1); }); + + it('should default to API value', () => { + const subs = component.notificationSubscriptionsForm.value; + + expect(subs.global_reviews).toBe(SubscriptionFrequency.Instant); + + expect(subs.global_file_updated).toBe(SubscriptionFrequency.Daily); + }); }); From 171177b95809fdccbc37bf57b27a97a5c4ed5b31 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 13:09:35 -0500 Subject: [PATCH 3/4] fix(nofitifcations): Move constants for notifications into constants and update SubscriptionEvent --- .../constants/notifications-constants.ts | 11 +++++++++++ .../notifications/notifications.component.ts | 16 +++------------- .../subscriptions/subscription-event.enum.ts | 3 +-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/app/features/settings/notifications/constants/notifications-constants.ts b/src/app/features/settings/notifications/constants/notifications-constants.ts index 943f9a041..438c3c545 100644 --- a/src/app/features/settings/notifications/constants/notifications-constants.ts +++ b/src/app/features/settings/notifications/constants/notifications-constants.ts @@ -12,3 +12,14 @@ export const SUBSCRIPTION_EVENTS: SubscriptionEventModel[] = [ labelKey: 'settings.notifications.notificationPreferences.items.preprints', }, ]; + +export const FORM_EVENT_TO_API_EVENT: Record = { + new_pending_submissions: 'new_pending_submissions', + files_updated: 'files_updated', + global_file_updated: 'global_file_updated', +}; + +export const API_EVENT_TO_FORM_EVENT: Record = { + new_pending_submissions: 'new_pending_submissions', + files_updated: 'global_file_updated', +}; diff --git a/src/app/features/settings/notifications/notifications.component.ts b/src/app/features/settings/notifications/notifications.component.ts index aa0194687..8e55541e8 100644 --- a/src/app/features/settings/notifications/notifications.component.ts +++ b/src/app/features/settings/notifications/notifications.component.ts @@ -18,7 +18,7 @@ import { LoaderService, ToastService } from '@osf/shared/services'; import { AccountSettings } from '../account-settings/models'; import { AccountSettingsSelectors, GetAccountSettings, UpdateAccountSettings } from '../account-settings/store'; -import { SUBSCRIPTION_EVENTS } from './constants'; +import { API_EVENT_TO_FORM_EVENT, FORM_EVENT_TO_API_EVENT, SUBSCRIPTION_EVENTS } from './constants'; import { EmailPreferencesForm, EmailPreferencesFormControls } from './models'; import { GetAllGlobalNotificationSubscriptions, @@ -86,16 +86,6 @@ export class NotificationsComponent implements OnInit { ) ); - private readonly API_EVENT_TO_FORM_EVENT: Record = { - new_pending_submissions: 'global_reviews', - files_updated: 'global_file_updated', - }; - - private readonly FORM_EVENT_TO_API_EVENT: Record = { - global_reviews: 'new_pending_submissions', - global_file_updated: 'files_updated', - }; - constructor() { effect(() => { if (this.emailPreferences()) { @@ -140,7 +130,7 @@ export class NotificationsComponent implements OnInit { const eventKey = event as unknown as string; - const apiEventName = this.FORM_EVENT_TO_API_EVENT[eventKey] ?? eventKey; + const apiEventName = FORM_EVENT_TO_API_EVENT[eventKey] ?? eventKey; let id: string | undefined; @@ -184,7 +174,7 @@ export class NotificationsComponent implements OnInit { continue; } - const formEventKey = this.API_EVENT_TO_FORM_EVENT[apiEvent]; + const formEventKey = API_EVENT_TO_FORM_EVENT[apiEvent]; if (formEventKey) { patch[formEventKey] = sub.frequency; diff --git a/src/app/shared/enums/subscriptions/subscription-event.enum.ts b/src/app/shared/enums/subscriptions/subscription-event.enum.ts index f3913133f..c98ba610a 100644 --- a/src/app/shared/enums/subscriptions/subscription-event.enum.ts +++ b/src/app/shared/enums/subscriptions/subscription-event.enum.ts @@ -1,6 +1,5 @@ export enum SubscriptionEvent { GlobalFileUpdated = 'global_file_updated', - GlobalMentions = 'global_mentions', - GlobalReviews = 'global_reviews', + GlobalReviews = 'new_pending_submissions', FileUpdated = 'file_updated', } From f4d5ca45cd5141da412c0ff12727404d1333d6f4 Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Thu, 13 Nov 2025 13:27:56 -0500 Subject: [PATCH 4/4] fix(test): test behavior change issue --- .../settings/notifications/notifications.component.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/features/settings/notifications/notifications.component.spec.ts b/src/app/features/settings/notifications/notifications.component.spec.ts index b47bc6cb9..03950cb0c 100644 --- a/src/app/features/settings/notifications/notifications.component.spec.ts +++ b/src/app/features/settings/notifications/notifications.component.spec.ts @@ -150,9 +150,7 @@ describe('NotificationsComponent', () => { it('should default to API value', () => { const subs = component.notificationSubscriptionsForm.value; - - expect(subs.global_reviews).toBe(SubscriptionFrequency.Instant); - + expect(subs.new_pending_submissions).toBe(SubscriptionFrequency.Instant); expect(subs.global_file_updated).toBe(SubscriptionFrequency.Daily); }); });