diff --git a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.spec.ts b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.spec.ts index 741cf6c145..d32f7cc0b9 100644 --- a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.spec.ts +++ b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.spec.ts @@ -8,15 +8,34 @@ import { RecordAffiliationService } from 'src/app/core/record-affiliations/recor import { RecordService } from 'src/app/core/record/record.service' import { RegisterService } from 'src/app/core/register/register.service' import { AffiliationsInterstitialComponent } from './affiliations-interstitial.component' -import { EMPTY } from 'rxjs' +import { EMPTY, of } from 'rxjs' import { PlatformInfoService } from 'src/app/cdk/platform-info' import { WINDOW_PROVIDERS } from 'src/app/cdk/window' +import { AffiliationInterstitialOrganizationService } from 'src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service' +import { Organization } from 'src/app/types/common.endpoint' describe('AffiliationsInterstitialComponent', () => { let component: AffiliationsInterstitialComponent let fixture: ComponentFixture + let affiliationOrganization: jasmine.SpyObj + + const domain = { value: 'my.edu', createdDate: { timestamp: 1 } } + const organization = { value: 'My University' } as Organization + + /** + * The template is not rendered by these tests, so the mocks only need to + * satisfy the component's own code. `UntypedFormBuilder` is deliberately + * left as the real one: the interstitial reads its controls back by name. + */ + function configure(recordService: unknown) { + affiliationOrganization = + jasmine.createSpyObj( + 'AffiliationInterstitialOrganizationService', + ['mostRecentDomain', 'resolveFromDomains'] + ) + affiliationOrganization.mostRecentDomain.and.returnValue(domain as any) + affiliationOrganization.resolveFromDomains.and.returnValue(of(organization)) - beforeEach(() => { TestBed.configureTestingModule({ declarations: [AffiliationsInterstitialComponent], providers: [ @@ -26,17 +45,11 @@ describe('AffiliationsInterstitialComponent', () => { get: () => EMPTY, }, }, + { provide: RecordService, useValue: recordService }, { - provide: RecordService, - useValue: { - getRecord: () => ({ - pipe: () => ({ - subscribe: () => {}, - }), - }), - }, + provide: AffiliationInterstitialOrganizationService, + useValue: affiliationOrganization, }, - { provide: RegisterService, useValue: {}, @@ -77,10 +90,69 @@ describe('AffiliationsInterstitialComponent', () => { }) fixture = TestBed.createComponent(AffiliationsInterstitialComponent) component = fixture.componentInstance - fixture.detectChanges() - }) + } + + const recordWithDomain = { + getRecord: () => + of({ + emails: { emailDomains: [domain] }, + }), + } it('should create', () => { + configure({ + getRecord: () => ({ + pipe: () => ({ + subscribe: () => {}, + }), + }), + }) + fixture.detectChanges() expect(component).toBeTruthy() }) + + it('pre-selects the organization the domain resolves to', () => { + configure(recordWithDomain) + + component.ngOnInit() + + expect(component.userDomainMatched).toEqual('my.edu') + expect(component.organizationFromDatabase).toBe(organization) + expect(component.rorIdHasBeenMatched).toBeTrue() + expect(component.form).toBeDefined() + expect(component.form.get('organization').value).toBe(organization) + }) + + // PD-13050: the form used to be built inside a stream that short-circuited + // to EMPTY when no organization came back, so it was never built at all and + // the interstitial rendered nothing but its spinner. + it('still builds the form when the domain resolves to no organization', () => { + configure(recordWithDomain) + affiliationOrganization.resolveFromDomains.and.returnValue(of(undefined)) + + component.ngOnInit() + + expect(component.form).toBeDefined() + expect(component.organizationFromDatabase).toBeUndefined() + expect(component.rorIdHasBeenMatched).toBeFalse() + }) + + it('builds the form once, from the first record emission carrying emails', () => { + configure({ + getRecord: () => + of( + undefined, + { emails: undefined }, + { emails: { emailDomains: [domain] } }, + { emails: { emailDomains: [domain] } } + ), + }) + + component.ngOnInit() + const form = component.form + + expect(form).toBeDefined() + expect(affiliationOrganization.resolveFromDomains).toHaveBeenCalledTimes(1) + expect(component.form).toBe(form) + }) }) diff --git a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.ts b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.ts index ed7cdbd23e..9a7190be06 100644 --- a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.ts +++ b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component.ts @@ -2,6 +2,7 @@ import { Component, EventEmitter, Inject, + inject, OnDestroy, OnInit, Output, @@ -13,22 +14,18 @@ import { Validators, AbstractControl, } from '@angular/forms' -import { Subject, Observable, of, EMPTY } from 'rxjs' -import { switchMap, first, takeUntil, map, tap } from 'rxjs/operators' +import { Subject, Observable, of } from 'rxjs' +import { switchMap, filter, first, takeUntil, tap } from 'rxjs/operators' import { WINDOW } from 'src/app/cdk/window' import { PlatformInfo, PlatformInfoService } from 'src/app/cdk/platform-info' import { RecordAffiliationService } from 'src/app/core/record-affiliations/record-affiliations.service' import { RecordService } from 'src/app/core/record/record.service' import { Organization, Value } from 'src/app/types/common.endpoint' -import { - affiliationToOrganization, - MAX_LENGTH_LESS_THAN_ONE_THOUSAND, -} from 'src/app/constants' +import { MAX_LENGTH_LESS_THAN_ONE_THOUSAND } from 'src/app/constants' import { dateMonthYearValidator } from 'src/app/shared/validators/date/date.validator' -import { OrganizationsService, UserService } from 'src/app/core' -import { RegisterService } from 'src/app/core/register/register.service' -import { AssertionVisibilityString } from 'src/app/types' +import { UserService } from 'src/app/core' +import { AffiliationInterstitialOrganizationService } from 'src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service' import { Affiliation, AffiliationType, @@ -91,14 +88,20 @@ export class AffiliationsInterstitialComponent implements OnInit, OnDestroy { $destroy: Subject = new Subject() organizationName: string + /** + * Injected here rather than through the constructor so the dialog subclass + * keeps its `super(...)` signature. + */ + private affiliationOrganization = inject( + AffiliationInterstitialOrganizationService + ) + constructor( @Inject(WINDOW) private window: Window, private platformService: PlatformInfoService, private recordAffiliationService: RecordAffiliationService, private formBuilder: UntypedFormBuilder, private recordService: RecordService, - private organizationService: OrganizationsService, - private registerService: RegisterService, private user: UserService ) {} @@ -106,34 +109,38 @@ export class AffiliationsInterstitialComponent implements OnInit, OnDestroy { this.platformService.get().subscribe((data) => { this.platform = data }) - // Attempt to detect organization from user’s email domain + // Attempt to detect organization from user’s email domain. + // + // `getRecord()` re-emits every time another slice of the record lands, so + // wait for the first emission that actually carries emails and build the + // form once from it — rebuilding on every later emission would discard + // whatever the user had already typed. + // + // The resolution itself must always emit, including when the domain maps + // to no organization. It used to fall through to `EMPTY` in that case, so + // the subscribe body never ran, the form was never built and the + // interstitial sat on its spinner forever. this.recordService .getRecord() .pipe( - map((record) => - this.sortDomainsByCreatedDate(record?.emails?.emailDomains) - ), - switchMap((domain: AssertionVisibilityString) => { - if (domain) { - this.userDomainMatched = domain.value - return this.registerService - .getEmailCategory(domain.value) - .pipe(map((response) => response.rorId)) - } - return EMPTY + filter((record) => !!record?.emails), + first(), + tap((record) => { + this.userDomainMatched = + this.affiliationOrganization.mostRecentDomain( + record.emails.emailDomains + )?.value }), - switchMap((rorId: string) => { - if (rorId) { - return this.organizationService - .getOrgDisambiguated('ROR', rorId) - .pipe(first()) - } - return EMPTY - }) + switchMap((record) => + this.affiliationOrganization.resolveFromDomains( + record.emails.emailDomains + ) + ), + takeUntil(this.destroy$) ) .subscribe((org) => { if (org) { - this.organizationFromDatabase = affiliationToOrganization(org) + this.organizationFromDatabase = org this.rorIdHasBeenMatched = true this.displayOrganizationHint = true } @@ -202,23 +209,11 @@ export class AffiliationsInterstitialComponent implements OnInit, OnDestroy { }) } - sortDomainsByCreatedDate( - domains: AssertionVisibilityString[] | undefined - ): AssertionVisibilityString { - if (!Array.isArray(domains) || domains.length === 0) return undefined - - const sorted = domains.slice().sort((a, b) => { - const aTimestamp = a.createdDate?.timestamp ?? 0 - const bTimestamp = b.createdDate?.timestamp ?? 0 - return bTimestamp - aTimestamp - }) - - return sorted[0] - } - ngOnDestroy(): void { this.destroy$.next() this.destroy$.complete() + this.$destroy.next() + this.$destroy.complete() } /** diff --git a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.spec.ts b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.spec.ts new file mode 100644 index 0000000000..5384f6f1db --- /dev/null +++ b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.spec.ts @@ -0,0 +1,87 @@ +import { TestBed } from '@angular/core/testing' +import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core' +import { MatDialogRef } from '@angular/material/dialog' +import { EMPTY, of } from 'rxjs' + +import { PlatformInfoService } from 'src/app/cdk/platform-info' +import { WINDOW_PROVIDERS } from 'src/app/cdk/window' +import { UserService } from 'src/app/core' +import { AffiliationInterstitialOrganizationService } from 'src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service' +import { RecordAffiliationService } from 'src/app/core/record-affiliations/record-affiliations.service' +import { RecordService } from 'src/app/core/record/record.service' +import { Organization } from 'src/app/types/common.endpoint' +import { AffiliationsInterstitialDialogComponent } from './affiliations-interstitial-dialog.component' + +describe('AffiliationsInterstitialDialogComponent', () => { + let component: AffiliationsInterstitialDialogComponent + let dialogRef: jasmine.SpyObj> + let affiliationOrganization: jasmine.SpyObj + + const domain = { value: 'my.edu', createdDate: { timestamp: 1 } } + const organization = { value: 'My University' } as Organization + + beforeEach(() => { + dialogRef = jasmine.createSpyObj>('MatDialogRef', [ + 'close', + ]) + affiliationOrganization = + jasmine.createSpyObj( + 'AffiliationInterstitialOrganizationService', + ['mostRecentDomain', 'resolveFromDomains'] + ) + affiliationOrganization.mostRecentDomain.and.returnValue(domain as any) + affiliationOrganization.resolveFromDomains.and.returnValue(of(organization)) + + TestBed.configureTestingModule({ + declarations: [AffiliationsInterstitialDialogComponent], + providers: [ + { provide: MatDialogRef, useValue: dialogRef }, + { provide: PlatformInfoService, useValue: { get: () => EMPTY } }, + { + provide: RecordService, + useValue: { + getRecord: () => of({ emails: { emailDomains: [domain] } }), + }, + }, + { + provide: AffiliationInterstitialOrganizationService, + useValue: affiliationOrganization, + }, + { provide: RecordAffiliationService, useValue: {} }, + { + provide: UserService, + useValue: { + getUserSession: () => ({ + pipe: () => ({ subscribe: () => {} }), + }), + }, + }, + WINDOW_PROVIDERS, + ], + schemas: [CUSTOM_ELEMENTS_SCHEMA], + }) + + component = TestBed.createComponent( + AffiliationsInterstitialDialogComponent + ).componentInstance + }) + + // The subclass forwards a fixed argument list to the base constructor, so + // it only keeps working as long as the two agree. + it('constructs and inherits the base resolution', () => { + component.ngOnInit() + + expect(component.userDomainMatched).toEqual('my.edu') + expect(component.organizationFromDatabase).toBe(organization) + expect(component.form).toBeDefined() + }) + + it('closes the dialog instead of emitting finish', () => { + component.finishIntertsitial() + + expect(dialogRef.close).toHaveBeenCalledWith({ + type: 'affiliation-interstitial', + addedAffiliation: undefined, + }) + }) +}) diff --git a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.ts b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.ts index 85ff0d7756..68de03ea26 100644 --- a/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.ts +++ b/src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component.ts @@ -20,10 +20,9 @@ import { MatDialogRef, MatDialogState, } from '@angular/material/dialog' -import { OrganizationsService, UserService } from 'src/app/core' +import { UserService } from 'src/app/core' import { RecordService } from 'src/app/core/record/record.service' import { RecordAffiliationService } from 'src/app/core/record-affiliations/record-affiliations.service' -import { RegisterService } from 'src/app/core/register/register.service' import { BaseInterstitialDialogInput, @@ -65,8 +64,6 @@ export class AffiliationsInterstitialDialogComponent extends AffiliationsInterst recordAffiliationService: RecordAffiliationService, formBuilder: UntypedFormBuilder, recordService: RecordService, - organizationService: OrganizationsService, - registerService: RegisterService, private dialogRef: MatDialogRef< AffiliationsInterstitialDialogComponent, AffilationsComponentDialogOutput @@ -79,8 +76,6 @@ export class AffiliationsInterstitialDialogComponent extends AffiliationsInterst recordAffiliationService, formBuilder, recordService, - organizationService, - registerService, user ) } diff --git a/src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service.ts b/src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service.ts new file mode 100644 index 0000000000..ce810796e7 --- /dev/null +++ b/src/app/core/login-interstitials-manager/affiliation-interstitial-organization.service.ts @@ -0,0 +1,107 @@ +import { Injectable } from '@angular/core' +import { Observable, of } from 'rxjs' +import { + catchError, + defaultIfEmpty, + first, + map, + shareReplay, + switchMap, +} from 'rxjs/operators' + +import { affiliationToOrganization } from 'src/app/constants' +import { OrganizationsService } from 'src/app/core/organizations/organizations.service' +import { RegisterService } from 'src/app/core/register/register.service' +import { Organization } from 'src/app/types/common.endpoint' +import { AssertionVisibilityString } from 'src/app/types' + +/** + * Resolves the single organization an email domain stands for. + * + * Both the affiliation interstitial's eligibility check and the interstitial + * itself need this answer, and they have to agree: if they disagreed the user + * would be shown an interstitial that then has nothing to offer. Keeping the + * lookup — and the choice of which domain to look up — in one place is what + * makes them agree. + */ +@Injectable({ + providedIn: 'root', +}) +export class AffiliationInterstitialOrganizationService { + private readonly byDomain = new Map< + string, + Observable + >() + + constructor( + private registerService: RegisterService, + private organizationsService: OrganizationsService + ) {} + + /** + * The interstitial only ever speaks about one domain: the most recently + * added one. + */ + mostRecentDomain( + domains: AssertionVisibilityString[] | undefined + ): AssertionVisibilityString | undefined { + if (!Array.isArray(domains) || domains.length === 0) { + return undefined + } + return domains + .slice() + .sort( + (a, b) => + (b.createdDate?.timestamp ?? 0) - (a.createdDate?.timestamp ?? 0) + )[0] + } + + resolveFromDomains( + domains: AssertionVisibilityString[] | undefined + ): Observable { + const domain = this.mostRecentDomain(domains) + return domain?.value ? this.resolve(domain.value) : of(undefined) + } + + /** + * Emits the organization the domain maps to, or `undefined` when there + * isn't exactly one. + * + * `email-domain/find-category` returns a `rorId` only for a domain that + * matches a single ROR. An unknown domain and a domain matching several + * both come back without one, so every "nothing to offer here" case — + * no domain, no ROR, several RORs, a ROR that resolves to no org, a failed + * lookup — reaches the caller the same way, as `undefined`. + * + * Cached per domain: the eligibility check and the interstitial ask the + * same question within one page view, and the second ask should not cost + * two more round trips. + */ + resolve(domain: string): Observable { + if (!this.byDomain.has(domain)) { + this.byDomain.set( + domain, + this.request(domain).pipe( + shareReplay({ bufferSize: 1, refCount: false }) + ) + ) + } + return this.byDomain.get(domain) + } + + private request(domain: string): Observable { + return this.registerService.getEmailCategory(domain).pipe( + first(), + switchMap((category) => + category?.rorId + ? this.organizationsService + .getOrgDisambiguated('ROR', category.rorId) + .pipe(first()) + : of(undefined) + ), + map((org) => (org ? affiliationToOrganization(org) : undefined)), + defaultIfEmpty(undefined), + catchError(() => of(undefined)) + ) + } +} diff --git a/src/app/core/login-interstitials-manager/implementations/login-affiliation-interstitials-manager.service.ts b/src/app/core/login-interstitials-manager/implementations/login-affiliation-interstitials-manager.service.ts index 31ee2089dc..0a4f4db554 100644 --- a/src/app/core/login-interstitials-manager/implementations/login-affiliation-interstitials-manager.service.ts +++ b/src/app/core/login-interstitials-manager/implementations/login-affiliation-interstitials-manager.service.ts @@ -1,6 +1,7 @@ import { Component, Inject, Injectable, Type } from '@angular/core' import { MatDialog } from '@angular/material/dialog' import { Observable, of } from 'rxjs' +import { map } from 'rxjs/operators' import { InterstitialsService } from 'src/app/cdk/interstitials/interstitials.service' import { UserRecord } from 'src/app/types/record.local' @@ -12,6 +13,7 @@ import { QaFlagsService } from '../../qa-flag/qa-flag.service' import { TogglzService } from '../../togglz/togglz.service' import { TogglzFlag } from 'src/app/types/config.endpoint' import { LoginBaseInterstitialManagerService } from '../abstractions/login-abstract-interstitial-manager.service' +import { AffiliationInterstitialOrganizationService } from '../affiliation-interstitial-organization.service' import { AffiliationsInterstitialComponent } from 'src/app/cdk/interstitials/affiliations-interstitial/interstitial-component/affiliations-interstitial.component' import { AffilationsComponentDialogInput, @@ -41,6 +43,7 @@ export class LoginAffiliationInterstitialManagerService extends LoginBaseInterst interstitialsService: InterstitialsService, togglzService: TogglzService, qaFlagService: QaFlagsService, + private affiliationOrganization: AffiliationInterstitialOrganizationService, @Inject(WINDOW) private _window: Window ) { // Pass dependencies to the parent @@ -71,7 +74,17 @@ export class LoginAffiliationInterstitialManagerService extends LoginBaseInterst if (userHasEmploymentAffiliation || isImpersonation || insideAnIframe) return of(false) - return of(true) + + // The interstitial's entire offer is "we think you work at X, add it to + // your record". Without a single X to name there is nothing to offer, so + // it must not open at all — eligibility is the only gate early enough to + // stop it, since being shown is what marks it as seen. + // + // A domain matching no ROR and a domain matching several both resolve to + // `undefined` here, which are exactly the two cases reported. + return this.affiliationOrganization + .resolveFromDomains(userRecord.emails.emailDomains) + .pipe(map((organization) => !!organization)) } // Return the dialog component that we want to display diff --git a/src/app/core/login-interstitials-manager/test/affiliation-interstitial-organization.service.spec.ts b/src/app/core/login-interstitials-manager/test/affiliation-interstitial-organization.service.spec.ts new file mode 100644 index 0000000000..2097c33180 --- /dev/null +++ b/src/app/core/login-interstitials-manager/test/affiliation-interstitial-organization.service.spec.ts @@ -0,0 +1,150 @@ +import { TestBed } from '@angular/core/testing' +import { of, throwError } from 'rxjs' + +import { OrganizationsService } from 'src/app/core/organizations/organizations.service' +import { RegisterService } from 'src/app/core/register/register.service' +import { AssertionVisibilityString } from 'src/app/types' +import { AffiliationInterstitialOrganizationService } from '../affiliation-interstitial-organization.service' + +describe('AffiliationInterstitialOrganizationService', () => { + let service: AffiliationInterstitialOrganizationService + let registerService: jasmine.SpyObj + let organizationsService: jasmine.SpyObj + + const domains = (...values: string[]): AssertionVisibilityString[] => + values.map( + (value, index) => + ({ + value, + createdDate: { timestamp: index }, + } as unknown as AssertionVisibilityString) + ) + + const orgDisambiguated = { + value: 'My University', + city: 'Bethesda', + region: 'MD', + country: 'US', + sourceId: '02mpq6x41', + disambiguatedAffiliationIdentifier: '02mpq6x41', + } + + beforeEach(() => { + registerService = jasmine.createSpyObj('RegisterService', [ + 'getEmailCategory', + ]) + organizationsService = jasmine.createSpyObj( + 'OrganizationsService', + ['getOrgDisambiguated'] + ) + + TestBed.configureTestingModule({ + providers: [ + AffiliationInterstitialOrganizationService, + { provide: RegisterService, useValue: registerService }, + { provide: OrganizationsService, useValue: organizationsService }, + ], + }) + + service = TestBed.inject(AffiliationInterstitialOrganizationService) + }) + + describe('mostRecentDomain', () => { + it('picks the domain with the newest createdDate', () => { + expect( + service.mostRecentDomain(domains('old.edu', 'new.edu'))?.value + ).toEqual('new.edu') + }) + + it('returns undefined for an empty or missing list', () => { + expect(service.mostRecentDomain([])).toBeUndefined() + expect(service.mostRecentDomain(undefined)).toBeUndefined() + }) + }) + + describe('resolveFromDomains', () => { + it('resolves the organization when the domain matches exactly one ROR', (done) => { + registerService.getEmailCategory.and.returnValue( + of({ category: 'PROFESSIONAL', rorId: '02mpq6x41' } as any) + ) + organizationsService.getOrgDisambiguated.and.returnValue( + of(orgDisambiguated as any) + ) + + service.resolveFromDomains(domains('my.edu')).subscribe((org) => { + expect(organizationsService.getOrgDisambiguated).toHaveBeenCalledWith( + 'ROR', + '02mpq6x41' + ) + expect(org.value).toEqual('My University') + expect(org.sourceId).toEqual('02mpq6x41') + done() + }) + }) + + // find-category omits rorId for an unknown domain and for one matching + // several RORs alike, which is what PD-13050 reported. + it('emits undefined, without an org lookup, when no rorId comes back', (done) => { + registerService.getEmailCategory.and.returnValue( + of({ category: 'UNDEFINED' } as any) + ) + + service.resolveFromDomains(domains('unknown.edu')).subscribe((org) => { + expect(org).toBeUndefined() + expect(organizationsService.getOrgDisambiguated).not.toHaveBeenCalled() + done() + }) + }) + + it('emits undefined when the ROR resolves to no organization', (done) => { + registerService.getEmailCategory.and.returnValue( + of({ category: 'PROFESSIONAL', rorId: '02mpq6x41' } as any) + ) + organizationsService.getOrgDisambiguated.and.returnValue(of(null)) + + service.resolveFromDomains(domains('my.edu')).subscribe((org) => { + expect(org).toBeUndefined() + done() + }) + }) + + it('emits undefined when the lookup fails', (done) => { + registerService.getEmailCategory.and.returnValue( + throwError(() => new Error('boom')) + ) + + service.resolveFromDomains(domains('my.edu')).subscribe((org) => { + expect(org).toBeUndefined() + done() + }) + }) + + it('emits undefined when the record has no domains', (done) => { + service.resolveFromDomains([]).subscribe((org) => { + expect(org).toBeUndefined() + expect(registerService.getEmailCategory).not.toHaveBeenCalled() + done() + }) + }) + + it('asks the backend once per domain, so eligibility and the interstitial share one answer', (done) => { + registerService.getEmailCategory.and.returnValue( + of({ category: 'PROFESSIONAL', rorId: '02mpq6x41' } as any) + ) + organizationsService.getOrgDisambiguated.and.returnValue( + of(orgDisambiguated as any) + ) + + service.resolveFromDomains(domains('my.edu')).subscribe(() => { + service.resolveFromDomains(domains('my.edu')).subscribe((org) => { + expect(registerService.getEmailCategory).toHaveBeenCalledTimes(1) + expect( + organizationsService.getOrgDisambiguated + ).toHaveBeenCalledTimes(1) + expect(org.value).toEqual('My University') + done() + }) + }) + }) + }) +}) diff --git a/src/app/core/login-interstitials-manager/test/login-affiliation-interstitials-manager.service.spec.ts b/src/app/core/login-interstitials-manager/test/login-affiliation-interstitials-manager.service.spec.ts index ccc7851f69..8a90b33ec3 100644 --- a/src/app/core/login-interstitials-manager/test/login-affiliation-interstitials-manager.service.spec.ts +++ b/src/app/core/login-interstitials-manager/test/login-affiliation-interstitials-manager.service.spec.ts @@ -12,6 +12,8 @@ import { LoginAffiliationInterstitialManagerService } from '../implementations/l import { AffiliationsInterstitialDialogComponent } from 'src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component' import { WINDOW_PROVIDERS } from 'src/app/cdk/window' import { TogglzFlag } from 'src/app/types/config.endpoint' +import { AffiliationInterstitialOrganizationService } from '../affiliation-interstitial-organization.service' +import { Organization } from 'src/app/types/common.endpoint' describe('LoginAffiliationInterstitialManagerService', () => { let service: LoginAffiliationInterstitialManagerService @@ -21,6 +23,7 @@ describe('LoginAffiliationInterstitialManagerService', () => { let mockInterstitialsService: jasmine.SpyObj let mockTogglzService: jasmine.SpyObj let mockQaFlagsService: jasmine.SpyObj + let mockAffiliationOrganizationService: jasmine.SpyObj beforeEach(() => { mockMatDialog = jasmine.createSpyObj('MatDialog', ['open']) @@ -35,6 +38,15 @@ describe('LoginAffiliationInterstitialManagerService', () => { 'QaFlagsService', ['isFlagEnabled'] ) + mockAffiliationOrganizationService = + jasmine.createSpyObj( + 'AffiliationInterstitialOrganizationService', + ['resolveFromDomains'] + ) + // Default: the domain maps to exactly one organization. + mockAffiliationOrganizationService.resolveFromDomains.and.returnValue( + of({ value: 'My University' } as Organization) + ) TestBed.configureTestingModule({ providers: [ @@ -43,6 +55,10 @@ describe('LoginAffiliationInterstitialManagerService', () => { { provide: TogglzService, useValue: mockTogglzService }, { provide: InterstitialsService, useValue: mockInterstitialsService }, { provide: QaFlagsService, useValue: mockQaFlagsService }, + { + provide: AffiliationInterstitialOrganizationService, + useValue: mockAffiliationOrganizationService, + }, WINDOW_PROVIDERS, ], }) @@ -118,6 +134,48 @@ describe('LoginAffiliationInterstitialManagerService', () => { done() }) }) + + // PD-13050. The interstitial has nothing to offer without a single + // matching organization, and showing it is what marks it as seen — so it + // has to be ruled out here, before anything is recorded. + it('should return false if the email domain resolves to no organization', (done) => { + mockAffiliationOrganizationService.resolveFromDomains.and.returnValue( + of(undefined) + ) + const userRecord = { + emails: { emailDomains: ['myuniversity.edu'] }, + affiliations: [], + } as unknown as UserRecord + + service + .userIsElegibleForInterstitial(userRecord) + .subscribe((isEligible) => { + expect(isEligible).toBeFalse() + done() + }) + }) + + it('should not look up an organization for a user already ruled out', (done) => { + const userRecord = { + emails: { emailDomains: ['myuniversity.edu'] }, + affiliations: [ + { + type: 'EMPLOYMENT', + affiliationGroup: ['Some Employment Data'], + }, + ], + } as unknown as UserRecord + + service + .userIsElegibleForInterstitial(userRecord) + .subscribe((isEligible) => { + expect(isEligible).toBeFalse() + expect( + mockAffiliationOrganizationService.resolveFromDomains + ).not.toHaveBeenCalled() + done() + }) + }) }) describe('getDialogComponentToShow', () => {