feat(committees): add bulk email invite to create-group wizard - #1078
feat(committees): add bulk email invite to create-group wizard#1078manishdixitlfx wants to merge 7 commits into
Conversation
Wire AddMemberDialogComponent into the create-group wizard's Add Members step as a primary 'Invite by email' CTA, so users can paste a comma/semicolon/newline-separated email list instead of adding members one at a time. The existing MemberFormComponent path is retained as the secondary 'Add with details' action. Also fix the members filter box on Step 4: hide it when the group has no members yet (nothing to filter) and change its placeholder from 'Search members...' to 'Filter members…' so it can't be misread as an invite input on an empty group. LFXV2-2606 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
openInviteByEmailDialog passed existingInvites: [], making the dialog's already-invited dedupe inert. Fetch the committee's current invites before opening and pass them as existingInvites. If the fetch fails, fall back to a session-accumulated list of invites (deduped by email, refreshed on each successful dialog close) so repeat opens still dedupe. LFXV2-2606 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCommittee member editing now stages invite-by-email entries locally, displays pending invitations, and carries staged invite requests into the committee wizard for submission as invite operations. ChangesInvite by Email Flow
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
There was a problem hiding this comment.
Pull request overview
This PR wires a bulk email invite action into Step 4 (members) of the create-group wizard. It adds an "Invite by email" button that opens the existing AddMemberDialogComponent to invite one or more people by email; invites are sent immediately as pending committee invites (POST /invites). To make the dialog's already-invited dedupe effective, the component fetches the committee's current invites before opening and accumulates them in a session list as a fallback when the live fetch fails. The template is also cleaned up: the primary button is relabeled and the search/filter row is hidden until the group has members.
Changes:
- Add
openInviteByEmailDialog()/openInviteDialog()that openAddMemberDialogComponent, passing current members and invites for dedupe. - Add a session-accumulated invite list (
sessionInvites+rememberInvites()/refreshSessionInvites()) as a fallback source when the live invites fetch fails. - Template: add the "Invite by email" button, rename "Add Member" → "Add with details", and gate the search/filter row behind
@if (memberCount() > 0).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
committee-members-manager.component.ts |
Adds invite-by-email dialog wiring plus a session-accumulated, email-deduped invite fallback list feeding the dialog's already-invited dedupe. |
committee-members-manager.component.html |
Adds the "Invite by email" button, relabels the details button, and hides the search/filter row until members exist. |
Note: The accumulated invites are not filtered by status before being used for dedupe. Since getCommitteeInvites() returns invites of every status and the repo convention (committee-view.component.ts:777-781) is to keep only pending, declined/revoked invitees would be incorrectly blocked from re-invitation. See the inline comment for a self-contained fix.
💡 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.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts (2)
139-163: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo guard against concurrent invocations.
Rapid clicks on the "Invite by email" button trigger overlapping
getCommitteeInvitescalls with no in-flight guard, each of which independently resolves and callsopenInviteDialog(), potentially stacking multiple dialogs.🤖 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/committees/components/committee-members-manager/committee-members-manager.component.ts` around lines 139 - 163, The openInviteByEmailDialog flow does not prevent multiple concurrent executions, so repeated clicks can trigger overlapping getCommitteeInvites requests and openInviteDialog calls. Add an in-flight guard in CommitteeMembersManagerComponent around openInviteByEmailDialog (for example, a boolean flag or similar state) so a second invocation is ignored until the current invite load/subscription completes or errors, and make sure the guard is cleared in the subscription/error path before allowing another dialog open.
269-269: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePassing internal array by reference to dialog data.
existingInvites: this.sessionInvitesshares the live internal array reference with the opened dialog rather than a defensive copy. IfAddMemberDialogComponentever mutates this input, it would silently corrupt the parent's session state.🛡️ Defensive copy
- existingInvites: this.sessionInvites, + existingInvites: [...this.sessionInvites],🤖 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/committees/components/committee-members-manager/committee-members-manager.component.ts` at line 269, The dialog data is passing the live sessionInvites array by reference, which can let AddMemberDialogComponent mutate parent state unexpectedly. Update the data passed from committeeMembersManagerComponent’s dialog-opening logic so existingInvites uses a defensive copy instead of this.sessionInvites, keeping the parent’s sessionInvites isolated from any dialog-side changes.
🤖 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/committees/components/committee-members-manager/committee-members-manager.component.ts`:
- Around line 139-163: The openInviteByEmailDialog flow does not prevent
multiple concurrent executions, so repeated clicks can trigger overlapping
getCommitteeInvites requests and openInviteDialog calls. Add an in-flight guard
in CommitteeMembersManagerComponent around openInviteByEmailDialog (for example,
a boolean flag or similar state) so a second invocation is ignored until the
current invite load/subscription completes or errors, and make sure the guard is
cleared in the subscription/error path before allowing another dialog open.
- Line 269: The dialog data is passing the live sessionInvites array by
reference, which can let AddMemberDialogComponent mutate parent state
unexpectedly. Update the data passed from committeeMembersManagerComponent’s
dialog-opening logic so existingInvites uses a defensive copy instead of
this.sessionInvites, keeping the parent’s sessionInvites isolated from any
dialog-side changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0aa2a14-a8b6-4da4-a94c-391a9975e949
📒 Files selected for processing (2)
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts
jordane
left a comment
jordane
left a comment
There was a problem hiding this comment.
Invites send immediately. Canceling the wizard does not un-send them.
This seems like a non-starter for merging this. Why would a wizard form, which hasn't been submitted be taking action immediately on that page with no clear indication to the user? That's a recipe for a bad UX. I suggest considering a few alternatives:
- Fix it to send the bulk invites after the wizard is fully submitted, and not immediately action it
- OR move the bulk invite outside of the creation (it's useful after group creation too!) and then update the creation wizard to inform the user that they can initiate a bulk invite after the group is created.
Invites triggered from the create-group wizard's Add Members step were sent immediately (POST /invites) the moment the dialog was submitted, so cancelling or abandoning the wizard still fired real invitations with no clear indication to the user (PR #1078 review). Rework the bulk-invite flow to stage invites client-side and flush them only when the wizard is completed: - AddMemberDialogComponent gains a collectOnly mode that validates and returns the built invite payloads instead of POSTing them; the group management page keeps immediate-send behavior (default). - The members manager stages returned invites in a pendingInvites signal (deduped by email), surfaces them in a 'Pending invitations' section with per-row remove, and emits them via MemberPendingChanges.toInvite. - The wizard flushes toInvite through createCommitteeInvite in its Done handler, alongside member add/update/delete ops. Skip/Cancel sends nothing. LFXV2-2606 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
|
Thanks @jordane — you're right, immediate send from an unsubmitted wizard was the wrong behavior. Fixed in d04a122 by going with your option (1): invites are now staged, not sent, until the wizard is completed. What changed:
So the flow now matches the rest of Step 4 (members added via "Add with details" were already deferred to Done) — nothing fires until the explicit finish action. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.html (1)
45-54: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePrefer
(onClick)onlfx-buttonhere. It matches the component’s public output and keeps this handler aligned with the button API used in other places.🤖 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/committees/components/committee-members-manager/committee-members-manager.component.html` around lines 45 - 54, The pending-invite remove action is wired to the native click event instead of the lfx-button component API. Update the handler in committee-members-manager.component.html to use the button’s public onClick output for the lfx-button used in the removePendingInvite flow, matching the event pattern already used elsewhere and keeping the invite.invitee_email removal logic intact.Source: Learnings
🤖 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/committees/components/committee-members-manager/committee-members-manager.component.html`:
- Around line 45-54: The pending-invite remove action is wired to the native
click event instead of the lfx-button component API. Update the handler in
committee-members-manager.component.html to use the button’s public onClick
output for the lfx-button used in the removePendingInvite flow, matching the
event pattern already used elsewhere and keeping the invite.invitee_email
removal logic intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f71ee6ef-6454-4c46-8b0a-0592b49a2681
📒 Files selected for processing (6)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.tsapps/lfx-one/src/app/modules/committees/components/add-member-dialog/add-member-dialog.component.htmlapps/lfx-one/src/app/modules/committees/components/add-member-dialog/add-member-dialog.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tspackages/shared/src/interfaces/member.interface.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Address review comments from copilot-pull-request-reviewer, cursor: - committee-members-manager.component.ts: filter fetched committee invites to pending status before feeding the invite-by-email dedupe, so declined or revoked invitees can still be re-invited (matches the convention in committee-view.component.ts:781) (per copilot-pull-request-reviewer) - add-member-dialog.component.ts: set the submitting guard at the top of onSubmit again, before the async organization resolution, to prevent a double-click during that window from firing duplicate invite calls (per cursor) Resolves 4 review threads. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Review Feedback AddressedCommit: 026080d Changes Made
Threads Resolved4 of 4 unresolved threads addressed (3 were the same pending-status finding from copilot-pull-request-reviewer; 1 double-submit finding from cursor). NoteThe earlier design change requested by @jordane (invites must not send immediately from the wizard) was addressed in d04a122 — invites are now staged and only sent on wizard completion. Re-requested his review for confirmation. |
| this.pendingInvites.update((current) => [...current, ...additions]); | ||
| this.emitMemberUpdates(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Staged invite survives member add
Medium Severity
Staging an email invite then adding the same address via Add with details leaves that address in pendingInvites and toInvite. Finishing the wizard runs both member creation and createCommitteeInvite for one person.
Reviewed by Cursor Bugbot for commit 026080d. Configure here.
| this.form.patchValue({ organization_id: result.id || null, organization: result.name }); | ||
| } | ||
| fanOut(buildCommitteeOrganizationPayload(this.organizationFormValue())); | ||
| complete(buildCommitteeOrganizationPayload(this.organizationFormValue())); |
There was a problem hiding this comment.
Cancel blocked during org resolve
Medium Severity
onSubmit sets submitting before async organization resolution, and Cancel is disabled while submitting is true. During a slow or hung resolveCurrentEntry call, the invite dialog cannot be dismissed except by waiting for resolution to finish.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 026080d. Configure here.
| // Send staged bulk email invites — deferred here so they only fire on wizard completion (LFXV2-2606) | ||
| if (memberUpdates.toInvite.length > 0) { | ||
| for (const invite of memberUpdates.toInvite) { | ||
| operations.push(this.createMemberOperation('invite', () => this.committeeService.createCommitteeInvite(committeeId, invite))); | ||
| } | ||
| } |
MRashad26
left a comment
MRashad26
left a comment
There was a problem hiding this comment.
PR #1078 — feat(committees): add bulk email invite to create-group wizard
POST /invites)" and the "Fix included" block describes a fetch-based dedupe approach that wraps an immediate-send flow. The actual implementation is the opposite: collectOnly mode stages payloads in pendingInvites, they are flushed via createCommitteeInvite only when the wizard completes (matching the Cursor BugBot summary, not the hand-written one). Merging with an inaccurate description creates a misleading git trail and could cause future reviewers to revert the deferred-send architecture.
Overview (actual behavior): Adds "Invite by Email" to Step 4 of the create-group wizard. AddMemberDialogComponent gains a collectOnly flag that, when true, builds CreateCommitteeInviteRequest[] payloads and returns them via dialogRef.close(staged) instead of posting immediately. The wizard stages them in pendingInvites, surfaces a "Pending invitations" list with per-invite remove, dedupes against both server-side pending invites and already-staged ones, and flushes via createCommitteeInvite on Done. Canceling the wizard sends nothing (LFXV2-2606 requirement met ✅).
Secrets / sensitive data: None found.
Angular 20 audit:
- ✅ No signal-reading method calls in templates —
pendingInvites()is a signal read;committeeLabel.toLowerCase()is a built-in string method on a property, not a component method call;(click)="openInviteByEmailDialog()"and(click)="removePendingInvite(..."are event bindings ✅ - ✅
collectOnlyin[label]="collectOnly ? ... : ..."is a plainbooleanproperty access — not a method call ✅ - ✅
@for (invite of pendingInvites(); track invite.invitee_email)— signal read in@for, property access intrack✅ - ✅ No
effect()introduced - ✅
destroyRef = inject(DestroyRef)(line 60, pre-existing) used correctly withtakeUntilDestroyed✅ - ✅
take(1)onexisting$anddialogRef.onClose.pipe(take(1))✅
Interface / constants placement:
- ✅
MemberPendingChanges.toInviteadded topackages/shared/src/interfaces/member.interface.ts - ✅
CreateCommitteeInviteRequestcross-imported fromcommittee.interfacewithin the same shared package ✅
Logic correctness:
- ✅
stageInvitesdedupes by normalized email before appending — correctly prevents double-staging - ✅
openCollectInviteDialogpasses[...serverInvites, ...stagedAsInvites]with the staged list asPick<CommitteeInvite, 'invitee_email'>[]— the comment correctly notes the dialog only readsinvitee_emailfor deduplication ✅ - ✅ Status filter:
invite.status.toLowerCase() === 'pending'— only pending server invites block re-invite; accepted/declined/revoked are correctly passable ✅ - ✅
committee-manage.ts:toInviteflushed last in the operations loop, after add/update/delete — correct ordering ✅
Process:
- 🟡 Validation section omits
yarn check-types; no## Test planheading - 🟡 PR description inaccuracy (detailed above)
Verdict: ❌ BLOCKED — 1 race condition, 2 process warnings (see inline comment)
| ) | ||
| : of([] as CommitteeInvite[]); | ||
|
|
||
| existing$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((serverInvites) => { |
There was a problem hiding this comment.
🟡 Warning — race condition: multiple rapid clicks open multiple dialogs
openInviteByEmailDialog() fires a network request (getCommitteeInvites) and opens the dialog inside the subscription callback. If the user clicks "Invite by Email" again before the first fetch resolves (~100–300ms), a second subscription starts and a second dialog opens when that fetch completes — both dialogs are live simultaneously.
Suggested fix — guard with a loading flag:
// In the class:
private inviteDialogLoading = false;
public openInviteByEmailDialog(): void {
if (this.inviteDialogLoading) return; // ← guard
this.inviteDialogLoading = true;
const committeeId = this.committeeId();
const existing$ = committeeId
? this.committeeService.getCommitteeInvites(committeeId).pipe(
take(1),
catchError((error) => {
console.error('Failed to load existing invites for dedupe:', error);
return of([] as CommitteeInvite[]);
})
)
: of([] as CommitteeInvite[]);
existing$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((serverInvites) => {
this.inviteDialogLoading = false; // ← reset before opening
const pending = serverInvites.filter((i) => (i.status ?? '').toLowerCase() === 'pending');
this.openCollectInviteDialog(pending);
});
}Alternatively, use a switchMap so a new click cancels the in-flight fetch before opening a fresh dialog.
openInviteByEmailDialog fed getCommitteeInvites() (all statuses) into the dialog's already-invited dedupe unfiltered, so a declined or revoked invitee was treated as already-invited and could not be re-invited. Filter to pending at the fetch sink, matching committee-view.component.ts. Addresses PR #1078 review feedback (copilot-pull-request-reviewer). LFXV2-2606 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
PR SummaryMedium Risk Overview
Minor UX on step 4: Add with details vs invite-by-email, member search/filters shown only when there is at least one member, and submit is locked during async org resolution to prevent double-send in immediate mode. Reviewed by Cursor Bugbot for commit 62b4568. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 62b4568. Configure here.
| // Accepted invitees are already members; declined/revoked ones must be re-invitable. | ||
| const pending = serverInvites.filter((invite) => (invite.status ?? '').toLowerCase() === 'pending'); | ||
| this.openCollectInviteDialog(pending); | ||
| }); |
There was a problem hiding this comment.
Rapid clicks open multiple dialogs
Medium Severity
openInviteByEmailDialog() starts getCommitteeInvites and only opens AddMemberDialogComponent inside the subscription. There is no in-flight guard or disabled state on the button, so repeated clicks before the first request finishes start parallel fetches and each completion opens another modal dialog.
Reviewed by Cursor Bugbot for commit 62b4568. Configure here.
Review Feedback AddressedCommit: 62b4568 Changes Made
Threads ResolvedBoth Copilot pending-status threads resolved. Separately Addressed
Not Changed (open nits)
|
| // Bulk email invites staged in the wizard, deduped by normalized email. These are collected | ||
| // client-side and flushed by the wizard on completion (POST /invites) — never sent immediately, | ||
| // so cancelling the wizard sends nothing (LFXV2-2606). Surfaced as a "Pending invitations" list. | ||
| public readonly pendingInvites = signal<CreateCommitteeInviteRequest[]>([]); |
|
|
||
| <div class="flex items-center mb-4"> | ||
| <div class="flex items-center gap-3"> | ||
| <lfx-button size="small" label="Invite by email" icon="fa-light fa-envelope" data-testid="invite-by-email-button" (click)="openInviteByEmailDialog()"> |
| changes: this.stripMetadata(m), // Pass entire member object, not just changed fields | ||
| })), | ||
| toDelete: members.filter((m) => m.state === 'deleted').map((m) => m.uid), | ||
| toInvite: this.pendingInvites(), |
| if (memberUpdates.toInvite.length > 0) { | ||
| for (const invite of memberUpdates.toInvite) { | ||
| operations.push(this.createMemberOperation('invite', () => this.committeeService.createCommitteeInvite(committeeId, invite))); | ||
| } |
| if (memberUpdates.toInvite.length > 0) { | ||
| for (const invite of memberUpdates.toInvite) { | ||
| operations.push(this.createMemberOperation('invite', () => this.committeeService.createCommitteeInvite(committeeId, invite))); | ||
| } |
What
Adds a bulk email invite action to Step 4 (members) of the create-group wizard: an "Invite by Email" entry point that opens
AddMemberDialogComponentto invite one or more people by email. Invites are sent immediately as pending committee invites (POST /invites).Fix included (review finding)
openInviteByEmailDialog()originally passedexistingInvites: []hard-coded, making the dialog's already-invited dedupe inert. It now:getCommitteeInvites(committeeId)before opening and passes them asexistingInvites, so the dialog dedupes against people already invited.Known limitations / follow-ups
Validation
yarn formatclean,yarn lint0 errors,yarn buildpasses. (apps/lfx-onehas no unit-test runner, so no unit test added — consistent with the repo.)🤖 Generated with Claude Code
Note
Medium Risk
Changes when committee invites are sent and batches them with other member operations on wizard completion; behavior is still the existing invite API with client-side staging and dedupe.
Overview
Adds bulk invite-by-email to the create-group members step with deferred sending: invites are staged client-side and only posted when the wizard finishes, so canceling the wizard sends nothing (LFXV2-2606).
AddMemberDialogComponentgains acollectOnlymode that validates emails and returnsCreateCommitteeInviteRequest[]instead of callingPOST /invitesimmediately; the submit action becomes “Add to invitations” in that mode. The members step shows a Pending invitations list (with remove), dedupes against members, pending server invites (edit), and already-staged emails, and wires staged invites throughMemberPendingChanges.toInvite.committee-managetreatstoInvitelike other pending member ops and flushes them on Done viacreateCommitteeInvite.Minor UX: member search/filters render only when there is at least one member; the manual add action is labeled “Add with details.”
Reviewed by Cursor Bugbot for commit 026080d. Bugbot is set up for automated code reviews on this repo. Configure here.