Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<ng-container i18n="@@inbox.pleaseCompleteThe"
>Please complete the process by connecting</ng-container
>
<strong>{{ notification?.source.sourceName.content }}</strong>
<strong>{{ notification?.source?.sourceName?.content }}</strong>
<ng-container i18n="@@inbox.yourOrcidRecord">
with your ORCID record.</ng-container
>
Expand All @@ -19,7 +19,7 @@
<a
target="_blank"
rel="noopener noreferrer"
[href]="notification?.authorizationUrl.uri"
[href]="notification?.authorizationUrl?.uri"
>
<button
mat-raised-button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
</div>

<div *ngIf="!isOrcidIntegration">
<strong>{{ notification?.source.sourceName.content }}</strong>
<strong>{{ notification?.source?.sourceName?.content }}</strong>
<ng-container i18n="@@inbox.likeYourPermission"
>would like your permission to interact with your ORCID Record as a trusted
party.</ng-container
Expand Down Expand Up @@ -75,7 +75,7 @@
<a
target="_blank"
rel="noopener noreferrer"
[href]="notification?.authorizationUrl.uri"
[href]="notification?.authorizationUrl?.uri"
>
<button
mat-raised-button
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<app-info-drop-down
[name]="notification?.source.sourceName.content"
[name]="notification?.source?.sourceName?.content"
[description]="notification?.sourceDescription"
></app-info-drop-down>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
})
})
})
18 changes: 12 additions & 6 deletions src/app/inbox/components/notification/notification.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/app/types/notifications.endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading