Skip to content

Commit 763e553

Browse files
cryptalithcryptalith
authored
PD-3855, PD-4963, PD-4968, PD-4972 (#2743)
* PD-3855 * PD-3855 * PD-3855
1 parent 143ee93 commit 763e553

10 files changed

Lines changed: 173 additions & 64 deletions

File tree

‎projects/orcid-registry-ui/src/lib/components/permission-notifications/permission-notifications.component.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { Component, EventEmitter, Input, Output } from '@angular/core'
1+
import {
2+
ChangeDetectionStrategy,
3+
Component,
4+
EventEmitter,
5+
Input,
6+
Output,
7+
} from '@angular/core'
28
import { DomSanitizer } from '@angular/platform-browser'
39
import {
410
NgClass,
@@ -45,6 +51,7 @@ export interface RegistryNotificationActionEvent {
4551
@Component({
4652
selector: 'orcid-registry-permission-notifications',
4753
standalone: true,
54+
changeDetection: ChangeDetectionStrategy.OnPush,
4855
imports: [
4956
NgClass,
5057
NgFor,

‎src/app/core/inbox/permission-notifications.service.spec.ts‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { TestBed } from '@angular/core/testing'
2-
import { of } from 'rxjs'
2+
import { of, throwError } from 'rxjs'
33
import { PermissionNotificationsService } from './permission-notifications.service'
44
import { InboxService } from './inbox.service'
55
import { InboxNotificationPermission } from '../../types/notifications.endpoint'
6+
import { AccountTrustedOrganizationsService } from '../account-trusted-organizations/account-trusted-organizations.service'
67

78
function createPermissionNotification(
89
overrides: Partial<InboxNotificationPermission> = {}
@@ -28,17 +29,27 @@ function createPermissionNotification(
2829
describe('PermissionNotificationsService', () => {
2930
let service: PermissionNotificationsService
3031
let inboxSpy: jasmine.SpyObj<InboxService>
32+
let trustedOrgsSpy: jasmine.SpyObj<AccountTrustedOrganizationsService>
3133

3234
beforeEach(() => {
3335
inboxSpy = jasmine.createSpyObj<InboxService>('InboxService', [
3436
'getUnreadCount',
3537
'fetchNotificationsIncremental',
3638
])
39+
trustedOrgsSpy = jasmine.createSpyObj<AccountTrustedOrganizationsService>(
40+
'AccountTrustedOrganizationsService',
41+
['get']
42+
)
43+
trustedOrgsSpy.get.and.returnValue(of([]))
3744

3845
TestBed.configureTestingModule({
3946
providers: [
4047
PermissionNotificationsService,
4148
{ provide: InboxService, useValue: inboxSpy },
49+
{
50+
provide: AccountTrustedOrganizationsService,
51+
useValue: trustedOrgsSpy,
52+
},
4253
],
4354
})
4455
service = TestBed.inject(PermissionNotificationsService)
@@ -96,6 +107,70 @@ describe('PermissionNotificationsService', () => {
96107
expect(result[0].source.sourceClientId.path).toBe('client-a')
97108
expect(result[1].source.sourceClientId.path).toBe('client-b')
98109
expect(inboxSpy.fetchNotificationsIncremental).toHaveBeenCalledWith(false)
110+
expect(trustedOrgsSpy.get).toHaveBeenCalled()
111+
done()
112+
})
113+
})
114+
115+
it('should filter out notifications from already trusted clientIds', (done) => {
116+
const perm1 = createPermissionNotification({
117+
putCode: 1,
118+
sentDate: 2000,
119+
source: {
120+
sourceClientId: { path: 'client-a' } as any,
121+
sourceName: { content: 'Org A' },
122+
},
123+
})
124+
const perm2 = createPermissionNotification({
125+
putCode: 2,
126+
sentDate: 1000,
127+
source: {
128+
sourceClientId: { path: 'client-b' } as any,
129+
sourceName: { content: 'Org B' },
130+
},
131+
})
132+
133+
trustedOrgsSpy.get.and.returnValue(of([{ clientId: 'client-a' } as any]))
134+
inboxSpy.getUnreadCount.and.returnValue(of(5))
135+
inboxSpy.fetchNotificationsIncremental.and.returnValue(
136+
of({ total: 5, notifications: [perm1, perm2], done: true })
137+
)
138+
139+
service.loadUnreadPermissionNotifications(3).subscribe((result) => {
140+
expect(result.length).toBe(1)
141+
expect(result[0].source.sourceClientId.path).toBe('client-b')
142+
done()
143+
})
144+
})
145+
146+
it('should not fail if trusted-orgs lookup fails (no filtering applied)', (done) => {
147+
const perm1 = createPermissionNotification({
148+
putCode: 1,
149+
sentDate: 2000,
150+
source: {
151+
sourceClientId: { path: 'client-a' } as any,
152+
sourceName: { content: 'Org A' },
153+
},
154+
})
155+
const perm2 = createPermissionNotification({
156+
putCode: 2,
157+
sentDate: 1000,
158+
source: {
159+
sourceClientId: { path: 'client-b' } as any,
160+
sourceName: { content: 'Org B' },
161+
},
162+
})
163+
164+
trustedOrgsSpy.get.and.returnValue(throwError(() => new Error('fail')))
165+
inboxSpy.getUnreadCount.and.returnValue(of(5))
166+
inboxSpy.fetchNotificationsIncremental.and.returnValue(
167+
of({ total: 5, notifications: [perm1, perm2], done: true })
168+
)
169+
170+
service.loadUnreadPermissionNotifications(3).subscribe((result) => {
171+
expect(result.length).toBe(2)
172+
expect(result[0].source.sourceClientId.path).toBe('client-a')
173+
expect(result[1].source.sourceClientId.path).toBe('client-b')
99174
done()
100175
})
101176
})

‎src/app/core/inbox/permission-notifications.service.ts‎

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,28 @@
11
import { Injectable } from '@angular/core'
22
import { Observable, of } from 'rxjs'
3-
import { first, last, map, switchMap, takeWhile } from 'rxjs/operators'
3+
import {
4+
catchError,
5+
first,
6+
last,
7+
map,
8+
switchMap,
9+
takeWhile,
10+
} from 'rxjs/operators'
411
import { InboxService } from './inbox.service'
512
import {
613
InboxNotification,
714
InboxNotificationPermission,
815
} from '../../types/notifications.endpoint'
16+
import { AccountTrustedOrganizationsService } from '../account-trusted-organizations/account-trusted-organizations.service'
917

1018
@Injectable({
1119
providedIn: 'root',
1220
})
1321
export class PermissionNotificationsService {
14-
constructor(private _inbox: InboxService) {}
22+
constructor(
23+
private _inbox: InboxService,
24+
private _trustedOrgs: AccountTrustedOrganizationsService
25+
) {}
1526

1627
/**
1728
* Load unread permission notifications, one per client (deduplicated by source client),
@@ -28,38 +39,58 @@ export class PermissionNotificationsService {
2839
return of([])
2940
}
3041
const limit = Math.min(maxToScan, unreadCount)
31-
return this._inbox.fetchNotificationsIncremental(false).pipe(
32-
takeWhile(
33-
(ev: {
34-
total: number
35-
notifications: InboxNotification[]
36-
done: boolean
37-
}) => {
38-
const grouped = this.groupUnreadPermissionByClient(
39-
ev.notifications
40-
)
41-
return (
42-
grouped.length < maxItems &&
43-
!ev.done &&
44-
ev.notifications.length < limit
42+
return this.getTrustedOrgClientIds().pipe(
43+
switchMap((trustedClientIds) =>
44+
this._inbox.fetchNotificationsIncremental(false).pipe(
45+
takeWhile(
46+
(ev: {
47+
total: number
48+
notifications: InboxNotification[]
49+
done: boolean
50+
}) => {
51+
const grouped = this.groupUnreadPermissionByClient(
52+
ev.notifications,
53+
trustedClientIds
54+
)
55+
return (
56+
grouped.length < maxItems &&
57+
!ev.done &&
58+
ev.notifications.length < limit
59+
)
60+
},
61+
true
62+
),
63+
last(),
64+
map((ev: { notifications: InboxNotification[] }) =>
65+
this.groupUnreadPermissionByClient(
66+
ev?.notifications ?? [],
67+
trustedClientIds
68+
).slice(0, maxItems)
4569
)
46-
},
47-
true
48-
),
49-
last(),
50-
map((ev: { notifications: InboxNotification[] }) =>
51-
this.groupUnreadPermissionByClient(ev?.notifications ?? []).slice(
52-
0,
53-
maxItems
5470
)
5571
)
5672
)
5773
})
5874
)
5975
}
6076

77+
private getTrustedOrgClientIds(): Observable<Set<string>> {
78+
return this._trustedOrgs.get().pipe(
79+
first(),
80+
map((orgs) => {
81+
const ids = (orgs || [])
82+
.map((o) => o?.clientId)
83+
.filter((id): id is string => typeof id === 'string' && id.length > 0)
84+
return new Set<string>(ids)
85+
}),
86+
// If the trusted-orgs call fails, just don’t filter.
87+
catchError(() => of(new Set<string>()))
88+
)
89+
}
90+
6191
private groupUnreadPermissionByClient(
62-
notifications: InboxNotification[]
92+
notifications: InboxNotification[],
93+
trustedClientIds: Set<string>
6394
): InboxNotificationPermission[] {
6495
const unreadPermission = (notifications || [])
6596
.filter(
@@ -71,6 +102,7 @@ export class PermissionNotificationsService {
71102
for (const n of unreadPermission) {
72103
const clientId = n?.source?.sourceClientId?.path
73104
if (!clientId) continue
105+
if (trustedClientIds?.has(clientId)) continue
74106
if (!byClient.has(clientId)) byClient.set(clientId, n)
75107
}
76108
return Array.from(byClient.values()).sort(

‎src/app/core/togglz/togglz-flags.enum.ts‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎src/app/record/components/top-bar/top-bar.component.ts‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { UserRecord } from '../../../types/record.local'
33
import { PlatformInfo, PlatformInfoService } from '../../../cdk/platform-info'
44
import { ModalNameComponent } from './modals/modal-name/modal-name.component'
55
import { ModalBiographyComponent } from './modals/modal-biography/modal-biography.component'
6-
import { takeUntil } from 'rxjs/operators'
6+
import { take, takeUntil } from 'rxjs/operators'
77
import { Subject } from 'rxjs'
88
import { UserService } from '../../../core'
99
import { RecordService } from '../../../core/record/record.service'
@@ -25,6 +25,8 @@ import {
2525
import { InboxNotificationPermission } from 'src/app/types/notifications.endpoint'
2626
import { first } from 'rxjs/operators'
2727
import { Router } from '@angular/router'
28+
import { TogglzService } from '../../../core/togglz/togglz.service'
29+
import { TogglzFlag } from '../../../types/config.endpoint'
2830

2931
@Component({
3032
selector: 'app-top-bar',
@@ -68,8 +70,8 @@ export class TopBarComponent implements OnInit, OnDestroy {
6870

6971
ariaLabelName: string
7072

71-
permissionPanelTitle = $localize`:@@topBar.permissionNotificationsTitle:Unread permission notifications`
72-
permissionPanelSubtitle = $localize`:@@topBar.permissionNotificationsSubtitle:You have updates waiting for your review.`
73+
permissionPanelTitle = $localize`:@@topBar.permissionNotificationsTitle:Organizations want to connect`
74+
permissionPanelSubtitle = $localize`:@@topBar.permissionNotificationsSubtitle:Connecting with trusted organizations helps keep your ORCID record up-to-date.`
7375
permissionPanelNotifications: RegistryPermissionNotification[] = []
7476
private permissionPanelRaw: InboxNotificationPermission[] = []
7577
private _permissionPanelLoadStarted = false
@@ -84,6 +86,7 @@ export class TopBarComponent implements OnInit, OnDestroy {
8486
private _recordEmails: RecordEmailsService,
8587
private _verificationEmailModalService: VerificationEmailModalService,
8688
private _router: Router,
89+
private _togglz: TogglzService,
8790
@Inject(WINDOW) private window: Window
8891
) {
8992
_platform
@@ -149,15 +152,22 @@ export class TopBarComponent implements OnInit, OnDestroy {
149152
)
150153
}
151154

152-
// Load permission notifications panel once (only for logged-in private record views)
153-
if (
154-
!this.isPublicRecord &&
155-
!this.recordWithIssues &&
156-
!this._permissionPanelLoadStarted
157-
) {
158-
this._permissionPanelLoadStarted = true
159-
this.loadUnreadPermissionNotifications()
160-
}
155+
// Load permission notifications panel once (only for logged-in private record views) and the togglz flag is active
156+
this._togglz
157+
.getStateOf(TogglzFlag.PERMISSION_NOTIFICATIONS)
158+
.pipe(take(1))
159+
.subscribe((value) => {
160+
if (value) {
161+
if (
162+
!this.isPublicRecord &&
163+
!this.recordWithIssues &&
164+
!this._permissionPanelLoadStarted
165+
) {
166+
this._permissionPanelLoadStarted = true
167+
this.loadUnreadPermissionNotifications()
168+
}
169+
}
170+
})
161171
})
162172
}
163173

@@ -202,7 +212,7 @@ export class TopBarComponent implements OnInit, OnDestroy {
202212
.subscribe((grouped) => {
203213
this.permissionPanelRaw = grouped
204214
this.permissionPanelNotifications = grouped.map((n) => {
205-
const orgName = n?.source?.sourceName?.content || ''
215+
const orgName = n?.sourceDescription || ''
206216
const escaped = orgName
207217
.replace(/&/g, '&amp;')
208218
.replace(/</g, '&lt;')

‎src/app/types/config.endpoint.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const TogglzFlag = {
1313
OAUTH_AFFILIATION_INTERSTITIAL: 'OAUTH_AFFILIATION_INTERSTITIAL',
1414
MAINTENANCE_MESSAGE: 'MAINTENANCE_MESSAGE',
1515
FEATURED_AFFILIATIONS: 'FEATURED_AFFILIATIONS',
16+
PERMISSION_NOTIFICATIONS: 'PERMISSION_NOTIFICATIONS',
1617
} as const
1718

1819
export type TogglzFlag = (typeof TogglzFlag)[keyof typeof TogglzFlag]

‎src/locale/messages.lr.xlf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13947,15 +13947,15 @@
1394713947
<target>LR</target>
1394813948
</trans-unit>
1394913949
<trans-unit id="topBar.permissionNotificationsTitle" datatype="html" resname="topBar.permissionNotificationsTitle">
13950-
<source>Unread permission notifications</source>
13950+
<source>Organizations want to connect</source>
1395113951
<context-group purpose="location">
1395213952
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1395313953
<context context-type="linenumber">71</context>
1395413954
</context-group>
1395513955
<target>LR</target>
1395613956
</trans-unit>
1395713957
<trans-unit id="topBar.permissionNotificationsSubtitle" datatype="html" resname="topBar.permissionNotificationsSubtitle">
13958-
<source>You have updates waiting for your review.</source>
13958+
<source>Connecting with trusted organizations helps keep your ORCID record up-to-date.</source>
1395913959
<context-group purpose="location">
1396013960
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1396113961
<context context-type="linenumber">72</context>

‎src/locale/messages.rl.xlf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13947,15 +13947,15 @@
1394713947
<target>RL</target>
1394813948
</trans-unit>
1394913949
<trans-unit id="topBar.permissionNotificationsTitle" datatype="html" resname="topBar.permissionNotificationsTitle">
13950-
<source>Unread permission notifications</source>
13950+
<source>Organizations want to connect</source>
1395113951
<context-group purpose="location">
1395213952
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1395313953
<context context-type="linenumber">71</context>
1395413954
</context-group>
1395513955
<target>RL</target>
1395613956
</trans-unit>
1395713957
<trans-unit id="topBar.permissionNotificationsSubtitle" datatype="html" resname="topBar.permissionNotificationsSubtitle">
13958-
<source>You have updates waiting for your review.</source>
13958+
<source>Connecting with trusted organizations helps keep your ORCID record up-to-date.</source>
1395913959
<context-group purpose="location">
1396013960
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1396113961
<context context-type="linenumber">72</context>

0 commit comments

Comments
 (0)