Skip to content

PD-5900 add backup email interstitial - #2887

Open
cryptalith wants to merge 2 commits into
mainfrom
lmendoa/PD-5900
Open

PD-5900 add backup email interstitial#2887
cryptalith wants to merge 2 commits into
mainfrom
lmendoa/PD-5900

Conversation

@cryptalith

Copy link
Copy Markdown
Member

What

Adds the backup email interstitial (PD-5900, Figma): after standard sign in, users with exactly one email address on their record are prompted to add a backup.

It runs first in the interstitial chain (backup email → public domains → affiliations) and is gated by LOGIN_BACKUP_EMAIL_INTERSTITIAL. Sign in flow only — no OAUTH_ variant, so the authorize flow skips it.

Backend companion: ORCID/ORCID-Source#7597 (the togglz enum constant, nothing else).

Reviewer note — this PR is wider than the ticket

Three changes here are not interstitial specific and are worth reviewing on their own terms. Happy to split any of them into a separate PR if you'd prefer:

  1. XsrfPreloadInterceptor now runs on every browser (was Firefox only). See below — it touches every HTTP request in the app.
  2. showInterstitialAsDialog no longer gates the dialog on the flag POST (shared by all three interstitials).
  3. The shown RUM event is emitted from the shared manager, so domains and affiliations also start reporting it.

The interstitial

Panel follows the Figma, including the primary email with its category — Professional (work icon), Personal (fingerprint) or no label when uncategorized — resolved via the existing email-domain/find-category endpoint that the affiliations interstitial already uses.

Validation covers the three AC errors. Two behaviours are deliberate and differ from the existing interstitials:

  • The dialog stays open on an invalid form. The other two fall through and close; the AC requires inline errors.
  • An address already on the user's own record is rejected locally. validateEmail.json validates with ignoreCurrentUser=true, so it returns clean for your own address — without this check the UI reported success while setEmails diffed it as "not new" and silently dropped it.

POST account/emails.json replaces the whole email set, so the save posts the existing emails alongside the new one. The confirmation is a success notice at the top of the record, matching the other interstitials.

Accessibility (this dialog only)

The audit found real WCAG failures inherited from the shared config. Fixed here for backup email; the same defects remain in domains and affiliations and want a follow-up ticket:

  • No accessible name at all (WCAG 4.1.2). mat-card-title does not feed Material's labelling queue — only mat-dialog-title does — so aria-label and aria-labelledby were both null. Now labelled.
  • aria-modal was false, leaving the record behind exposed to AT while disableClose blocks Esc.
  • Keyboard users could not dismiss it. The decline control was a bare <a> with no href, which cannot receive focus. It is now a <button>.
  • Errors were silent — they sit outside mat-form-field, so Material's aria-describedby wiring never ran. Now role="alert" plus an explicit link.
  • Panel padding moved to the dialog surface: the old :host.columns-12 rule only applied on desktop, so mobile rendered with no padding at all.

Observability

First analytics in this subsystem. New interstitial RUM journey: shown, backup_added, dismissed, validation_error, save_error, correlated by system_journeyId with system_elapsedMs as time to decision, so add rate and dismiss rate are single queries.

Attribute keys avoid orcid/email/pid/delegator — the RUM sanitizer redacts any key matching those, so an obvious name like backupEmail would have arrived unusable. No address is ever sent. Docs: src/app/rum/interstitial-observability.md.

XSRF preload, now all browsers

FirefoxXsrfPreloadInterceptorXsrfPreloadInterceptor, moved to core/xsrf/ next to the fallback interceptor.

The old comment claimed other browsers were fine. They are not: Angular only attaches x-xsrf-token when the cookie already exists, so on a cold start the first mutating request goes out unauthenticated, 403s, and merely sets the cookie for next time. That is why addInterstitialFlag failed, and why local sign in often needed two attempts — signin/auth.json fails the same way.

Removing the browser gate also exposed a hard window.location.reload() to every browser, which would loop if the backend never issues the cookie. It is now guarded to fire at most once, still reporting to RUM either way, and csrf.json is excluded so the preload cannot gate on itself.

Testing

  • 719 unit tests passing, production AOT build clean, yarn build:i18n run.
  • Verified end to end against a local stack (JDK 21 / Tomcat 11) with the togglz enabled: eligibility, all three error states, the own-address guard, save, decline, the success notice, and ordering against the other interstitials.
  • Confirmed after the XSRF change that the cookie is present on page load and addInterstitialFlag returns 200 and persists — it previously 403'd every time.
  • Keyboard and AT checks done in the browser: aria-label and aria-modal="true" present, decline reachable by keyboard, errors carry role="alert", mobile padding restored.

Translations

New @@interstitial.* and @@topBar.* ids extracted into src/locale. Translations are needed before enabling in production; the two error messages reuse existing @@side-bar.* ids.

Rollout

Toggle defaults off. Enable in QA first. Rollback is flipping it off — with one caveat: the XSRF and dialog-resilience changes are not behind the toggle.

QA steps, including reading the metrics from the browser console without New Relic: orcid-constiution/docs/playbooks/qa-backup-email-interstitial.md.

Shows an interstitial after standard sign in to users with exactly one email
address on their record, prompting them to add a backup. It runs first in the
interstitial chain (backup email, then public domains, then affiliations) and
is gated by the LOGIN_BACKUP_EMAIL_INTERSTITIAL togglz. Sign in flow only.

Panel follows the Figma: primary email with its Professional/Personal category
(from email-domain/find-category), backup email field, and the three inline
errors from the AC. The confirmation is a success notice at the top of the
record, matching the other interstitials.

Accessibility fixes for this dialog: it now has an accessible name and
aria-modal, the decline control is a button so keyboard users can dismiss it
(disableClose blocks Esc), and inline errors are announced. Panel padding moved
to the dialog surface, which also had no padding at all on mobile.

Adds the first analytics for the interstitial subsystem: an 'interstitial' RUM
journey reporting shown, added, dismissed, validation error and save error, so
add and dismiss rates are measurable. The shown event is emitted from the shared
manager, so all three interstitials get a comparable denominator.

Also makes the XSRF preload interceptor run on every browser instead of only
Firefox. Without a cookie the first mutating request goes out with no
x-xsrf-token header, 403s, and only sets the cookie as a side effect, which made
addInterstitialFlag fail and local sign in take two attempts. The reload it
falls back to is now guarded so it cannot loop. Recording an interstitial visit
no longer gates the dialog opening either, so a failed flag POST cannot hide the
interstitial while marking it as seen.
@amontenegro

Copy link
Copy Markdown
Member

Material reserves a fixed block below every mat-form-field for hints and
errors. The inline errors here are siblings outside the field, following the
convention used elsewhere, so that reserved space was only a gap. Reuses the
global no-hint class to drop it.

Label, field and message now sit in an 8px flex stack, matching the design,
instead of 4px above the field and a large gap below.

Also splits the two error reds the design distinguishes: the label and field
outline take state-warning-darkest, the message state-warning-dark. Adds the
explicit 18px line height, since the orc-font-* classes never set one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants