PD-13050 don't show the affiliation interstitial without a matching ROR

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<AffiliationsInterstitialComponent>
+  let affiliationOrganization: jasmine.SpyObj<AffiliationInterstitialOrganizationService>
+
+  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>(
+        '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)
+  })
 })

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,49 +88,59 @@ export class AffiliationsInterstitialComponent implements OnInit, OnDestroy {
   $destroy: Subject<void> = new Subject<void>()
   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
   ) {}
 
   ngOnInit(): void {
     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()
   }
 
   /**

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<MatDialogRef<any>>
+  let affiliationOrganization: jasmine.SpyObj<AffiliationInterstitialOrganizationService>
+
+  const domain = { value: 'my.edu', createdDate: { timestamp: 1 } }
+  const organization = { value: 'My University' } as Organization
+
+  beforeEach(() => {
+    dialogRef = jasmine.createSpyObj<MatDialogRef<any>>('MatDialogRef', [
+      'close',
+    ])
+    affiliationOrganization =
+      jasmine.createSpyObj<AffiliationInterstitialOrganizationService>(
+        '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,
+    })
+  })
+})

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
     )
   }