diff --git a/src/app/inbox/components/notification-permission-institutional-connection/notification-permission-institutional-connection.component.html b/src/app/inbox/components/notification-permission-institutional-connection/notification-permission-institutional-connection.component.html index a1e1498ece..41288c4a64 100644 --- a/src/app/inbox/components/notification-permission-institutional-connection/notification-permission-institutional-connection.component.html +++ b/src/app/inbox/components/notification-permission-institutional-connection/notification-permission-institutional-connection.component.html @@ -10,7 +10,7 @@ Please complete the process by connecting - {{ notification?.source.sourceName.content }} + {{ notification?.source?.sourceName?.content }} with your ORCID record. @@ -19,7 +19,7 @@ - {{ notification?.source.sourceName.content }} + {{ notification?.source?.sourceName?.content }} would like your permission to interact with your ORCID Record as a trusted party. diff --git a/src/app/inbox/components/notification-your-record-amended/notification-your-record-amended.component.spec.ts b/src/app/inbox/components/notification-your-record-amended/notification-your-record-amended.component.spec.ts index f602825d44..f38abf1599 100644 --- a/src/app/inbox/components/notification-your-record-amended/notification-your-record-amended.component.spec.ts +++ b/src/app/inbox/components/notification-your-record-amended/notification-your-record-amended.component.spec.ts @@ -43,4 +43,21 @@ describe('NotificationYourRecordAmendedComponent', () => { it('should create', () => { expect(component).toBeTruthy() }) + + // A notification raised by ORCID itself carries no member source, and the + // endpoint sends `source: null`. Rendering must not throw: an exception here + // aborts the change-detection pass and takes the rest of the page — the + // global header buttons included — down with it (PD-13322). + it('renders a notification whose source is null', () => { + component.notification = { + notificationType: 'AMENDED', + putCode: 1, + subject: 'Your record was amended', + amendedSection: 'EMPLOYMENT', + source: null, + items: { items: [] }, + } as any + + expect(() => fixture.detectChanges()).not.toThrow() + }) }) diff --git a/src/app/inbox/components/notification/notification.component.spec.ts b/src/app/inbox/components/notification/notification.component.spec.ts index 7e972c5bf9..d359080ee7 100644 --- a/src/app/inbox/components/notification/notification.component.spec.ts +++ b/src/app/inbox/components/notification/notification.component.spec.ts @@ -56,4 +56,49 @@ describe('NotificationComponent', () => { it('should create', () => { expect(component).toBeTruthy() }) + + // `notificationTitle` runs for every row in the inbox, so an unguarded read + // of a null source throws during change detection and takes the whole page + // down with it — the global header buttons included (PD-13322). + describe('with a notification whose source is null', () => { + function notification(notificationType: string) { + return { + notificationType, + putCode: 1, + subject: 'A subject composed by the backend', + source: null, + } as any + } + + it('falls back to the subject for an amended notification', () => { + expect(() => + component.notificationTitle(notification('AMENDED')) + ).not.toThrow() + expect(component.notificationTitle(notification('AMENDED'))).toBe( + 'A subject composed by the backend' + ) + }) + + it('falls back to the subject for an institutional connection', () => { + expect(() => + component.notificationTitle(notification('INSTITUTIONAL_CONNECTION')) + ).not.toThrow() + expect( + component.notificationTitle(notification('INSTITUTIONAL_CONNECTION')) + ).toBe('A subject composed by the backend') + }) + + it('still uses the source name when one is present', () => { + const withSource = { + notificationType: 'AMENDED', + putCode: 1, + subject: 'ignored', + source: { sourceName: { content: 'Example University' } }, + } as any + + expect(component.notificationTitle(withSource)).toContain( + 'Example University' + ) + }) + }) }) diff --git a/src/app/inbox/components/notification/notification.component.ts b/src/app/inbox/components/notification/notification.component.ts index a2de03d750..b019afa5cc 100644 --- a/src/app/inbox/components/notification/notification.component.ts +++ b/src/app/inbox/components/notification/notification.component.ts @@ -131,15 +131,21 @@ export class NotificationComponent } notificationTitle(notification: InboxNotification) { + // A notification can arrive with `source: null` — the endpoint sends it + // even though the type says otherwise. This runs for every row, so an + // unguarded read here throws during change detection and takes down the + // whole page, the global header included (PD-13322). Fall back to the + // subject the backend already composed rather than rendering "undefined". + const sourceName = notification?.source?.sourceName?.content switch (notification?.notificationType) { case 'AMENDED': - return `${ - notification.source.sourceName.content - } ${$localize`:@@inbox.hadMadeChanges:has made changes to your ORCID record`}` + return sourceName + ? `${sourceName} ${$localize`:@@inbox.hadMadeChanges:has made changes to your ORCID record`}` + : `${notification?.subject}` case 'INSTITUTIONAL_CONNECTION': - return `${$localize`:@@inbox.connectingYour:Connecting your`} ${ - notification.source.sourceName.content - } ${$localize`:@@inbox.accountWithYourOrcid:account with your ORCID record`}` + return sourceName + ? `${$localize`:@@inbox.connectingYour:Connecting your`} ${sourceName} ${$localize`:@@inbox.accountWithYourOrcid:account with your ORCID record`}` + : `${notification?.subject}` case 'PERMISSION': // The subject of the permission request is define by the member with the API diff --git a/src/app/types/notifications.endpoint.ts b/src/app/types/notifications.endpoint.ts index f3b1a823de..04dde7dcea 100644 --- a/src/app/types/notifications.endpoint.ts +++ b/src/app/types/notifications.endpoint.ts @@ -46,7 +46,10 @@ export interface InboxNotification { sourceDescription?: any encryptedPutCode?: any subject: string - source: SourceWithAssertionOrigin + // Nullable in practice: notifications raised by ORCID itself, and those + // whose member source is no longer resolvable, arrive with `source: null`. + // Declaring it required hid PD-13322 from `strictTemplates` for years. + source: SourceWithAssertionOrigin | null } export interface InboxNotificationAmended extends InboxNotification {