feat(committees): refactor to multi-step wizard workflow#197
Conversation
- Convert committee-manage to PrimeNG Stepper with 4 steps: 1. Category Selection 2. Basic Info 3. Settings 4. Members - Create new sub-components for each wizard step - Add reusable card-selector shared component - Add committee category configurations with icons, descriptions - Refactor meeting-type-selection to use card-selector - Add CardSelectorOption and MemberPendingChanges interfaces - Remove unused committee-form component - Remove unused interfaces: CommitteeSettings, CommitteeSummary, CommitteeValidationError, CommitteeValidationResult - Remove unused getValidCommitteeCategories function - Fix user service queries to use SUB instead of LF_USERNAME LFXV2-879 LFXV2-880 Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors committee management into a 4-step stepper workflow with dedicated components for category selection, basic info, settings, and member management. Introduces a shared CardSelectorComponent and updates services to use signal-based state. Removes interfaces from committee and form APIs while extending member state tracking. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CommitteeManage as Committee<br/>Manage<br/>(Orchestrator)
participant StepComponent as Step<br/>Component
participant Service as Committee<br/>Service
participant API as Backend<br/>API
User->>CommitteeManage: Open (create/edit)
CommitteeManage->>CommitteeManage: Initialize step 1
loop Step Navigation
User->>StepComponent: Fill form / Select options
StepComponent->>CommitteeManage: Form value change
CommitteeManage->>CommitteeManage: Validate current step
alt Can proceed
User->>CommitteeManage: Next/Previous
CommitteeManage->>CommitteeManage: Navigate step
CommitteeManage->>StepComponent: Load/Reset component
end
end
User->>CommitteeManage: Done (Final step)
CommitteeManage->>CommitteeManage: Collect all form data
alt Create Mode
CommitteeManage->>Service: createCommittee()
Service->>API: POST /committees
API-->>Service: Committee + ID
Service-->>CommitteeManage: Success
else Edit Mode + Member Changes
par Parallel Ops
CommitteeManage->>Service: updateCommittee()
Service->>API: PUT /committees/{id}
CommitteeManage->>Service: addCommitteeMember()
Service->>API: POST /members
CommitteeManage->>Service: updateCommitteeMember()
Service->>API: PUT /members/{id}
CommitteeManage->>Service: deleteCommitteeMember()
Service->>API: DELETE /members/{id}
and
API-->>Service: Member operations results
end
end
CommitteeManage->>CommitteeManage: Show toast results
CommitteeManage->>User: Navigate away
sequenceDiagram
participant UI as UI (Step 4)
participant Manager as Members<br/>Manager<br/>Component
participant Dialog as Member<br/>Form<br/>Dialog
participant Service as Committee<br/>Service
participant Confirm as Confirmation<br/>Dialog
User->>Manager: Click Add/Edit Member
Manager->>Dialog: Open with context
Dialog->>Dialog: Initialize form
User->>Dialog: Fill member details
User->>Dialog: Submit
Dialog-->>Manager: Return member data
Manager->>Manager: Add to membersWithState
Manager->>UI: Emit memberUpdatesChange
User->>Manager: Click Delete Member
Manager->>Confirm: Open confirmation
User->>Confirm: Confirm delete
Confirm-->>Manager: Confirmed
Manager->>Manager: Remove from membersWithState
Manager->>Service: deleteCommitteeMember() <br/>(if existing member)
Service-->>Manager: Delete result
Manager->>UI: Emit memberUpdatesChange
Manager->>UI: Show toast (success/error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/lfx-one/src/server/services/user.service.ts (1)
379-423: Rename parameter and update documentation to align with SUB column usage.The function now queries the
SUBcolumn instead ofLF_USERNAME, but the parameter remains namedlfUsernameand JSDoc still refers to "Linux Foundation username". Update both to reflect that the function operates on OIDC subject identifiers:
- Rename parameter from
lfUsernametouserSubthroughout the function- Update JSDoc: replace "Linux Foundation username from OIDC" with "User subject identifier (SUB) from OIDC"
- This affects the parameter declaration at line 379 and its usage at line 423
Verify that all call sites of
getMyProjects()pass the user's SUB claim rather than their LF username, as this is a breaking semantic change.apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.ts (1)
37-66: Signal read inside RxJS pipe may not react to changes.Reading
this.committeeId()inside themapoperator (line 48) captures the signal value at subscription time. IfcommitteeIdchanges later, the filter won't update becausetoSignalsubscribes once.If
committeeIdcan change during the component's lifecycle, consider restructuring:private initializeParentCommitteeOptions(): Signal<{ label: string; value: string | null }[]> { const uid = this.projectContextService.getProjectUid() || this.projectContextService.getFoundationId(); - const committees = toSignal( - this.committeeService.getCommitteesByProject(uid).pipe( - map((committees: Committee[]) => { - const topLevelCommittees = committees.filter((committee) => !committee.parent_uid); - const currentCommitteeId = this.committeeId(); - const availableCommittees = currentCommitteeId - ? topLevelCommittees.filter((committee) => committee.uid !== currentCommitteeId) - : topLevelCommittees; - return availableCommittees; - }) - ), - { initialValue: [] } - ); + const committees = toSignal( + this.committeeService.getCommitteesByProject(uid), + { initialValue: [] } + ); return computed(() => { - const options = committees().map((committee) => ({ + const allCommittees = committees(); + const topLevelCommittees = allCommittees.filter((committee) => !committee.parent_uid); + const currentCommitteeId = this.committeeId(); + const availableCommittees = currentCommitteeId + ? topLevelCommittees.filter((committee) => committee.uid !== currentCommitteeId) + : topLevelCommittees; + + const options = availableCommittees.map((committee) => ({ label: committee.display_name || committee.name, value: committee.uid, })); return [{ label: 'No Parent ' + this.committeeLabel, value: null }, ...options]; }); }This moves all filtering logic into the
computed, ensuring reactivity tocommitteeIdchanges.
🧹 Nitpick comments (12)
apps/lfx-one/src/app/shared/services/account-context.service.ts (1)
30-33: Consider makingselectedAccountreadonly to external consumers.Exposing
selectedAccountas a publicWritableSignalallows external code to call.set()directly, bypassing thesetAccount()method which handles storage persistence. This could lead to state inconsistency where the UI shows one account but storage has another.Consider using a readonly signal pattern:
- public readonly selectedAccount: WritableSignal<Account>; + private readonly _selectedAccount: WritableSignal<Account>; + public readonly selectedAccount: Signal<Account> = computed(() => this._selectedAccount());Then update internal usages to use
_selectedAccount.set(). This ensures all external mutations go throughsetAccount().apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.html (1)
1-117: Inline member form looks solid; consider a couple more test hooksThe dual‑mode UX (search vs manual fields), validation, and
data-testidcoverage on the container, form, and core fields all look good and consistent with the rest of the app.Optionally, you might add explicit
data-testidattributes for the “Enter details manually” and “← Back to search” buttons to make those transitions easier to target in E2E tests.packages/shared/src/constants/committees.constants.ts (1)
5-5: Good centralization of committee UI config; small type‑safety tweaks possibleThe new
CommitteeCategoryInfo/COMMITTEE_CATEGORY_CONFIGS, stepper constants, andCOMMITTEE_SETTINGS_FEATURESgive a clear single source of truth for committee UI metadata—nice improvement.Two non‑blocking suggestions to reduce drift over time:
Tie categories to the config map by type
Right nowCOMMITTEE_CATEGORIESandCOMMITTEE_CATEGORY_CONFIGSboth hard‑code the same string keys. You could introduce atype CommitteeCategoryKey = keyof typeof COMMITTEE_CATEGORY_CONFIGS;and reuse that forvalueinCOMMITTEE_CATEGORIES(and anywhere else), so adding/removing a category in one place is enforced by the compiler.Make form steps more self‑consistent
COMMITTEE_TOTAL_STEPSandCOMMITTEE_FORM_STEPSare manually kept in sync withCOMMITTEE_STEP_TITLES. Consider either:
- Deriving the numeric indices from
COMMITTEE_STEP_TITLES(e.g., mapping by index), or- At least marking
COMMITTEE_FORM_STEPSasas constand reusing those values wherever possible, so a step insert/remove has fewer chances to desync.None of this blocks the PR; it just tightens the linkage between these config structures.
Also applies to: 22-158, 342-424
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.html (1)
1-33: Category selection template is clean; consider a root-level test idThe template is straightforward and uses the shared card selector nicely, with a clear label and error message tied to the
categorycontrol.To make this step easier to target in E2E tests, you might add a
data-testidto the outer<div>(e.g.,data-testid="committee-category-selection"), in addition to thetestIdPrefixonlfx-card-selector.apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.html (1)
22-26: Consider extracting form control to template variable.The expression
form().get(control())is called multiple times (lines 24, 25, 42). For better performance and readability, consider extracting it to a template variable.+@let formControl = form().get(control()); <div class="p-4 border rounded-lg cursor-pointer transition-all hover:border-blue-600 hover:bg-blue-50" [ngClass]="{ - 'border-blue-600 bg-blue-50': form().get(control())?.value === option.value, - 'border-slate-200 bg-white': form().get(control())?.value !== option.value, + 'border-blue-600 bg-blue-50': formControl?.value === option.value, + 'border-slate-200 bg-white': formControl?.value !== option.value, }" (click)="onSelect(option.value)" [attr.data-testid]="testIdPrefix() + '-option-' + option.value"> ... </div> -@if (form().get(control())?.errors?.['required'] && form().get(control())?.touched) { +@if (formControl?.errors?.['required'] && formControl?.touched) { <p class="text-sm text-red-600 mt-2">{{ errorMessage() }}</p> }Also applies to: 42-43
apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.html (1)
25-27: Consider template variables for repeated form.get() calls.Similar to other templates, extracting form controls to template variables improves readability and avoids repeated method calls.
Example for the name field:
@let nameControl = form().get('name'); @if (nameControl?.errors?.['required'] && nameControl?.touched) { <p class="text-sm text-red-600">{{ committeeLabel }} name is required</p> }Also applies to: 77-79
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html (1)
114-157: Complex conditional button logic may benefit from helper methods.The footer navigation contains multiple nested conditionals (lines 114-157) controlling which buttons appear. While functionally correct, this complexity could be simplified by extracting button visibility logic to computed signals in the component.
Consider adding computed signals in the TypeScript component:
public readonly showNextButton = computed(() => !this.isLastFormStep() && !this.isLastStep() ); public readonly showCreateButton = computed(() => this.isLastFormStep() && !this.isEditMode() ); // etc.This would simplify the template to:
@if (showNextButton()) { <lfx-button ... /> }apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts (1)
45-46: Consider cleaning up DynamicDialogRef on component destroy.The
dialogRefshould be closed when the component is destroyed to prevent orphaned dialogs.+import { OnDestroy } from '@angular/core'; -export class CommitteeMembersManagerComponent implements OnInit { +export class CommitteeMembersManagerComponent implements OnInit, OnDestroy { // ... existing code ... + public ngOnDestroy(): void { + this.dialogRef?.close(); + }apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.ts (1)
28-38: Consider usingeffect()instead oftoObservable()for signal-to-signal reactivity.Since you're converting a signal to an observable just to react to changes, Angular 19's
effect()provides a more idiomatic approach:-import { toObservable } from '@angular/core/rxjs-interop'; +import { effect } from '@angular/core'; -import { filter, take } from 'rxjs'; public constructor() { - // If we have an existing member, show individual fields immediately - toObservable(this.member) - .pipe( - filter((member) => member !== null), - take(1) - ) - .subscribe(() => { - this.showIndividualFields.set(true); - }); + // If we have an existing member, show individual fields immediately + effect(() => { + if (this.member() !== null) { + this.showIndividualFields.set(true); + } + }, { allowSignalWrites: true }); }However, both approaches work correctly here.
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (3)
63-68: Redundant initial assignment forcurrentStep.The
currentStepsignal is initialized here but immediately reassigned in the constructor (lines 92-111). Consider declaring without the redundant initializer or moving the initialization logic directly here.// Stepper state private internalStep = signal<number>(1); - public currentStep = toSignal(of(1), { initialValue: 1 }); + public currentStep!: ReturnType<typeof toSignal<number>>; public readonly totalSteps = COMMITTEE_TOTAL_STEPS;
200-210: Consider extracting form data preparation into a shared helper.The form value preparation logic (lines 200-210) is duplicated in
onSubmitAll(lines 294-304). Extracting this into a private method likeprepareCommitteeData()would reduce duplication.private prepareCommitteeData(): Record<string, unknown> { const formValue = { ...this.form().value, calendar: { public: this.form().value.public || false }, display_name: this.form().value.display_name || this.form().value.name, website: this.form().value.website || null, project_uid: this.project()?.uid || null, }; return this.cleanFormData(formValue); }
519-528:window.pageYOffsetis deprecated; preferwindow.scrollY.While functionally equivalent,
pageYOffsetis deprecated. Additionally, consider using@ViewChildfor a more Angular-idiomatic approach to DOM access.private scrollToStepper(): void { const committeeManage = document.getElementById('committee-manage'); if (committeeManage) { - const elementTop = committeeManage.getBoundingClientRect().top + window.pageYOffset; + const elementTop = committeeManage.getBoundingClientRect().top + window.scrollY; window.scrollTo({ top: elementTop - 100, behavior: 'smooth', }); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (30)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html(2 hunks)apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts(11 hunks)apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.ts(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.ts(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.html(0 hunks)apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.scss(0 hunks)apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.ts(1 hunks)apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html(2 hunks)apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts(6 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts(3 hunks)apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.html(1 hunks)apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.ts(1 hunks)apps/lfx-one/src/app/shared/services/account-context.service.ts(3 hunks)apps/lfx-one/src/app/shared/services/committee.service.ts(1 hunks)apps/lfx-one/src/server/services/user.service.ts(2 hunks)packages/shared/src/constants/committees.constants.ts(3 hunks)packages/shared/src/interfaces/committee.interface.ts(0 hunks)packages/shared/src/interfaces/components.interface.ts(1 hunks)packages/shared/src/interfaces/member.interface.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/shared/src/interfaces/committee.interface.ts
- apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.html
- apps/lfx-one/src/app/modules/committees/components/committee-form/committee-form.component.scss
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/shared/services/committee.service.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.tspackages/shared/src/interfaces/member.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.tsapps/lfx-one/src/app/shared/services/account-context.service.tspackages/shared/src/constants/committees.constants.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.tsapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.tsapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/shared/services/committee.service.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.htmlapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.tspackages/shared/src/interfaces/member.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.tsapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.htmlapps/lfx-one/src/app/shared/services/account-context.service.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.htmlpackages/shared/src/constants/committees.constants.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.htmlapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.htmlapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.htmlapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/shared/services/committee.service.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.tspackages/shared/src/interfaces/member.interface.tsapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.tsapps/lfx-one/src/app/shared/services/account-context.service.tspackages/shared/src/constants/committees.constants.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.tsapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.tsapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.tsapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.tsapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.tsapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.tsapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.tsapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-one/shared package
Files:
packages/shared/src/interfaces/components.interface.tspackages/shared/src/interfaces/member.interface.tspackages/shared/src/constants/committees.constants.ts
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.htmlapps/lfx-one/src/app/shared/components/card-selector/card-selector.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.htmlapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.htmlapps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html
apps/**/**/server/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/**/server/**/*.{ts,js}: Use Pino for structured JSON logs with sensitive data redaction in all backend services
Always use err field for errors to leverage Pino's error serializer - correct: req.log.error({ err: error, ...metadata }, 'message')
Log INFO level for business operation completions (created, updated, deleted) and successful data retrieval
Log WARN level for error conditions leading to exceptions and data quality issues
Log DEBUG level for internal operations, preparation steps, and intent statements
Log ERROR level for system failures, unhandled exceptions, and critical errors
Files:
apps/lfx-one/src/server/services/user.service.ts
🧬 Code graph analysis (12)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/constants/accounts.constants.ts (1)
ACCOUNTS(10-31)
apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts (1)
packages/shared/src/constants/committees.constants.ts (2)
COMMITTEE_SETTINGS_FEATURES(379-424)COMMITTEE_LABEL(17-20)
apps/lfx-one/src/app/shared/services/committee.service.ts (1)
packages/shared/src/interfaces/member.interface.ts (2)
CreateCommitteeMemberRequest(70-114)CommitteeMember(10-64)
apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.ts (2)
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.ts (1)
Component(13-38)packages/shared/src/interfaces/components.interface.ts (1)
CardSelectorOption(25-32)
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.ts (3)
apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.ts (1)
Component(9-35)packages/shared/src/constants/committees.constants.ts (4)
COMMITTEE_LABEL(17-20)FILTERED_COMMITTEE_CATEGORIES(192-199)COMMITTEE_CATEGORIES(165-186)COMMITTEE_CATEGORY_CONFIGS(37-158)packages/shared/src/interfaces/components.interface.ts (1)
CardSelectorOption(25-32)
apps/lfx-one/src/app/shared/services/account-context.service.ts (2)
packages/shared/src/interfaces/account.interface.ts (1)
Account(8-13)packages/shared/src/constants/accounts.constants.ts (1)
ACCOUNTS(10-31)
packages/shared/src/constants/committees.constants.ts (1)
packages/shared/src/constants/colors.constants.ts (1)
lfxColors(9-106)
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts (3)
packages/shared/src/interfaces/member.interface.ts (1)
CommitteeMember(10-64)packages/shared/src/interfaces/committee.interface.ts (1)
Committee(8-56)packages/shared/src/constants/committees.constants.ts (2)
MEMBER_ROLES(209-221)VOTING_STATUSES(231-237)
apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.ts (1)
packages/shared/src/interfaces/member.interface.ts (1)
CommitteeMember(10-64)
apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.ts (3)
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.ts (1)
Component(13-38)apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts (1)
Component(11-24)packages/shared/src/constants/committees.constants.ts (1)
COMMITTEE_LABEL(17-20)
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
packages/shared/src/interfaces/components.interface.ts (2)
CardSelectorOption(25-32)CardSelectorOptionInfo(10-19)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (3)
packages/shared/src/interfaces/member.interface.ts (1)
MemberPendingChanges(139-146)packages/shared/src/constants/committees.constants.ts (3)
COMMITTEE_TOTAL_STEPS(357-357)COMMITTEE_STEP_TITLES(350-350)COMMITTEE_LABEL(17-20)packages/shared/src/interfaces/committee.interface.ts (1)
Committee(8-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (28)
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html (1)
30-30: Spacing refinement looks good.The Tailwind class adjustments (
gap-1,py-1,-mt-1) effectively tighten vertical spacing in the scrollable meetings list, aligning with the PR's UI refinement goals. The data-testid attribute is properly preserved and follows the hierarchical naming convention.apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
39-39: Inconsistent data source for account selection.The component uses the global
ACCOUNTSarray directly (lines 39 and 55) whileAccountContextServicehas a computedavailableAccountssignal that may return user-specific organizations. This bypasses the service's logic for filtering accounts based on user memberships.Consider using the service's
availableAccountsfor consistency:- public readonly availableAccounts = ACCOUNTS; + public readonly availableAccounts = this.accountContextService.availableAccounts;- const selectedAccount = ACCOUNTS.find((acc) => acc.accountId === value); + const selectedAccount = this.accountContextService.availableAccounts().find((acc) => acc.accountId === value);If the intent is to always show all accounts in this dashboard regardless of user membership, please add a comment explaining this design decision.
Also applies to: 55-57
apps/lfx-one/src/app/shared/services/account-context.service.ts (3)
40-46: LGTM!The computed signal correctly implements the fallback logic: returns user-specific organizations when initialized and available, otherwise falls back to the global
ACCOUNTSarray. This is a clean reactive pattern.
48-51: LGTM!Clean constructor initialization with proper fallback to
DEFAULT_ACCOUNTwhen no stored value exists.
57-73: LGTM!The initialization logic properly handles the edge cases:
- Validates stored account against available organizations
- Migrates to first organization if stored account is invalid
- Empty organizations array correctly results in fallback to global ACCOUNTS (since
initializedstays false)apps/lfx-one/src/app/shared/services/committee.service.ts (1)
84-86: Partial member updates via PUT—confirm backend contractChanging
memberDatatoPartial<CreateCommitteeMemberRequest>is a nice fit for incremental inline edits, but it also means callers can now send sparse (or even empty) payloads on aPUT. That’s fine if the/members/{memberId}endpoint treatsPUTas a partial update, but it will break APIs that expect the full resource.I’d suggest double‑checking the backend contract and updating any API docs accordingly so expectations are clear.
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.html (1)
64-64: Template now treatscommittee/isEditingas properties—ensure TS matchesThe template now uses
committee?.enable_votingandisEditing(no parentheses). That’s correct only if the component switched these from a signal/method to a property/getter. Ifcommitteeis still a signal (committee()) orisEditingis still a method, Angular will either throw or never update the view as expected.Please confirm the component class exposes
committeeandisEditingas properties (or getters) to match this new usage.Also applies to: 135-135
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html (1)
24-28: Making the committee name clickable is a good UX improvementWrapping the committee name in a
routerLinkto/groups/{{committee.uid}}aligns with how the members count already navigates and makes the table more discoverable. Styling is preserved, so there’s no visual regression.Looks good to me.
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html (1)
24-31: Good reuse of the shared card selector for meeting typeSwitching to
<lfx-card-selector>here removes a lot of bespoke markup and keeps the meeting flow consistent with the new committee category selection UX. The bindings (meetingTypeOptions(),form(),control="meeting_type",testIdPrefix) all look coherent.Just make sure the backing form group defines a
meeting_typecontrol and thatmeetingTypeOptions()returns the newCardSelectorOptionshape so everything stays type‑safe.packages/shared/src/interfaces/components.interface.ts (1)
6-32: Card selector interfaces look well‑structured and reusable
CardSelectorOptionInfoand the genericCardSelectorOption<T = string>give the card selector a clear, strongly‑typed contract (label/value plus display metadata), which should keep committee and meeting type options consistent across the app.No issues from my side with this shape; it’s a good foundation for the shared
lfx-card-selectorcomponent.packages/shared/src/interfaces/member.interface.ts (1)
116-146: LGTM! Well-designed state tracking interfaces.The new interfaces provide a clean pattern for tracking member lifecycle states and pending changes:
CommitteeMemberStateclearly defines the four possible statesCommitteeMemberWithStateappropriately extends the base interface with state metadataMemberPendingChangesprovides a structured way to batch operationsThe documentation is clear and the design supports both optimistic UI updates and batch API operations.
apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.ts (1)
1-24: LGTM! Clean component implementation.The component follows Angular 19 best practices:
- Uses standalone component pattern
- Leverages signal-based inputs with
input.required()- Properly imports dependencies
- Exposes shared constants for template use
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html (1)
65-75: Good defensive rendering for member management step.The conditional rendering ensures members can only be managed after the committee is created. The placeholder message clearly communicates this requirement to users.
apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.ts (1)
1-35: LGTM! Well-designed generic component.The
CardSelectorComponentdemonstrates excellent practices:
- Generic type parameter provides type safety for different value types
- Signal-based inputs follow Angular 19 patterns
- Proper form integration with
setValue()andmarkAsTouched()- Event emission for parent components to react to selection changes
- Appropriate default values for optional inputs
This component is reusable and type-safe.
apps/lfx-one/src/app/shared/components/card-selector/card-selector.component.html (1)
30-31: The color concatenation pattern is working correctly with the current codebase.All
CardSelectorOptioncolors are provided vialfxColors, which returns hex format strings (e.g.,#009AFF). The concatenationoption.info.color + '1A'successfully appends the opacity suffix to create a valid CSS color value. There are no instances ofrgb(),hsl(), or named colors being passed to this component. Thedata-testidattribute is also already present on line 28.apps/lfx-one/src/app/modules/committees/components/committee-settings/committee-settings.component.html (1)
30-32: Color concatenation is safe and working as designed.The colors from
lfxColorsare guaranteed to be hex format strings (e.g.,#009AFF,#00BC7D). The patternfeature.color + '15'produces valid 8-digit hex colors with alpha transparency (e.g.,#009AFF15= hex with ~8% opacity). No changes needed.Additionally,
data-testidattributes are already present and properly named following the hierarchical convention:feature-{key}andtoggle-{key}.Likely an incorrect or invalid review comment.
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/committee-category-selection.component.ts (2)
13-18: LGTM - Well-structured standalone component.The component correctly uses Angular 19 patterns with
inject(),input.required(), andcomputed()for reactive state management. The persona-based category filtering is cleanly implemented.
29-37: No issue with category config lookup.All category values from both
COMMITTEE_CATEGORIESandFILTERED_COMMITTEE_CATEGORIEShave corresponding entries inCOMMITTEE_CATEGORY_CONFIGS. Theinfoproperty will never beundefinedsince the code only maps over these predefined arrays.apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts (3)
122-124: Non-null assertion onthis.member!.uidmay cause runtime error.When
isEditingis true butmemberis undefined (edge case), accessingthis.member!.uidwill throw. Consider adding a guard:- const operation = this.isEditing - ? this.committeeService.updateCommitteeMember(committeeId, this.member!.uid as string, memberData) + const operation = this.isEditing && this.member?.uid + ? this.committeeService.updateCommitteeMember(committeeId, this.member.uid, memberData) : this.committeeService.createCommitteeMember(committeeId, memberData);Note: This comment refers to member-form.component.ts lines 122-124. Disregard if not applicable to this file.
69-77: LGTM - Clean computed signals for derived state.The
visibleMembers,memberCount, andvotingCountcomputed signals are well-structured and follow Angular 19 best practices for reactive derived state.
179-207: LGTM - Delete handling with proper state management.The delete logic correctly distinguishes between new members (removed completely) and existing members (marked as deleted), maintaining data integrity for the pending changes workflow.
apps/lfx-one/src/app/modules/committees/components/inline-member-form/inline-member-form.component.ts (1)
14-19: LGTM - Clean standalone component setup.The component correctly uses Angular 19 standalone patterns with appropriate imports and signal-based state management.
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts (2)
103-108: LGTM - Clean wizard mode implementation.The wizard mode correctly short-circuits API calls and returns the prepared data through the dialog, enabling the parent component to batch changes before committing.
51-61: LGTM - Config-based initialization is appropriate for dialog components.Moving from computed signals to static readonly properties is correct for dialog components where the config data doesn't change after initialization.
apps/lfx-one/src/app/modules/committees/components/committee-basic-info/committee-basic-info.component.ts (1)
17-23: LGTM - Clean component restructuring.The component has been properly simplified to focus on basic info, with appropriate use of Angular 19 standalone patterns and signal-based inputs.
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
1-94: Clean integration with the new CardSelectorComponent.The refactoring properly uses the shared
CardSelectorComponentwith correct type annotations. The computed signal pattern formeetingTypeOptionsefficiently handles the role-based filtering, and themeetingTypeInfomapping is complete with allMeetingTypevalues includingNONE.apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (2)
313-349: forkJoin will fail if the committee update fails, but member operations may have started.With
forkJoin, ifupdateCommittee$fails, the entire subscription errors out. However, sincemembers$usesconcatwith individual error catching, some member operations may complete before the committee update response. Consider whether this partial state is acceptable or if committee update should complete first.If the committee update must succeed before member changes, consider sequential execution:
updateCommittee$.pipe( switchMap(() => members$), // ... )
42-89: Well-structured multi-step wizard implementation using Angular 19 signals.The component properly leverages Angular 19's signal-based state management with computed signals for derived state (
canProceed,canGoNext,hasMemberUpdates, etc.). The separation of concerns between stepper navigation, form validation, and member operations is clean.
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
...c/app/modules/committees/components/committee-basic-info/committee-basic-info.component.html
Show resolved
Hide resolved
...les/committees/components/committee-members-manager/committee-members-manager.component.html
Show resolved
Hide resolved
...les/committees/components/committee-members-manager/committee-members-manager.component.html
Show resolved
Hide resolved
...dules/committees/components/committee-members-manager/committee-members-manager.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR successfully refactors the committee management interface from a single-form approach to a multi-step wizard workflow using PrimeNG Stepper, improving the user experience by breaking complex form entry into logical, manageable steps. The implementation follows the established patterns from the meetings module and introduces a reusable card-selector component that can be shared across different features.
Key Changes
- Multi-Step Wizard Implementation: Introduces a 4-step committee creation/editing workflow (Category Selection → Basic Info → Settings → Add Members) with step validation and flexible navigation
- Reusable Card Selector Component: Creates a new shared component for card-based selection patterns, used by both committees and meetings modules
- Enhanced Member Management: Implements state tracking for member changes (new/modified/deleted) with wizard mode support for creating committees before members exist
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/member.interface.ts |
Adds member state tracking interfaces for managing pending changes during wizard workflow |
packages/shared/src/interfaces/components.interface.ts |
Defines card selector option interfaces for reusable selection components |
packages/shared/src/interfaces/committee.interface.ts |
Removes unused interfaces (CommitteeSettings, CommitteeSummary, validation interfaces) as part of cleanup |
packages/shared/src/constants/committees.constants.ts |
Adds comprehensive category configurations with icons, descriptions, colors and step constants for wizard |
apps/lfx-one/src/server/services/user.service.ts |
Fixes user service queries to use SUB instead of LF_USERNAME for consistency |
apps/lfx-one/src/app/shared/services/committee.service.ts |
Updates member update method signature to accept partial payloads |
apps/lfx-one/src/app/shared/services/account-context.service.ts |
Refactors to use computed signals for availableAccounts, improving reactivity |
apps/lfx-one/src/app/shared/components/card-selector/* |
New reusable component for card-based selection UI with customizable grid layouts |
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/* |
Refactored to use new card-selector component, removing duplicate code |
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/* |
Minor styling adjustment to meeting list container |
apps/lfx-one/src/app/modules/dashboards/board-member/* |
Updates account selection to use ACCOUNTS directly |
apps/lfx-one/src/app/modules/committees/components/member-form/* |
Adds wizard mode support to return data without API calls when committee doesn't exist yet |
apps/lfx-one/src/app/modules/committees/components/inline-member-form/* |
New component for simplified inline member entry with search and manual modes |
apps/lfx-one/src/app/modules/committees/components/committee-table/* |
Converts committee names to clickable links for better navigation |
apps/lfx-one/src/app/modules/committees/components/committee-settings/* |
New step component for configuring committee features with visual cards |
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/* |
New component for managing members with state tracking and pending changes |
apps/lfx-one/src/app/modules/committees/components/committee-category-selection/* |
New first step for category selection using card-selector component |
apps/lfx-one/src/app/modules/committees/components/committee-basic-info/* |
Refactored from committee-form to focus only on basic info step |
apps/lfx-one/src/app/modules/committees/components/committee-form/* |
Removed - replaced by step-based components |
apps/lfx-one/src/app/modules/committees/committee-manage/* |
Major refactor to implement stepper workflow with navigation, validation, and member operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
...e/src/app/modules/committees/components/committee-settings/committee-settings.component.html
Show resolved
Hide resolved
...dules/committees/components/committee-members-manager/committee-members-manager.component.ts
Show resolved
Hide resolved
...dules/committees/components/committee-members-manager/committee-members-manager.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
Outdated
Show resolved
Hide resolved
- Remove unused inline-member-form component - Convert form signal to regular class property (no state changes) - Extract isVotingMember() helper to reduce code duplication - Add boundary checks to getStepTitle() method - Add takeUntilDestroyed to form subscriptions in members manager LFXV2-879 Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (1)
225-261: Navigating away on member-op error will discard pending changesIn
onDone(), the error callback logs and shows a toast, but still callsthis.router.navigate(['/groups']);. This means that if saving member changes fails in create mode, the user is taken back to the list and loses the ability to inspect or retry the failed operations without reconstructing them, which is a poor UX for transient API issues.A better pattern here is to stay on the members step, keep
memberUpdatesas-is, and just clearsubmittingso the user can correct or retry..subscribe({ next: (results) => { const totalSuccess = results.reduce((sum, result) => sum + result.success, 0); const totalFailed = results.reduce((sum, result) => sum + result.failed, 0); this.showMemberOperationToast(totalSuccess, totalFailed, totalSuccess + totalFailed); this.router.navigate(['/groups']); }, error: (error) => { console.error('Error processing member changes:', error); this.messageService.add({ severity: 'error', summary: 'Error', detail: 'Failed to save member changes', }); - this.router.navigate(['/groups']); + // Stay on the page so the user can retry or adjust their changes + // MemberUpdates state is preserved; just re-enable the UI }, });
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (1)
540-567: Member operation batching is straightforward and type-safe enough
buildMemberOperations()builds a flat list of Observables for delete, update, and add operations based onmemberUpdates, which is then consumable by bothonDone()(sequential concat) andonSubmitAll()(viaconcat(...).pipe(toArray())). Given that both callers only execute this when acommitteeIdis guaranteed (create post-success or edit mode), the non-null assertion onthis.committeeId()!is acceptable, though adding a guard would make this invariant explicit if you want extra defensiveness.- private buildMemberOperations() { - const operations: ReturnType<typeof this.createMemberOperation>[] = []; - const committeeId = this.committeeId()!; + private buildMemberOperations() { + const committeeId = this.committeeId(); + if (!committeeId) { + // No operations possible without an ID + return [] as ReturnType<typeof this.createMemberOperation>[]; + } + const operations: ReturnType<typeof this.createMemberOperation>[] = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html(2 hunks)apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts(9 hunks)apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts(1 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts (3)
packages/shared/src/interfaces/member.interface.ts (4)
CommitteeMemberWithState(126-133)CreateCommitteeMemberRequest(70-114)CommitteeMember(10-64)CommitteeMemberState(120-120)packages/shared/src/constants/committees.constants.ts (1)
COMMITTEE_LABEL(17-20)packages/shared/src/utils/form.utils.ts (1)
generateTempId(9-11)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (2)
packages/shared/src/interfaces/member.interface.ts (1)
MemberPendingChanges(139-146)packages/shared/src/constants/committees.constants.ts (4)
COMMITTEE_TOTAL_STEPS(357-357)COMMITTEE_STEP_TITLES(350-350)COMMITTEE_FORM_STEPS(364-373)COMMITTEE_LABEL(17-20)
🔇 Additional comments (14)
apps/lfx-one/src/app/modules/meetings/components/meeting-registrants-manager/meeting-registrants-manager.component.ts (1)
93-107: Form subscriptions now correctly teardown with component lifecycleWiring the
searchandstatusvalueChangesthroughtakeUntilDestroyed()fixes the prior leak risk from constructor‑time subscriptions and keeps the filtering signals in sync without manual cleanup. This looks good and aligns with the rest of the component’s lifecycle handling.apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html (1)
13-159: Stepper layout and navigation wiring are consistent and testableThe 4-step
p-stepperintegration, conditional rendering of the members step, and the footer button matrix (Previous / Cancel-or-Skip / Next / Create / Done / Update) are wired coherently with the TS signals (currentStep,isEditMode,isLastStep,isLastFormStep,canProceed). Data-testids are applied in a consistentcommittee-manage-*pattern, which should make the wizard easy to exercise in E2E tests.apps/lfx-one/src/app/modules/committees/components/committee-members-manager/committee-members-manager.component.ts (4)
87-107: Constructor subscriptions now lifecycle-safeHooking
searchandstatusvalueChangesthroughtakeUntilDestroyed()in the constructor ensures the filters won’t leak subscriptions when this manager is destroyed and recreated. This directly addresses the earlier concern about unbounded subscriptions while keeping the signal-based filtering responsive.
121-160: Dialog result handling for add/edit members is robustBoth
openAddMemberDialogandopenEditMemberDialogscope theironClosesubscriptions withtake(1), then delegate to dedicated handlers. This avoids leaks from long-lived dialog refs and keeps the add/edit flows cleanly separated, which should make future adjustments to payload mapping or validation straightforward.
216-306: Member mapping and pending-changes emission are consistent with interfacesThe conversion from
CreateCommitteeMemberRequesttoCommitteeMemberinhandleEditMemberResult/handleAddMemberResult, plus the tempId/state handling, lines up withCommitteeMemberWithStateandMemberPendingChanges. UsingemitMemberUpdates()after local state updates ensures the parent always sees a coherent toAdd/toUpdate/toDelete view without extra diffing.
308-347: Search/status filtering and voting helper are clear and extensible
initFilteredMembers()cleanly composes text search with status filtering, and extractingisVotingMember()centralizes the voting-role logic so status checks aren’t duplicated across the component. This is a good balance between readability and flexibility if more filters are added later.apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.ts (8)
63-82: Stepper state and validation signals are wired coherently
internalStep+currentStep(query-param aware), together withCOMMITTEE_TOTAL_STEPS/COMMITTEE_FORM_STEPS, give a clear model for navigation. The derived flags (canProceed,canGoNext,isFirstStep,isLastFormStep,isLastStep,currentStepTitle,hasMemberUpdates) are well factored for the template and accurately reflect the wizard state.
90-123: Route-driven step initialization and form-reactive validation look solidRebinding
currentStepin the constructor to atoSignaloverqueryParamMap(edit mode) vs.internalStep(create mode) gives predictable URL-backed navigation, and theform.valueChangessubscription (scoped withtakeUntilDestroyed(this.destroyRef)) plus theeffectthat re-evaluatesupdateCanProceed()on step changes keep the validation gate (canProceed) in sync without extra manual wiring.
179-223: Committee create/update flow is clear and reuses the same form-shaping logicBoth create and update paths share the same normalization of
form.value(calendar, display_name fallback, website/project_uid normalization) and then pass throughcleanFormData, which keeps the payloads consistent. The branching onisEditMode()intoupdateCommitteevscreateCommitteewith dedicated success/error handlers is straightforward and easy to follow.
268-348: Edit-mode combined save (committee + members) is well-batched and resilient
onSubmitAll()validates the form, shapescommitteeDataviacleanFormData, then runs the committee update and the (possibly empty) sequence of member operations in parallel viaforkJoin. Aggregating member results withreduceand routing them throughshowMemberOperationToast()provides clear feedback while keeping the committee success path simple. The error handler correctly leaves the user on the page with an error toast so they can retry.
388-408: Form group structure cleanly reflects the 3 configuration steps
createCommitteeFormGroup()clearly segments controls by wizard step (category, basic info, settings), including URL pattern validation forwebsite. This matches the HTML step breakdown and makes it easy to see which fields belong to which step, which will help maintainers extend or adjust validations per step later.
468-517: Step validation and title resolution correctly guard forward navigation
canNavigateToStep()’s “all previous steps must be valid” loop combined withisStepValid()perCOMMITTEE_FORM_STEPSguarantees that users can’t skip required steps while still allowing free back-navigation.getStepTitle()’s bounds check aroundstepTitlesavoids off-by-one issues and keeps the template-friendlycurrentStepTitlesafe.
519-538: Scroll behavior and name auto-generation are user-friendly touches
scrollToStepper()ensures the wizard header stays in view when changing steps, andgenerateGroupName()only auto-fills the name from the category when moving from step 1→2 in create mode and the name is still empty, which avoids clobbering user input while still providing a sensible default.
569-603: Per-member operation wrapping and toasts give clear aggregate feedback
createMemberOperation()normalizes each API call into a{ type, success, failed }envelope with local error catching and logging, andshowMemberOperationToast()then interprets the aggregate (all success / partial / all failed) into concise user messages. This separation should make it easy to tweak messaging or behavior later without touching the core operation wiring.
Summary
Changes
Jira Tickets
🤖 Generated with Claude Code