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
@@ -1,4 +1,10 @@
import { Component, EventEmitter, Input, Output } from '@angular/core'
import {
ChangeDetectionStrategy,
Component,
EventEmitter,
Input,
Output,
} from '@angular/core'
import { DomSanitizer } from '@angular/platform-browser'
import {
NgClass,
Expand Down Expand Up @@ -45,6 +51,7 @@ export interface RegistryNotificationActionEvent {
@Component({
selector: 'orcid-registry-permission-notifications',
standalone: true,
changeDetection: ChangeDetectionStrategy.OnPush,
imports: [
NgClass,
NgFor,
Expand Down
77 changes: 76 additions & 1 deletion src/app/core/inbox/permission-notifications.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { TestBed } from '@angular/core/testing'
import { of } from 'rxjs'
import { of, throwError } from 'rxjs'
import { PermissionNotificationsService } from './permission-notifications.service'
import { InboxService } from './inbox.service'
import { InboxNotificationPermission } from '../../types/notifications.endpoint'
import { AccountTrustedOrganizationsService } from '../account-trusted-organizations/account-trusted-organizations.service'

function createPermissionNotification(
overrides: Partial<InboxNotificationPermission> = {}
Expand All @@ -28,17 +29,27 @@ function createPermissionNotification(
describe('PermissionNotificationsService', () => {
let service: PermissionNotificationsService
let inboxSpy: jasmine.SpyObj<InboxService>
let trustedOrgsSpy: jasmine.SpyObj<AccountTrustedOrganizationsService>

beforeEach(() => {
inboxSpy = jasmine.createSpyObj<InboxService>('InboxService', [
'getUnreadCount',
'fetchNotificationsIncremental',
])
trustedOrgsSpy = jasmine.createSpyObj<AccountTrustedOrganizationsService>(
'AccountTrustedOrganizationsService',
['get']
)
trustedOrgsSpy.get.and.returnValue(of([]))

TestBed.configureTestingModule({
providers: [
PermissionNotificationsService,
{ provide: InboxService, useValue: inboxSpy },
{
provide: AccountTrustedOrganizationsService,
useValue: trustedOrgsSpy,
},
],
})
service = TestBed.inject(PermissionNotificationsService)
Expand Down Expand Up @@ -96,6 +107,70 @@ describe('PermissionNotificationsService', () => {
expect(result[0].source.sourceClientId.path).toBe('client-a')
expect(result[1].source.sourceClientId.path).toBe('client-b')
expect(inboxSpy.fetchNotificationsIncremental).toHaveBeenCalledWith(false)
expect(trustedOrgsSpy.get).toHaveBeenCalled()
done()
})
})

it('should filter out notifications from already trusted clientIds', (done) => {
const perm1 = createPermissionNotification({
putCode: 1,
sentDate: 2000,
source: {
sourceClientId: { path: 'client-a' } as any,
sourceName: { content: 'Org A' },
},
})
const perm2 = createPermissionNotification({
putCode: 2,
sentDate: 1000,
source: {
sourceClientId: { path: 'client-b' } as any,
sourceName: { content: 'Org B' },
},
})

trustedOrgsSpy.get.and.returnValue(of([{ clientId: 'client-a' } as any]))
inboxSpy.getUnreadCount.and.returnValue(of(5))
inboxSpy.fetchNotificationsIncremental.and.returnValue(
of({ total: 5, notifications: [perm1, perm2], done: true })
)

service.loadUnreadPermissionNotifications(3).subscribe((result) => {
expect(result.length).toBe(1)
expect(result[0].source.sourceClientId.path).toBe('client-b')
done()
})
})

it('should not fail if trusted-orgs lookup fails (no filtering applied)', (done) => {
const perm1 = createPermissionNotification({
putCode: 1,
sentDate: 2000,
source: {
sourceClientId: { path: 'client-a' } as any,
sourceName: { content: 'Org A' },
},
})
const perm2 = createPermissionNotification({
putCode: 2,
sentDate: 1000,
source: {
sourceClientId: { path: 'client-b' } as any,
sourceName: { content: 'Org B' },
},
})

trustedOrgsSpy.get.and.returnValue(throwError(() => new Error('fail')))
inboxSpy.getUnreadCount.and.returnValue(of(5))
inboxSpy.fetchNotificationsIncremental.and.returnValue(
of({ total: 5, notifications: [perm1, perm2], done: true })
)

service.loadUnreadPermissionNotifications(3).subscribe((result) => {
expect(result.length).toBe(2)
expect(result[0].source.sourceClientId.path).toBe('client-a')
expect(result[1].source.sourceClientId.path).toBe('client-b')
done()
})
})
Expand Down
82 changes: 57 additions & 25 deletions src/app/core/inbox/permission-notifications.service.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
import { Injectable } from '@angular/core'
import { Observable, of } from 'rxjs'
import { first, last, map, switchMap, takeWhile } from 'rxjs/operators'
import {
catchError,
first,
last,
map,
switchMap,
takeWhile,
} from 'rxjs/operators'
import { InboxService } from './inbox.service'
import {
InboxNotification,
InboxNotificationPermission,
} from '../../types/notifications.endpoint'
import { AccountTrustedOrganizationsService } from '../account-trusted-organizations/account-trusted-organizations.service'

@Injectable({
providedIn: 'root',
})
export class PermissionNotificationsService {
constructor(private _inbox: InboxService) {}
constructor(
private _inbox: InboxService,
private _trustedOrgs: AccountTrustedOrganizationsService
) {}

/**
* Load unread permission notifications, one per client (deduplicated by source client),
Expand All @@ -28,38 +39,58 @@ export class PermissionNotificationsService {
return of([])
}
const limit = Math.min(maxToScan, unreadCount)
return this._inbox.fetchNotificationsIncremental(false).pipe(
takeWhile(
(ev: {
total: number
notifications: InboxNotification[]
done: boolean
}) => {
const grouped = this.groupUnreadPermissionByClient(
ev.notifications
)
return (
grouped.length < maxItems &&
!ev.done &&
ev.notifications.length < limit
return this.getTrustedOrgClientIds().pipe(
switchMap((trustedClientIds) =>
this._inbox.fetchNotificationsIncremental(false).pipe(
takeWhile(
(ev: {
total: number
notifications: InboxNotification[]
done: boolean
}) => {
const grouped = this.groupUnreadPermissionByClient(
ev.notifications,
trustedClientIds
)
return (
grouped.length < maxItems &&
!ev.done &&
ev.notifications.length < limit
)
},
true
),
last(),
map((ev: { notifications: InboxNotification[] }) =>
this.groupUnreadPermissionByClient(
ev?.notifications ?? [],
trustedClientIds
).slice(0, maxItems)
)
},
true
),
last(),
map((ev: { notifications: InboxNotification[] }) =>
this.groupUnreadPermissionByClient(ev?.notifications ?? []).slice(
0,
maxItems
)
)
)
})
)
}

private getTrustedOrgClientIds(): Observable<Set<string>> {
return this._trustedOrgs.get().pipe(
first(),
map((orgs) => {
const ids = (orgs || [])
.map((o) => o?.clientId)
.filter((id): id is string => typeof id === 'string' && id.length > 0)
return new Set<string>(ids)
}),
// If the trusted-orgs call fails, just don’t filter.
catchError(() => of(new Set<string>()))
)
}

private groupUnreadPermissionByClient(
notifications: InboxNotification[]
notifications: InboxNotification[],
trustedClientIds: Set<string>
): InboxNotificationPermission[] {
const unreadPermission = (notifications || [])
.filter(
Expand All @@ -71,6 +102,7 @@ export class PermissionNotificationsService {
for (const n of unreadPermission) {
const clientId = n?.source?.sourceClientId?.path
if (!clientId) continue
if (trustedClientIds?.has(clientId)) continue
if (!byClient.has(clientId)) byClient.set(clientId, n)
}
return Array.from(byClient.values()).sort(
Expand Down
16 changes: 0 additions & 16 deletions src/app/core/togglz/togglz-flags.enum.ts

This file was deleted.

36 changes: 23 additions & 13 deletions src/app/record/components/top-bar/top-bar.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { UserRecord } from '../../../types/record.local'
import { PlatformInfo, PlatformInfoService } from '../../../cdk/platform-info'
import { ModalNameComponent } from './modals/modal-name/modal-name.component'
import { ModalBiographyComponent } from './modals/modal-biography/modal-biography.component'
import { takeUntil } from 'rxjs/operators'
import { take, takeUntil } from 'rxjs/operators'
import { Subject } from 'rxjs'
import { UserService } from '../../../core'
import { RecordService } from '../../../core/record/record.service'
Expand All @@ -25,6 +25,8 @@ import {
import { InboxNotificationPermission } from 'src/app/types/notifications.endpoint'
import { first } from 'rxjs/operators'
import { Router } from '@angular/router'
import { TogglzService } from '../../../core/togglz/togglz.service'
import { TogglzFlag } from '../../../types/config.endpoint'

@Component({
selector: 'app-top-bar',
Expand Down Expand Up @@ -68,8 +70,8 @@ export class TopBarComponent implements OnInit, OnDestroy {

ariaLabelName: string

permissionPanelTitle = $localize`:@@topBar.permissionNotificationsTitle:Unread permission notifications`
permissionPanelSubtitle = $localize`:@@topBar.permissionNotificationsSubtitle:You have updates waiting for your review.`
permissionPanelTitle = $localize`:@@topBar.permissionNotificationsTitle:Organizations want to connect`
permissionPanelSubtitle = $localize`:@@topBar.permissionNotificationsSubtitle:Connecting with trusted organizations helps keep your ORCID record up-to-date.`
permissionPanelNotifications: RegistryPermissionNotification[] = []
private permissionPanelRaw: InboxNotificationPermission[] = []
private _permissionPanelLoadStarted = false
Expand All @@ -84,6 +86,7 @@ export class TopBarComponent implements OnInit, OnDestroy {
private _recordEmails: RecordEmailsService,
private _verificationEmailModalService: VerificationEmailModalService,
private _router: Router,
private _togglz: TogglzService,
@Inject(WINDOW) private window: Window
) {
_platform
Expand Down Expand Up @@ -149,15 +152,22 @@ export class TopBarComponent implements OnInit, OnDestroy {
)
}

// Load permission notifications panel once (only for logged-in private record views)
if (
!this.isPublicRecord &&
!this.recordWithIssues &&
!this._permissionPanelLoadStarted
) {
this._permissionPanelLoadStarted = true
this.loadUnreadPermissionNotifications()
}
// Load permission notifications panel once (only for logged-in private record views) and the togglz flag is active
this._togglz
.getStateOf(TogglzFlag.PERMISSION_NOTIFICATIONS)
.pipe(take(1))
.subscribe((value) => {
if (value) {
if (
!this.isPublicRecord &&
!this.recordWithIssues &&
!this._permissionPanelLoadStarted
) {
this._permissionPanelLoadStarted = true
this.loadUnreadPermissionNotifications()
}
}
})
})
}

Expand Down Expand Up @@ -202,7 +212,7 @@ export class TopBarComponent implements OnInit, OnDestroy {
.subscribe((grouped) => {
this.permissionPanelRaw = grouped
this.permissionPanelNotifications = grouped.map((n) => {
const orgName = n?.source?.sourceName?.content || ''
const orgName = n?.sourceDescription || ''
const escaped = orgName
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
Expand Down
1 change: 1 addition & 0 deletions src/app/types/config.endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const TogglzFlag = {
OAUTH_AFFILIATION_INTERSTITIAL: 'OAUTH_AFFILIATION_INTERSTITIAL',
MAINTENANCE_MESSAGE: 'MAINTENANCE_MESSAGE',
FEATURED_AFFILIATIONS: 'FEATURED_AFFILIATIONS',
PERMISSION_NOTIFICATIONS: 'PERMISSION_NOTIFICATIONS',
} as const

export type TogglzFlag = (typeof TogglzFlag)[keyof typeof TogglzFlag]
Expand Down
4 changes: 2 additions & 2 deletions src/locale/messages.lr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -13947,15 +13947,15 @@
<target>LR</target>
</trans-unit>
<trans-unit id="topBar.permissionNotificationsTitle" datatype="html" resname="topBar.permissionNotificationsTitle">
<source>Unread permission notifications</source>
<source>Organizations want to connect</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
<context context-type="linenumber">71</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="topBar.permissionNotificationsSubtitle" datatype="html" resname="topBar.permissionNotificationsSubtitle">
<source>You have updates waiting for your review.</source>
<source>Connecting with trusted organizations helps keep your ORCID record up-to-date.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
<context context-type="linenumber">72</context>
Expand Down
4 changes: 2 additions & 2 deletions src/locale/messages.rl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -13947,15 +13947,15 @@
<target>RL</target>
</trans-unit>
<trans-unit id="topBar.permissionNotificationsTitle" datatype="html" resname="topBar.permissionNotificationsTitle">
<source>Unread permission notifications</source>
<source>Organizations want to connect</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
<context context-type="linenumber">71</context>
</context-group>
<target>RL</target>
</trans-unit>
<trans-unit id="topBar.permissionNotificationsSubtitle" datatype="html" resname="topBar.permissionNotificationsSubtitle">
<source>You have updates waiting for your review.</source>
<source>Connecting with trusted organizations helps keep your ORCID record up-to-date.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/record/components/top-bar/top-bar.component.ts</context>
<context context-type="linenumber">72</context>
Expand Down
Loading