PD-5900 add backup email interstitial - #2887
Open
cryptalith wants to merge 2 commits into
Open
Conversation
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.
Member
|
Task linked: PD-5900 [Interstitial] Add backup email address |
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 — noOAUTH_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:
XsrfPreloadInterceptornow runs on every browser (was Firefox only). See below — it touches every HTTP request in the app.showInterstitialAsDialogno longer gates the dialog on the flag POST (shared by all three interstitials).shownRUM 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-categoryendpoint that the affiliations interstitial already uses.Validation covers the three AC errors. Two behaviours are deliberate and differ from the existing interstitials:
validateEmail.jsonvalidates withignoreCurrentUser=true, so it returns clean for your own address — without this check the UI reported success whilesetEmailsdiffed it as "not new" and silently dropped it.POST account/emails.jsonreplaces 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:
mat-card-titledoes not feed Material's labelling queue — onlymat-dialog-titledoes — soaria-labelandaria-labelledbywere both null. Now labelled.aria-modalwas false, leaving the record behind exposed to AT whiledisableCloseblocks Esc.<a>with nohref, which cannot receive focus. It is now a<button>.mat-form-field, so Material'saria-describedbywiring never ran. Nowrole="alert"plus an explicit link.:host.columns-12rule only applied on desktop, so mobile rendered with no padding at all.Observability
First analytics in this subsystem. New
interstitialRUM journey:shown,backup_added,dismissed,validation_error,save_error, correlated bysystem_journeyIdwithsystem_elapsedMsas 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 likebackupEmailwould have arrived unusable. No address is ever sent. Docs:src/app/rum/interstitial-observability.md.XSRF preload, now all browsers
FirefoxXsrfPreloadInterceptor→XsrfPreloadInterceptor, moved tocore/xsrf/next to the fallback interceptor.The old comment claimed other browsers were fine. They are not: Angular only attaches
x-xsrf-tokenwhen 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 whyaddInterstitialFlagfailed, and why local sign in often needed two attempts —signin/auth.jsonfails 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, andcsrf.jsonis excluded so the preload cannot gate on itself.Testing
yarn build:i18nrun.addInterstitialFlagreturns 200 and persists — it previously 403'd every time.aria-labelandaria-modal="true"present, decline reachable by keyboard, errors carryrole="alert", mobile padding restored.Translations
New
@@interstitial.*and@@topBar.*ids extracted intosrc/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.