Skip to content

Commit e2b4539

Browse files
cryptalithcryptalith
committed
PD-3855
1 parent 5158cbd commit e2b4539

7 files changed

Lines changed: 134 additions & 37 deletions

File tree

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

Lines changed: 73 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,24 @@ 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+
{ provide: AccountTrustedOrganizationsService, useValue: trustedOrgsSpy },
4250
],
4351
})
4452
service = TestBed.inject(PermissionNotificationsService)
@@ -96,6 +104,70 @@ describe('PermissionNotificationsService', () => {
96104
expect(result[0].source.sourceClientId.path).toBe('client-a')
97105
expect(result[1].source.sourceClientId.path).toBe('client-b')
98106
expect(inboxSpy.fetchNotificationsIncremental).toHaveBeenCalledWith(false)
107+
expect(trustedOrgsSpy.get).toHaveBeenCalled()
108+
done()
109+
})
110+
})
111+
112+
it('should filter out notifications from already trusted clientIds', (done) => {
113+
const perm1 = createPermissionNotification({
114+
putCode: 1,
115+
sentDate: 2000,
116+
source: {
117+
sourceClientId: { path: 'client-a' } as any,
118+
sourceName: { content: 'Org A' },
119+
},
120+
})
121+
const perm2 = createPermissionNotification({
122+
putCode: 2,
123+
sentDate: 1000,
124+
source: {
125+
sourceClientId: { path: 'client-b' } as any,
126+
sourceName: { content: 'Org B' },
127+
},
128+
})
129+
130+
trustedOrgsSpy.get.and.returnValue(of([{ clientId: 'client-a' } as any]))
131+
inboxSpy.getUnreadCount.and.returnValue(of(5))
132+
inboxSpy.fetchNotificationsIncremental.and.returnValue(
133+
of({ total: 5, notifications: [perm1, perm2], done: true })
134+
)
135+
136+
service.loadUnreadPermissionNotifications(3).subscribe((result) => {
137+
expect(result.length).toBe(1)
138+
expect(result[0].source.sourceClientId.path).toBe('client-b')
139+
done()
140+
})
141+
})
142+
143+
it('should not fail if trusted-orgs lookup fails (no filtering applied)', (done) => {
144+
const perm1 = createPermissionNotification({
145+
putCode: 1,
146+
sentDate: 2000,
147+
source: {
148+
sourceClientId: { path: 'client-a' } as any,
149+
sourceName: { content: 'Org A' },
150+
},
151+
})
152+
const perm2 = createPermissionNotification({
153+
putCode: 2,
154+
sentDate: 1000,
155+
source: {
156+
sourceClientId: { path: 'client-b' } as any,
157+
sourceName: { content: 'Org B' },
158+
},
159+
})
160+
161+
trustedOrgsSpy.get.and.returnValue(throwError(() => new Error('fail')))
162+
inboxSpy.getUnreadCount.and.returnValue(of(5))
163+
inboxSpy.fetchNotificationsIncremental.and.returnValue(
164+
of({ total: 5, notifications: [perm1, perm2], done: true })
165+
)
166+
167+
service.loadUnreadPermissionNotifications(3).subscribe((result) => {
168+
expect(result.length).toBe(2)
169+
expect(result[0].source.sourceClientId.path).toBe('client-a')
170+
expect(result[1].source.sourceClientId.path).toBe('client-b')
99171
done()
100172
})
101173
})

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

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
import { Injectable } from '@angular/core'
22
import { Observable, of } from 'rxjs'
3-
import { first, last, map, switchMap, takeWhile } from 'rxjs/operators'
3+
import { catchError, first, last, map, switchMap, takeWhile } from 'rxjs/operators'
44
import { InboxService } from './inbox.service'
55
import {
66
InboxNotification,
77
InboxNotificationPermission,
88
} from '../../types/notifications.endpoint'
9+
import { AccountTrustedOrganizationsService } from '../account-trusted-organizations/account-trusted-organizations.service'
910

