Skip to content

[PM-40867] feat: Show email verification status and verify action on My account settings page - #22387

Draft
enmande wants to merge 9 commits into
mainfrom
auth/pm-40867/show-email-verification-status-on-my-account-settings
Draft

[PM-40867] feat: Show email verification status and verify action on My account settings page#22387
enmande wants to merge 9 commits into
mainfrom
auth/pm-40867/show-email-verification-status-on-my-account-settings

Conversation

@enmande

@enmande enmande commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-40867

📔 Objective

  • Adds a verification status indicator and CTA to the Email field on Settings > My account, so users can see whether their email is verified and trigger verification from there.
    • Reuses the existing re-send and confirm flows.
  • Removes the "verify your email" vault banner and its now-unused verifyEmailDesc locale key.

📸 Screenshots

Full Exercise

Full exercise showing verification flow as well as "Verified ✅" badge reactivity; if a stale "Verify Email" CTA is displayed (for example, on an old tab from before verification was performed), subsequent clicks will surface the "already verified" error from the server as an info toast and the state is updated.

pm-40867__full-exercise.mov

Verified Badge Appears, Banner is Removed

Screen.Recording.2026-08-12.at.3.56.53.PM.mov

Verified Badge

Screenshot 2026-08-12 at 2 35 47 PM

Verify Email Button

Screenshot 2026-08-12 at 4 29 43 PM

Rate Limiting

Verify email is rate-limited to 5 calls/60 minutes (server opinion).

pm-40867__rate-limiting.mov

@enmande enmande added ai-review Request a Claude code review t:feature Change Type - Feature Development labels Aug 12, 2026
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed after 5cf8806, which only merges main into the branch — the PR's own changes are unchanged since the previous approving pass, and the single earlier finding remains resolved by the optional chaining on ErrorResponse.message.

Re-verified post-merge that nothing in main reintroduced coupling to the removed banner: no remaining references to VerifyEmailComponent, shouldShowVerifyEmailBanner, VisibleVaultBanner.VerifyEmail, or verifyEmailDesc in code (non-en locale files retaining the key is normal Crowdin behavior), and firstValueFrom is still used in vault-banners.service.ts after dropping the map import. Also confirmed emailVerified derives reactively from activeAccount$, ngOnInit refreshes AccountService from the authoritative profile fetch via the existing setAccountEmailVerified abstraction, the form renders only after loading() clears (so no stale CTA flash), and the verified / emailAlreadyVerified keys plus the slot="inline-end" projection on bit-form-field all exist.

Code Review Details

No findings.

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.00%. Comparing base (e1c772a) to head (5cf8806).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...src/app/auth/settings/account/profile.component.ts 50.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #22387      +/-   ##
==========================================
+ Coverage   53.99%   54.00%   +0.01%     
==========================================
  Files        4306     4305       -1     
  Lines      136674   136654      -20     
  Branches    21585    21582       -3     
==========================================
+ Hits        73793    73801       +8     
+ Misses      57504    57472      -32     
- Partials     5377     5381       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@enmande
enmande marked this pull request as ready for review August 13, 2026 15:56
@enmande
enmande requested review from a team as code owners August 13, 2026 15:56
Comment thread apps/web/src/app/auth/settings/account/profile.component.ts Outdated
…ified" server error as a localized info message.
Comment thread apps/web/src/app/auth/settings/account/profile.component.ts Outdated
enmande and others added 2 commits August 14, 2026 10:47
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work! Thank you for the re-addition of the translated error + the component tests that are useful in this case! :shipit:

@enmande
enmande marked this pull request as draft August 18, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants