feat(profile): add meeting-invitation email preference - #1073
Conversation
WalkthroughAdds a preferred meeting-invitation email override across shared contracts, backend NATS-backed profile APIs, a client service wrapper, and account settings UI controls. ChangesMeeting invite email preference
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AccountSettingsComponent
participant UserService
participant ProfileController
participant MeetingPreferenceService
participant NATS
User->>AccountSettingsComponent: Select meeting-invite email
AccountSettingsComponent->>UserService: Set preference
UserService->>ProfileController: PUT meeting-invite preference
ProfileController->>MeetingPreferenceService: Update preference
MeetingPreferenceService->>NATS: Request preferred email update
NATS-->>MeetingPreferenceService: Update result
MeetingPreferenceService-->>ProfileController: Preference result
ProfileController-->>UserService: HTTP response
UserService-->>AccountSettingsComponent: Updated preference
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
fayazg
added
the
do-not-merge
There was a problem hiding this comment.
Pull request overview
Adds a user-selectable meeting-invitation email preference to Account Settings. The per-email row actions (Make Primary / Delete) are consolidated into a ... kebab menu, a new Meeting Invitations action lets a user pick which verified email receives meeting invites (backed by the meeting-service preferred_email.{get,set} NATS RPC forwarding the user's v1 API-gateway token), and a green Meeting Invites badge marks the selected address. The feature fails soft (no badge) when the RPC is unavailable, and depends on upstream LFXV2-2599 being deployed for end-to-end function.
Changes:
- New shared
MeetingInviteEmailinterface and twoNatsSubjectsentries for the meeting-service preferred-email RPC. - New server
MeetingPreferenceService+ twoProfileControllerendpoints/routes (GET/PUT /api/profile/emails/meeting-invite) that forwardreq.apiGatewayToken. - Angular
UserServicemethods plus Account Settings kebab-menu refactor,Meeting Invitesbadge, and per-row menu-item map.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/user-profile.interface.ts | Adds MeetingInviteEmail interface (nullable email_id/email). |
| packages/shared/src/enums/nats.enum.ts | Adds MEETING_PREFERRED_EMAIL_GET/SET subjects with token-envelope note. |
| apps/lfx-one/src/server/services/meeting-preference.service.ts | New NATS proxy service for get/set preferred meeting-invite email. |
| apps/lfx-one/src/server/routes/profile.route.ts | Registers GET/PUT meeting-invite routes (PUT guarded during impersonation). |
| apps/lfx-one/src/server/controllers/profile.controller.ts | Adds controller handlers with email validation and v1-token gating. |
| apps/lfx-one/src/app/shared/services/user.service.ts | Adds client HTTP methods (get fails soft; set surfaces errors). |
| apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts | Adds invite-email signal, per-row metadata flags, and kebab menu-item map. |
| apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html | Replaces inline action buttons with kebab menu; adds Meeting Invites badge. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/lfx-one/src/server/services/meeting-preference.service.ts (1)
97-116: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse Nats error codes here.
nats@^2.29.3exposesNatsError.code(for exampleTIMEOUTandNO_RESPONDERS), so this branch should key off the code instead oferror.message.includes('timeout') || error.message.includes('503'); message text is not a stable contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/services/meeting-preference.service.ts` around lines 97 - 116, The error handling in the catch block of meeting-preference.service.ts should rely on NATS error codes instead of inspecting error.message text. Update the logic around the logger.warning call in the meeting preference handler to detect NatsError.code values such as TIMEOUT and NO_RESPONDERS, and use those codes to decide when to return the temporary-unavailable response. Keep the existing fallback response for all other errors, and preserve the current request context and error logging.apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts (1)
485-509: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
catchErrorininitMeetingInviteData.
userService.getMeetingInviteEmail()already catches its own errors and resolves to{ email_id: null, email: null }, so the observable never errors. ThecatchError(() => of(null))at Line 487 is unreachable and can be dropped for clarity.♻️ Proposed simplification
- return toSignal(this.emailRefresh.pipe(switchMap(() => this.userService.getMeetingInviteEmail().pipe(catchError(() => of(null))))), { initialValue: null }); + return toSignal(this.emailRefresh.pipe(switchMap(() => this.userService.getMeetingInviteEmail())), { initialValue: null });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts` around lines 485 - 509, The redundant error handler in initMeetingInviteData is unreachable because userService.getMeetingInviteEmail already converts failures into a null-like result, so remove the extra catchError(() => of(null)) from the toSignal pipeline. Keep the existing switchMap and initialValue logic intact, and ensure initMeetingInviteData still returns a Signal<MeetingInviteEmail | null> with the same behavior.apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html (1)
111-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce duplicate
menuItemsMap().get(...)lookups.The signal/map lookup at Line 111 (
@ifcondition) and Line 123 ([model]binding) is duplicated. Angular's@ifsupports anasalias to compute it once.♻️ Proposed refactor using `as` alias
- `@if` ((menuItemsMap().get(emailItem.email) ?? []).length > 0) { + `@if` ((menuItemsMap().get(emailItem.email) ?? []); as emailMenuItems) { + `@if` (emailMenuItems.length > 0) { <div class="relative inline-block"> <lfx-button icon="fa-solid fa-ellipsis-vertical" [text]="true" [rounded]="true" size="small" [disabled]="impersonating()" [ariaLabel]="impersonating() ? 'This action is unavailable while impersonating another user' : 'Email actions'" styleClass="!text-slate-400 hover:!text-slate-600 hover:!bg-slate-100" (onClick)="emailMenu.toggle($event)" [attr.data-testid]="'email-menu-' + emailItem.email" /> - <lfx-menu `#emailMenu` [model]="menuItemsMap().get(emailItem.email) ?? []" [popup]="true" appendTo="body" /> + <lfx-menu `#emailMenu` [model]="emailMenuItems" [popup]="true" appendTo="body" /> </div> + } }Note: the HTMLHint
spec-char-escapehint on Line 111 (flagging>) is a known false positive for Angular control-flow@ifexpressions and can be disregarded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html` around lines 111 - 124, The `account-settings.component.html` template repeats the same `menuItemsMap().get(emailItem.email) ?? []` lookup in both the `@if` condition and the `lfx-menu` `[model]` binding. Refactor the `@if` block to use an `as` alias so the computed menu items are stored once and reused inside the block, updating the `emailMenu` and `menuItemsMap` usage accordingly.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/lfx-one/src/server/controllers/profile.controller.ts`:
- Around line 513-542: The getMeetingInviteEmail flow currently uses
req.apiGatewayToken, which can resolve the impersonator’s /v1/me instead of the
viewed profile’s user. Update the getMeetingInviteEmail handler in
profile.controller.ts to block this endpoint during impersonation using the same
impersonation guard pattern used elsewhere, or change
MeetingPreferenceService.getMeetingInviteEmail to accept a target-scoped
credential if available. Ensure the check happens before calling
this.meetingPreferenceService.getMeetingInviteEmail and returns a clear
rejection/forbidden response when impersonating.
In `@apps/lfx-one/src/server/services/meeting-preference.service.ts`:
- Line 78: The warning-level logs in meeting-preference.service still emit the
raw email, which will end up in production logs on failed preference updates.
Update the logging in the set-preference flow (including the warning paths
around the NATS request handling in the service methods that currently call
logger.warning) to remove or obfuscate the email before logging, while keeping
the debug log in set_meeting_invite_email unchanged or similarly sanitized.
Prefer a hashed or redacted value in the warning payload so the log remains
useful without exposing the user’s address.
- Around line 35-62: The getMeetingInviteEmail flow currently returns null for
both “no preference” and NATS/request failures, so outages are indistinguishable
downstream. Update MeetingPreferenceService.getMeetingInviteEmail to return a
distinct failure signal on exceptions/timeouts and NATS errors (for example a
discriminated result or error state instead of null), while keeping the existing
success shape for parsed email data. Then adjust the controller/service callers
that consume getMeetingInviteEmail to branch on the new failure case separately
from the “no override” case.
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html`:
- Around line 111-124: The `account-settings.component.html` template repeats
the same `menuItemsMap().get(emailItem.email) ?? []` lookup in both the `@if`
condition and the `lfx-menu` `[model]` binding. Refactor the `@if` block to use
an `as` alias so the computed menu items are stored once and reused inside the
block, updating the `emailMenu` and `menuItemsMap` usage accordingly.
In
`@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts`:
- Around line 485-509: The redundant error handler in initMeetingInviteData is
unreachable because userService.getMeetingInviteEmail already converts failures
into a null-like result, so remove the extra catchError(() => of(null)) from the
toSignal pipeline. Keep the existing switchMap and initialValue logic intact,
and ensure initMeetingInviteData still returns a Signal<MeetingInviteEmail |
null> with the same behavior.
In `@apps/lfx-one/src/server/services/meeting-preference.service.ts`:
- Around line 97-116: The error handling in the catch block of
meeting-preference.service.ts should rely on NATS error codes instead of
inspecting error.message text. Update the logic around the logger.warning call
in the meeting preference handler to detect NatsError.code values such as
TIMEOUT and NO_RESPONDERS, and use those codes to decide when to return the
temporary-unavailable response. Keep the existing fallback response for all
other errors, and preserve the current request context and error logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 766d66b1-6ce5-409a-bf41-5d7eed226d22
📒 Files selected for processing (8)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.htmlapps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.tsapps/lfx-one/src/app/shared/services/user.service.tsapps/lfx-one/src/server/controllers/profile.controller.tsapps/lfx-one/src/server/routes/profile.route.tsapps/lfx-one/src/server/services/meeting-preference.service.tspackages/shared/src/enums/nats.enum.tspackages/shared/src/interfaces/user-profile.interface.ts
Address review comments from coderabbitai[bot]: - profile.route.ts: block GET /emails/meeting-invite during impersonation — req.apiGatewayToken is session-scoped, so the lookup would resolve the impersonator's preference instead of the target's (per coderabbitai[bot]) - meeting-preference.service.ts: drop raw email from the two warning-level logs, which emit in production and would persist PII (per coderabbitai[bot]) - profile.controller.ts: drop raw email from the INFO success log for the same reason; email remains only in DEBUG (dev-only) logs Resolves 2 review threads. LFXV2-2599 Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Review Feedback AddressedCommit: 27f34f6 Changes Made
No Change Needed
Threads Resolved2 of 3 unresolved threads resolved this iteration; 1 left open (declined with rationale, awaiting reviewer confirmation). |
Move the per-email Make Primary and Delete actions into a vertical kebab menu and add a Meeting Invitations option backed by the meeting-service preferred_email NATS RPC. A "Meeting Invites" badge marks the selected email. The RPC forwards the user's v1 API-gateway token. LFXV2-2599 Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Address review comments from coderabbitai[bot]: - profile.route.ts: block GET /emails/meeting-invite during impersonation — req.apiGatewayToken is session-scoped, so the lookup would resolve the impersonator's preference instead of the target's (per coderabbitai[bot]) - meeting-preference.service.ts: drop raw email from the two warning-level logs, which emit in production and would persist PII (per coderabbitai[bot]) - profile.controller.ts: drop raw email from the INFO success log for the same reason; email remains only in DEBUG (dev-only) logs Resolves 2 review threads. LFXV2-2599 Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
|
Rebased onto latest Conflict resolved: single import-block collision in Both commits replayed with GPG signature + DCO sign-off intact. |
PR SummaryMedium Risk Overview Per-email row actions move from separate Make Primary / Delete buttons into a kebab menu (aligned with other profile tabs). The menu adds Meeting Invitations to set which verified address receives invites; a Meeting Invites badge appears only when the user has an explicit override (null preference still means primary, with no badge). Server-side: new Reviewed by Cursor Bugbot for commit e2812e1. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html (1)
114-127: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExpose the email menu state to assistive technology.
MenuComponentexposesonShowandonHidebut no visibility property. Track the menu state from these events, and extendButtonComponentto forwardaria-haspopup="menu"andaria-expandedto its internal<p-button>. Addaria-controlswhen the menu has a stable ID.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html` around lines 114 - 127, Track each email menu’s open state using the visible lfx-menu onShow and onHide events, and assign a stable unique ID to the menu. Extend ButtonComponent’s inputs and internal p-button bindings to forward aria-haspopup="menu", aria-expanded, and aria-controls, then bind these values on the email-menu trigger so aria-expanded reflects the tracked state and aria-controls references the menu ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html`:
- Around line 114-127: Track each email menu’s open state using the visible
lfx-menu onShow and onHide events, and assign a stable unique ID to the menu.
Extend ButtonComponent’s inputs and internal p-button bindings to forward
aria-haspopup="menu", aria-expanded, and aria-controls, then bind these values
on the email-menu trigger so aria-expanded reflects the tracked state and
aria-controls references the menu ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d81adfe7-9c7f-4a9b-b7bc-22df1ba26a2e
📒 Files selected for processing (3)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.htmlapps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.tsapps/lfx-one/src/app/shared/services/user.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts
- apps/lfx-one/src/app/shared/services/user.service.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (7)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts:301
- Choosing the primary address does not clear the override. This always sends the address, while the meeting-service contract only clears for an empty/null selection or the
"primary"sentinel; a normal primary address is persisted as an explicit override, so the Meeting Invites badge remains. Send a distinct reset value when the selected row is primary and pass it through the BFF.
this.userService.setMeetingInviteEmail(email.email).subscribe({
apps/lfx-one/src/server/controllers/profile.controller.ts:580
- This writes the raw email address to request debug metadata.
logger.startOperationdoes not sanitize metadata automatically, and Pino's configured redaction does not include email, so enabling debug logging persists PII. Omit the address here; the operation name and request ID are sufficient.
const startTime = logger.startOperation(req, 'set_meeting_invite_email', { email: emailAddress });
apps/lfx-one/src/server/controllers/profile.controller.ts:579
- The type assertion does not validate runtime input.
RegExp.teststring-coerces values, so{ "email": ["a@b.com"] }passesEMAIL_REGEXbut forwards an array even though the RPC contract requires a string. Require a string before format validation.
This issue also appears on line 580 of the same file.
const emailAddress = (req.body?.email as string) ?? '';
apps/lfx-one/src/server/services/meeting-preference.service.ts:52
- An upstream
{ error }is converted tonull, butnullis also the valid “no override” state. The controller then returns HTTP 200 with null fields (and the client also masks failures), so an outage or authorization failure makes an existing preference disappear and logs the read as successful. Propagate the failure and reserve null fields for a successful null reply.
if (parsed.error) {
logger.warning(req, 'get_meeting_invite_email', 'NATS preferred_email.get returned an error', {
error: parsed.error,
});
return null;
apps/lfx-one/src/server/services/meeting-preference.service.ts:78
- This debug metadata contains the user's raw email address. The logger does not automatically apply
sanitize, and Pino's redaction list excludes email, so debug-enabled environments persist PII. Omit the metadata rather than forwarding the address to logging.
logger.debug(req, 'set_meeting_invite_email', 'Setting preferred meeting-invite email via NATS', { email });
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts:110
- The meeting-service resolves addresses case-insensitively and returns the SFDC record's casing, but this compares that value exactly against the Auth0 email list. If casing differs, the selected row gets no badge and still offers Meeting Invitations. Normalize both values before the invite-email comparisons.
This issue also appears on line 301 of the same file.
const inviteEmail = this.meetingInviteEmail();
apps/lfx-one/src/server/services/meeting-preference.service.ts:35
- This new token-forwarding RPC has no unit coverage even though server services already use Vitest (for example,
meeting.service.spec.ts). Add tests for the exact token/email payload, null and populated success replies,{ error }propagation, and transport failures; these contract-sensitive branches currently regress unnoticed.
public async getMeetingInviteEmail(req: Request, v1Token: string): Promise<MeetingInviteEmail | null> {
Summary
Settings → Email Settings into a vertical
...kebab menu, matching theprofile page's Work History / Identities tabs.
user's meeting-invitation address, backed by the meeting-service
lfx.meeting-service.preferred_email.{get,set}NATS RPC.badge on whichever email is currently selected (badge appears only once a
user has explicitly chosen; a null preference = using primary shows none).
req.apiGatewayToken),which the meeting service uses to act as the user against v1
/v1/me.Notes
MeetingInviteEmailinterface added to@lfx-one/shared; server proxy in a dedicatedmeeting-preference.service.default (no separate reset action).
user-service (SFDC), which is distinct from the Auth0 "Verified" state shown
in the UI. An Auth0-verified email not yet active/verified in SFDC will be
rejected by the meeting service — this is expected upstream behavior.
Review trade-offs
.spec.ts); testing isPlaywright E2E via
data-testid(email-menu-*,meeting-invites-badgeadded here). No new specs added to avoid introducing net-new test infra.
metadata, consistent with the existing
setPrimaryEmailsibling in the samefiles; the logger already redacts secrets. Left as-is for consistency and
debuggability rather than diverging in a single new method.
Dependencies
preferred_emailRPC (LFXV2-2599) tobe deployed for end-to-end function.
Jira: https://linuxfoundation.atlassian.net/browse/LFXV2-2599
🤖 Generated with Claude Code