1011
@Injectable({
1112
providedIn: 'root',
1213
})
1314
export class PermissionNotificationsService {
14-
constructor(private _inbox: InboxService) {}
15+
constructor(
16+
private _inbox: InboxService,
17+
private _trustedOrgs: AccountTrustedOrganizationsService
18+
) {}
1519

1620
/**
1721
* Load unread permission notifications, one per client (deduplicated by source client),
@@ -28,38 +32,58 @@ export class PermissionNotificationsService {
2832
return of([])
2933
}
3034
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
35+
return this.getTrustedOrgClientIds().pipe(
36+
switchMap((trustedClientIds) =>
37+
this._inbox.fetchNotificationsIncremental(false).pipe(
38+
takeWhile(
39+
(ev: {
40+
total: number
41+
notifications: InboxNotification[]
42+
done: boolean
43+
}) => {
44+
const grouped = this.groupUnreadPermissionByClient(
45+
ev.notifications,
46+
trustedClientIds
47+
)
48+
return (
49+
grouped.length < maxItems &&
50+
!ev.done &&
51+
ev.notifications.length < limit
52+
)
53+
},
54+
true
55+
),
56+
last(),
57+
map((ev: { notifications: InboxNotification[] }) =>
58+
this.groupUnreadPermissionByClient(
59+
ev?.notifications ?? [],
60+
trustedClientIds
61+
).slice(0, maxItems)
4062
)
41-
return (
42-
grouped.length < maxItems &&
43-
!ev.done &&
44-
ev.notifications.length < limit
45-
)
46-
},
47-
true
48-
),
49-
last(),
50-
map((ev: { notifications: InboxNotification[] }) =>
51-
this.groupUnreadPermissionByClient(ev?.notifications ?? []).slice(
52-
0,
53-
maxItems
5463
)
5564
)
5665
)
5766
})
5867
)
5968
}
6069

70+
private getTrustedOrgClientIds(): Observable<Set<string>> {
71+
return this._trustedOrgs.get().pipe(
72+
first(),
73+
map((orgs) => {
74+
const ids = (orgs || [])
75+
.map((o) => o?.clientId)
76+
.filter((id): id is string => typeof id === 'string' && id.length > 0)
77+
return new Set<string>(ids)
78+
}),
79+
// If the trusted-orgs call fails, just don’t filter.
80+
catchError(() => of(new Set<string>()))
81+
)
82+
}
83+
6184
private groupUnreadPermissionByClient(
62-
notifications: InboxNotification[]
85+
notifications: InboxNotification[],
86+
trustedClientIds: Set<string>
6387
): InboxNotificationPermission[] {
6488
const unreadPermission = (notifications || [])
6589
.filter(
@@ -71,6 +95,7 @@ export class PermissionNotificationsService {
7195
for (const n of unreadPermission) {
7296
const clientId = n?.source?.sourceClientId?.path
7397
if (!clientId) continue
98+
if (trustedClientIds?.has(clientId)) continue
7499
if (!byClient.has(clientId)) byClient.set(clientId, n)
75100
}
76101
return Array.from(byClient.values()).sort(

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ export class TopBarComponent implements OnInit, OnDestroy {
7070

7171
ariaLabelName: string
7272

73-
permissionPanelTitle = $localize`:@@topBar.permissionNotificationsTitle:Unread permission notifications`
74-
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.`
7575
permissionPanelNotifications: RegistryPermissionNotification[] = []
7676
private permissionPanelRaw: InboxNotificationPermission[] = []
7777
private _permissionPanelLoadStarted = false
@@ -157,7 +157,7 @@ export class TopBarComponent implements OnInit, OnDestroy {
157157
.getStateOf(TogglzFlag.PERMISSION_NOTIFICATIONS)
158158
.pipe(take(1))
159159
.subscribe((value) => {
160-
if (value) {
160+
if (!value) {
161161
if (
162162
!this.isPublicRecord &&
163163
!this.recordWithIssues &&

‎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>

‎src/locale/messages.xlf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12533,14 +12533,14 @@
1253312533
</context-group>
1253412534
</trans-unit>
1253512535
<trans-unit id="topBar.permissionNotificationsTitle" datatype="html" resname="topBar.permissionNotificationsTitle">
12536-
<source>Unread permission notifications</source>
12536+
<source>Organizations want to connect</source>
1253712537
<context-group purpose="location">
1253812538
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1253912539
<context context-type="linenumber">71</context>
1254012540
</context-group>
1254112541
</trans-unit>
1254212542
<trans-unit id="topBar.permissionNotificationsSubtitle" datatype="html" resname="topBar.permissionNotificationsSubtitle">
12543-
<source>You have updates waiting for your review.</source>
12543+
<source>Connecting with trusted organizations helps keep your ORCID record up-to-date.</source>
1254412544
<context-group purpose="location">
1254512545
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
1254612546
<context context-type="linenumber">72</context>

‎src/locale/messages.xx.xlf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13947,15 +13947,15 @@
1394713947
<target>X</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>X</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)