feat(ui): add subcommittee support and enhanced filtering #43
Conversation
Add comprehensive committee management enhancements including: - Subcommittee support with parent committee selection dropdown - Enhanced filtering with dynamic counts for committee types and voting status - Committee table menu actions (View/Delete) following established patterns - Color-coded committee types with custom pipe for consistent styling - Performance improvements by replacing formatDate methods with Angular date pipe - Documentation updates for data transformation best practices The parent committee selection prevents circular references and only shows top-level committees as options. All filters now display counts and are sorted alphabetically for better UX. Generated with [Claude Code](https://claude.ai/code) Closes LFXV2-256 Signed-off-by: Asitha de Silva <[email protected]>
|
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 the Committees area into a single-column dashboard with summary metrics and filters (including voting status), introduces a standalone CommitteeTable, adds an optional parent committee selector in the form, removes subcommittees UI, adds committee-type color constants and pipe, updates the Committee interface, and adds Angular pipe guidance docs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as CommitteeDashboardComponent
participant Filters as Signals (search/category/votingStatus)
participant Table as CommitteeTableComponent
User->>Dashboard: Load dashboard
Dashboard->>Filters: Initialize signals and computed metrics
Dashboard->>Table: Pass filtered committees
User->>Dashboard: Update search/category/voting status
Dashboard->>Filters: Update filters and reset paging
Dashboard->>Table: Update committees input
User->>Table: Click Edit/View/Delete
Table-->>Dashboard: emit edit/view/delete events
Dashboard->>Dashboard: Handle navigation/confirm delete
sequenceDiagram
actor User
participant Form as CommitteeFormComponent
participant Project as ProjectService
participant Data as Committees API
User->>Form: Open create/edit committee dialog
Form->>Project: Get current projectId
alt projectId exists
Form->>Data: Fetch project committees (as Signal)
Data-->>Form: Committees list
Form->>Form: Build parent options (top-level, exclude current)
else no projectId
Form->>Form: Use default "No Parent Committee" option
end
User->>Form: Select Parent Committee (parent_uid)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive subcommittee support with enhanced filtering and improved performance to the committee management system. The changes focus on implementing hierarchical committee structures with parent-child relationships and modernizing the UI with better filters and performance optimizations.
Key Changes
- Added subcommittee functionality with parent committee selection in forms and hierarchical display in tables
- Enhanced filtering system with committee type and voting status filters that include dynamic counts
- Replaced method-based date formatting with Angular's built-in date pipe for better performance
- Implemented comprehensive menu actions (View/Delete) in committee tables using signal-based patterns
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/committee.interface.ts | Updated Committee interface to use parent_uid for hierarchical relationships |
| packages/shared/src/constants/committees.ts | Added committee type color mappings and utility functions for consistent styling |
| docs/architecture/frontend/angular-patterns.md | Added documentation for pipe vs method best practices in templates |
| apps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.ts | Created new pipe for committee type color transformation |
| apps/lfx-pcc/src/app/shared/components/treetable/ | New reusable TreeTable component for hierarchical data display |
| apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/ | Simplified component by removing unused button imports and elements |
| apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/ | New dedicated table component with menu actions and hierarchical display |
| apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/ | Added parent committee dropdown with proper filtering logic |
| apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/ | Enhanced dashboard with statistics, improved filters, and table integration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
...pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts
Show resolved
Hide resolved
...src/app/modules/project/committees/components/committee-table/committee-table.component.html
Show resolved
Hide resolved
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (3)
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (1)
33-41: Fix runtime error: computed reads upcomingMeeting before it’s initializedinitializeCommittees() uses a computed that calls this.upcomingMeeting(). Since committees is initialized in the constructor and upcomingMeeting is assigned later in ngOnInit, the computed will attempt to call undefined as a function and throw.
Move committees initialization to after upcomingMeeting is set.
public committees!: Signal<string>; - public constructor() { - this.committees = this.initializeCommittees(); - } + public constructor() {} public ngOnInit() { runInInjectionContext(this.injector, () => { this.upcomingMeeting = this.initializeUpcomingMeeting(); + this.committees = this.initializeCommittees(); }); }Alternative (if you prefer to keep constructor init): guard the call site inside initializeCommittees:
return computed(() => this.upcomingMeeting?.()?.meeting_committees?.map(c => c.name).join(', ') ?? '');apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (1)
37-43: Loading state bug: template uses loading(), code toggles submitting().The button binds to loading(), but only submitting is toggled in code. Either switch the template to submitting() or derive loading from submitting.
Apply this diff to mirror submitting:
// Create form group internally public form = signal<FormGroup>(this.createCommitteeFormGroup()); - public loading = signal<boolean>(false); + public loading = computed(() => this.submitting());This keeps the template unchanged while reflecting the actual submit state.
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
241-251: Make committees initialization reactive to project changesinitializeCommittees reads project() once. If project signal resolves after construction, committees would stay []. Make it reactive to project changes by combining the project signal with refresh.
Apply these diffs:
- Update rxjs-interop import:
-import { toSignal } from '@angular/core/rxjs-interop'; +import { toSignal, toObservable } from '@angular/core/rxjs-interop';
- Add combineLatest import:
-import { BehaviorSubject, of } from 'rxjs'; +import { BehaviorSubject, of, combineLatest } from 'rxjs';
- Rewrite initializeCommittees:
- private initializeCommittees(): Signal<Committee[]> { - return toSignal( - this.project() - ? this.refresh.pipe( - tap(() => this.committeesLoading.set(true)), - switchMap(() => this.committeeService.getCommitteesByProject(this.project()!.uid).pipe(tap(() => this.committeesLoading.set(false)))) - ) - : of([]), - { initialValue: [] } - ); - } + private initializeCommittees(): Signal<Committee[]> { + const project$ = toObservable(this.project); + return toSignal( + combineLatest([project$, this.refresh]).pipe( + tap(() => this.committeesLoading.set(true)), + switchMap(([project]) => { + if (!project) { + this.committeesLoading.set(false); + return of([]); + } + return this.committeeService + .getCommitteesByProject(project.uid) + .pipe(tap(() => this.committeesLoading.set(false))); + }) + ), + { initialValue: [] } + ); + }Also applies to: 20-21, 6-6
🧹 Nitpick comments (17)
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html (1)
32-32: Mark decorative icon as aria-hidden for accessibilityThe calendar icon is decorative. Hide it from assistive technologies.
- <i class="fa-light fa-calendar text-xl text-gray-400 mb-2"></i> + <i class="fa-light fa-calendar text-xl text-gray-400 mb-2" aria-hidden="true"></i>docs/architecture/frontend/angular-patterns.md (1)
168-197: Refine guidance: prefer pipes, but allow signals; clarify change-detection noteThe blanket “always” statement conflicts with our established use of Angular Signals in templates (memoized; not re-executed each CD cycle). Adjust wording to avoid discouraging signal usage and fix the CD note.
-For data transformation in templates, always use Angular pipes instead of component methods: +Prefer Angular pipes for data transformation in templates. Angular Signals (e.g., `value()`) are also appropriate for simple, memoized transformations. Avoid invoking arbitrary component methods in templates:-5. **Change Detection**: Methods are called on every change detection cycle, pipes only when needed +5. **Change Detection**: Regular component methods are called on every change detection cycle; pipes and signals re-run only when their inputs changeOptional: Add a short note after the GOOD example:
+> Note: When you see calls like `size()` or `styleClass()` in templates across this codebase, those are Angular Signals, not plain methods. Signals are memoized and safe to use in templates.This aligns with prior learnings about signals usage in templates.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html (1)
39-52: Add data-testid to new Parent Committee selector (testing guideline).Per repo guidelines for apps/**/*.html, add a data-testid. Suggest consistent naming: section-component-element.
Apply this diff:
<lfx-select size="small" [form]="form()" control="parent_uid" [options]="parentCommitteeOptions()" placeholder="Select a parent committee" styleClass="w-full" id="parent-committee" - [showClear]="true"></lfx-select> + [showClear]="true" + [attr.data-testid]="'committees-committee-form-parent-select'"></lfx-select>apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (3)
51-53: Good addition; consider reusing a typed option interface.parentCommitteeOptions is clear. If this pattern is reused, consider a shared Option interface to avoid repetition.
179-207: Sort parent options alphabetically (deterministic UX, aligns with filter sorting goal).Sorting by committee name improves predictability and aligns with the PR’s emphasis on alphabetically sorted options.
Apply this diff:
// Transform to dropdown options - const options = availableCommittees.map((committee) => ({ - label: committee.name, - value: committee.id, - })); + const options = availableCommittees + .map((committee) => ({ + label: committee.name, + value: committee.id, + })) + .sort((a, b) => a.label.localeCompare(b.label, undefined, { sensitivity: 'base' }));
214-214: Potential type/contract mismatch: parent_uid null vs interface string.The form sets parent_uid to null for “No Parent Committee”, but Committee.parent_uid is typed as string in packages/shared/src/interfaces/committee.interface.ts. Confirm backend expects null; otherwise map null to '' or omit the field before submit, or update the interface to string | null.
If backend expects empty string or missing property, adjust the submit payload:
// In onSubmit(), before calling the service const formValue = this.form().value as any; const payload = { ...formValue, parent_uid: formValue.parent_uid ?? '', // or: delete parent_uid if null }; // use payload instead of formValue for create/updateAlternatively, update Committee interface to allow null:
export interface Committee { // … parent_uid: string | null; // … }apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html (6)
5-6: Add data-testid to table root (testing guideline).Add a stable selector for E2E and component tests.
Apply this diff:
- <lfx-table [value]="organizedCommittees()" styleClass="committee-table"> + <lfx-table [value]="organizedCommittees()" styleClass="committee-table" [attr.data-testid]="'committees-committee-table-root'">
15-17: Rename header to “Committee Type” for consistency with PR objective.The PR objective renames “Category” to “Committee Type.” Update the column header accordingly.
Apply this diff:
- <span class="text-xs font-medium text-gray-700">Type</span> + <span class="text-xs font-medium text-gray-700">Committee Type</span>
69-71: Prefer ngClass over class interpolation for composability.Using [ngClass] avoids class string collisions and is more idiomatic.
Apply this diff:
- <span class="text-xs font-medium {{ committee.category | committeeTypeColor }}">{{ committee.category }}</span> + <span class="text-xs font-medium" [ngClass]="committee.category | committeeTypeColor">{{ committee.category }}</span>
38-59: Standardize data-testid naming to guideline format.Adopt the '[section]-[component]-[element]' convention to improve test readability and searchability.
Apply this diff:
- <tr class="border-b border-gray-50 hover:bg-gray-50/50 transition-colors" [attr.data-testid]="'committee-table-row-' + committee.id"> + <tr class="border-b border-gray-50 hover:bg-gray-50/50 transition-colors" [attr.data-testid]="'committees-committee-table-row-' + committee.id">And for action buttons below (also see lines 124, 133, 142):
- [attr.data-testid]="'committee-edit-' + committee.id" /> + [attr.data-testid]="'committees-committee-table-edit-' + committee.id" /> - [attr.data-testid]="'committee-members-' + committee.id" /> + [attr.data-testid]="'committees-committee-table-members-' + committee.id" /> - [attr.data-testid]="'committee-more-' + committee.id" /> + [attr.data-testid]="'committees-committee-table-more-' + committee.id" />
151-159: Add data-testid to empty state for deterministic assertions.Provide a stable selector for “no data” assertions.
Apply this diff:
- <td colspan="8" class="text-center py-8"> + <td colspan="8" class="text-center py-8" [attr.data-testid]="'committees-committee-table-empty'">
116-143: Consider explicit accessible labels for icon-only buttons.If lfx-button doesn’t propagate aria-label automatically, add an accessible label to improve a11y and screen reader support.
If supported by lfx-button, add ariaLabel (or set label with visually-hidden text):
- <lfx-button + <lfx-button icon="fa-light fa-edit" [text]="true" [rounded]="true" size="small" severity="secondary" styleClass="h-8 w-8 p-0 text-gray-500 hover:text-gray-700" (onClick)="onEdit(committee)" - [attr.data-testid]="'committee-edit-' + committee.id" /> + [attr.data-testid]="'committees-committee-table-edit-' + committee.id" + ariaLabel="Edit committee {{ committee.name }}" />Please apply similarly for Members and More buttons if the API supports it. If not, we can add aria-label via [attr.aria-label].
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html (1)
47-49: Optional: Move percent computation into TS to reduce template workThe percent expression invokes computed signals multiple times in template. Moving it into a computed signal (e.g., publicCommitteesPercent) in the component improves readability and reduces redundant calls.
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts (2)
4-14: Enable OnPush change detection for performanceWrapper components benefit from OnPush to minimize checks.
Apply this diff:
-import { CommonModule } from '@angular/common'; -import { Component, ContentChild, input, output, TemplateRef } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { Component, ContentChild, input, output, TemplateRef, ChangeDetectionStrategy } from '@angular/core'; @@ @Component({ selector: 'lfx-treetable', standalone: true, imports: [CommonModule, TreeTableModule], templateUrl: './treetable.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })
29-39: Type the expand/collapse outputs to concrete PrimeNG event shapesOutputs and handlers are typed as any. If available in your PrimeNG version, use the specific event types for TreeTable expand/collapse to improve DX and prevent regressions.
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (2)
359-364: Menu item “disabled” won’t react to isDeleting changesMenuItem.disabled is evaluated at creation time; the runtime signal change won’t update it. Guard in the command instead (or rebuild items on state change).
- styleClass: 'text-red-500', - disabled: this.isDeleting(), - command: () => this.deleteCommittee(), + styleClass: 'text-red-500', + command: () => { + if (!this.isDeleting()) { + this.deleteCommittee(); + } + },
65-67: Consider extracting a typed FilterOption interfaceMultiple places use { label: string; value: string | null }. Define a shared FilterOption for consistency and maintainability (e.g., in packages/shared). This aligns with the repo’s emphasis on type safety.
Also applies to: 91-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts(10 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts(4 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts(1 hunks)apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.html(1 hunks)apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts(1 hunks)apps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.ts(1 hunks)docs/architecture/frontend/angular-patterns.md(1 hunks)packages/shared/src/constants/committees.ts(1 hunks)packages/shared/src/interfaces/committee.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{ts,tsx,js,jsx,css,scss}
📄 CodeRabbit Inference Engine (CLAUDE.md)
License headers are required on all source files
Files:
packages/shared/src/interfaces/committee.interface.tsapps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.tspackages/shared/src/constants/committees.tsapps/lfx-pcc/src/app/shared/components/treetable/treetable.component.tsapps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts
packages/shared/src/{interfaces,constants,enums}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-pcc/shared package
Files:
packages/shared/src/interfaces/committee.interface.tspackages/shared/src/constants/committees.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
packages/shared/src/interfaces/committee.interface.tsapps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.tspackages/shared/src/constants/committees.tsapps/lfx-pcc/src/app/shared/components/treetable/treetable.component.tsapps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts
packages/shared/src/interfaces/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Interfaces go into the shared packages
Files:
packages/shared/src/interfaces/committee.interface.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/interfaces/committee.interface.tsapps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.tsapps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.tspackages/shared/src/constants/committees.tsapps/lfx-pcc/src/app/shared/components/treetable/treetable.component.tsapps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts
apps/**/src/**/*.html
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/**/src/**/*.html: Always add data-testid attributes when creating new components for reliable test targeting
Use data-testid naming convention - '[section]-[component]-[element]' for hierarchical structure
Files:
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.htmlapps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.htmlapps/lfx-pcc/src/app/shared/components/treetable/treetable.component.htmlapps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.htmlapps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
🧠 Learnings (2)
📚 Learning: 2025-08-06T00:40:54.228Z
Learnt from: asithade
PR: linuxfoundation/lfx-v2-pcc-ui#24
File: apps/lfx-pcc/src/app/shared/components/multi-select/multi-select.component.html:16-18
Timestamp: 2025-08-06T00:40:54.228Z
Learning: In Angular applications, when reviewing template code that uses parentheses syntax like `size()` or `styleClass()`, these may be Angular signals rather than regular methods. Angular signals are memoized and only re-compute when their dependencies change, unlike regular methods which execute on every change detection cycle. Therefore, the performance concerns about avoiding method calls in templates don't apply to signals.
Applied to files:
docs/architecture/frontend/angular-patterns.md
📚 Learning: 2025-08-07T16:35:57.360Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-pcc-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T16:35:57.360Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts
🧬 Code Graph Analysis (5)
apps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.ts (1)
packages/shared/src/constants/committees.ts (1)
getCommitteeTypeColor(70-72)
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts (4)
apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (1)
Component(16-73)apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts (1)
Component(9-40)apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
Component(27-367)packages/shared/src/interfaces/committee.interface.ts (1)
Committee(4-28)
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts (4)
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts (1)
Component(15-108)apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (1)
Component(22-227)apps/lfx-pcc/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (1)
Component(16-73)apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
Component(27-367)
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (3)
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts (1)
Component(15-108)apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts (1)
Component(24-223)packages/shared/src/interfaces/committee.interface.ts (1)
Committee(4-28)
apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (1)
packages/shared/src/interfaces/committee.interface.ts (1)
Committee(4-28)
🪛 LanguageTool
docs/architecture/frontend/angular-patterns.md
[grammar] ~164-~164: There might be a mistake here.
Context: ...## 🔄 Data Transformation Best Practices ### Use Pipes Instead of Methods For data tr...
(QB_NEW_EN)
⏰ 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: Code Quality Checks
🔇 Additional comments (15)
apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html (1)
39-52: LGTM: Optional parent control wired to Signals and clearable UI.Binding to a Signal-based options provider and allowing clear maps well to the subcommittee design. The helper text is clear.
apps/lfx-pcc/src/app/shared/pipes/committee-type-colors.pipe.ts (1)
1-15: LGTM: Focused, pure pipe leveraging shared mapping.
- License header present.
- Standalone pipe is minimal and reusable.
- Relies on centralized getCommitteeTypeColor for consistency.
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.html (1)
1-34: LGTM: Clean content projection with clear contexts.Header/body/emptymessage templates are guarded and pass a well-structured context. Event wiring is straightforward.
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html (1)
120-127: LGTM: Dedicated table component + test idGood move to encapsulate the table into lfx-committee-table and expose a stable data-testid="committee-dashboard-table" for automation. This aligns with the new component architecture and testability goals.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts (2)
42-44: Verify parent_uid nullability vs interface typingLogic treats “no parent” as falsy (!c.parent_uid), but the shared interface types parent_uid as string (non-null). If backend returns null, the typing is inaccurate; if it returns '', logic still works but is brittle.
Would you confirm the backend payload for committees with no parent?
- If null is possible/preferred, update the interface to parent_uid: string | null in packages/shared/src/interfaces/committee.interface.ts and keep the truthy checks.
- If empty string is used, consider normalizing in a mapper to null to avoid edge cases.
80-107: LGTM: Action menu and event outputsMenu setup is clean; selectedCommittee via signal + command closures is straightforward. View/Delete emitters match the dashboard’s handlers.
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.ts (1)
9-20: LGTM: PrimeNG wrapped behind LFX façadeThis wrapper aligns with the “wrap PrimeNG components” learning. Content projection and event pass-through look good.
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (8)
75-79: LGTM: Statistics signalsComputed totals for total/public/voting are clean and feed the dashboard metrics effectively.
113-118: LGTM: Voting status filter handlerHandler updates the signal and resets paging as expected.
120-123: LGTM: Search resets paginationKeeps UX consistent during search refinement.
144-159: LGTM: Table action wiringChild component emits are correctly mapped to view/edit/delete flows via selectedCommittee.
231-234: LGTM: Added votingStatus controlForm wiring looks correct and consistent with the new filter.
253-276: LGTM: Committee type options with counts and alphabetical orderMeets the “Committee Type” filter requirement with counts and sorting.
278-291: LGTM: Voting status options with countsLabels and counts align with objectives; “All Voting Status” default is clear.
315-324: LGTM: Voting status filtering logicClear mapping from filter value to boolean flags (enabled/disabled).
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Show resolved
Hide resolved
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Show resolved
Hide resolved
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Show resolved
Hide resolved
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Show resolved
Hide resolved
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Outdated
Show resolved
Hide resolved
...cc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
Show resolved
Hide resolved
...c/src/app/modules/project/committees/components/committee-table/committee-table.component.ts
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/treetable/treetable.component.html
Outdated
Show resolved
Hide resolved
- Add conditional check to prevent date pipe failure when updated_at is undefined - Display '-' as fallback when no updated_at value exists - Ensures table renders correctly for committees without update timestamps 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
- Add data-testid attributes for all UI elements: New Committee CTA, statistics cards, search input, filters, and Add First Committee button - Map all 19 committee categories to specific colors based on meeting type color schemes - Group committees logically: Board/governance (red), Technical (purple/blue), Legal/compliance (amber), Marketing (green), Finance (emerald), Working groups (orange/yellow), Other (gray) - Remove unused treetable component to reduce bundle size - Ensure comprehensive test coverage with proper element targeting 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
- Remove subcommittees table section that referenced non-existent data structure - Remove subcommittees count display from committee stats - Clean up unused TableComponent import and Injector dependency - Add TODO comments for future subcommittee implementation - Fixes TypeScript build errors and reduces bundle size 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts (2)
17-17: Add error handling to seterror/loadingon fetch failuresNetwork errors from
getCommittee/getCommitteeMemberscurrently surface unhandled, leavingloading/membersLoadingstuck. AddcatchErrorto set flags and returnnull.Apply:
-import { BehaviorSubject, combineLatest, of, switchMap } from 'rxjs'; +import { BehaviorSubject, combineLatest, of, switchMap, catchError } from 'rxjs'; @@ private initializeCommittee(): Signal<Committee | null> { return toSignal( combineLatest([this.route.paramMap, this.refresh]).pipe( switchMap(([params]) => { const committeeId = params?.get('id'); if (!committeeId) { this.error.set(true); return of(null); } return combineLatest([this.committeeService.getCommittee(committeeId), this.committeeService.getCommitteeMembers(committeeId)]).pipe( switchMap(([committee, members]) => { this.members.set(members); this.loading.set(false); this.membersLoading.set(false); return of(committee); }) - ); + ); }) - ), + ), + catchError((err) => { + console.error('Failed to load committee or members:', err); + this.error.set(true); + this.loading.set(false); + this.membersLoading.set(false); + return of(null); + }), { initialValue: null } ); }Also applies to: 147-169
171-189: MakeactionMenuItemsreactive via a computed SignalThe current
disabled: this.isDeleting()is evaluated only once at init and won’t update asdeletingchanges. ConvertactionMenuItemsinto aSignal<MenuItem[]>so itsdisabledflag stays in sync:• In committee-view.component.ts:
- Change
topublic actionMenuItems: MenuItem[];public readonly actionMenuItems: Signal<MenuItem[]>;- Import from
@angular/core:import { computed, Signal } from '@angular/core';• Replace your
initializeActionMenuItems()to return a computed signal:
diff -private initializeActionMenuItems(): MenuItem[] { - return [ - { label: 'Edit', icon: 'fa-light fa-edit', command: () => this.editCommittee() }, - { separator: true }, - { - label: 'Delete', - icon: 'fa-light fa-trash', - styleClass: 'text-red-500', - disabled: this.isDeleting(), - command: () => this.deleteCommittee(), - }, - ]; -} +private initializeActionMenuItems(): Signal<MenuItem[]> { + return computed(() => [ + { label: 'Edit', icon: 'fa-light fa-edit', command: () => this.editCommittee() }, + { separator: true }, + { + label: 'Delete', + icon: 'fa-light fa-trash', + styleClass: 'text-red-500', + disabled: this.isDeleting(), + command: () => this.deleteCommittee(), + }, + ]); +}
• In the template, call the signal rather than bind it directly:-<p-menu [model]="actionMenuItems"></p-menu> +<p-menu [model]="actionMenuItems()"></p-menu>
🧹 Nitpick comments (5)
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts (2)
89-91: SetmembersLoadingto true when refreshing membersOn refresh, the loading indicator never re-enables. Flip it to true before triggering the refresh stream.
Apply:
public refreshMembers(): void { - this.refresh.next(); + this.membersLoading.set(true); + this.refresh.next(); }
191-204: Consider using Angular DatePipe in the template for i18n and consistencyOptional: Replace manual date formatting signals with the built-in
datepipe in the template (respectsLOCALE_ID, reduces code). Keep if template already consumes these signals widely.Also applies to: 206-219
packages/shared/src/constants/committees.ts (2)
95-97: Trim input before lookup to avoid accidental defaultingGuard against trailing/leading spaces from APIs or user input.
Apply:
export function getCommitteeTypeColor(type: string): string { - return COMMITTEE_TYPE_COLORS[type as keyof typeof COMMITTEE_TYPE_COLORS] || DEFAULT_COMMITTEE_TYPE_COLOR; + const normalized = (type ?? '').trim(); + return COMMITTEE_TYPE_COLORS[normalized as keyof typeof COMMITTEE_TYPE_COLORS] || DEFAULT_COMMITTEE_TYPE_COLOR; }
85-91: Export aCommitteeTypealias for stronger typing in consumersExpose
CommitteeType = keyof typeof COMMITTEE_TYPE_COLORSso component inputs/forms can constrain to known values where appropriate (while still allowingstringwhere backend may be open-ended).Apply:
} as const; +export type CommitteeType = keyof typeof COMMITTEE_TYPE_COLORS; + /** * Default color scheme for unknown committee types */ export const DEFAULT_COMMITTEE_TYPE_COLOR = 'text-gray-500';apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html (1)
23-31: Align data-testid values to the '[section]-[component]-[element]' conventionIds are present (good), but they don’t follow the standardized naming. Rename to stable, hierarchical selectors and add missing hooks for key values.
Apply:
- <lfx-button - label="New Committee" - icon="fa-light fa-plus" - severity="primary" - (onClick)="openCreateDialog()" - size="small" - data-testid="committee-new-cta"> + <lfx-button + label="New Committee" + icon="fa-light fa-plus" + severity="primary" + (onClick)="openCreateDialog()" + size="small" + data-testid="committee-dashboard-header-new-committee-btn"> </lfx-button> @@ - <div class="grid grid-cols-1 md:grid-cols-4 gap-3 w-full"> - <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="stats-total-committees"> + <div class="grid grid-cols-1 md:grid-cols-4 gap-3 w-full"> + <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="committee-dashboard-stats-total-card"> <div class="p-3"> <div class="text-[10px] text-gray-500 font-normal mb-1">Total Committees</div> <div class="flex items-center gap-2"> - <span class="text-[24px] text-gray-900 font-normal">{{ totalCommittees() }}</span> + <span class="text-[24px] text-gray-900 font-normal" data-testid="committee-dashboard-stats-total">{{ totalCommittees() }}</span> <div class="flex items-center gap-1 text-emerald-600"> <i class="fa-light fa-trending-up text-[8px]"></i> <span class="text-[10px] font-medium">+2 this week</span> </div> </div> </div> </lfx-card> @@ - <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="stats-public-committees"> + <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="committee-dashboard-stats-public-card"> <div class="p-3"> <div class="text-[10px] text-gray-500 font-normal mb-1">Public Committees</div> <div class="flex items-center gap-2"> - <span class="text-[24px] text-gray-900 font-normal">{{ publicCommittees() }}</span> + <span class="text-[24px] text-gray-900 font-normal" data-testid="committee-dashboard-stats-public">{{ publicCommittees() }}</span> <div class="inline-flex items-center gap-1 px-1.5 py-0.5 rounded bg-blue-100 text-blue-600 border border-blue-200"> - <span class="text-[9px] font-medium" - >{{ totalCommittees() > 0 ? ((publicCommittees() / totalCommittees()) * 100 | number: '1.0-0') : 0 }}% of total</span - > + <span class="text-[9px] font-medium" data-testid="committee-dashboard-stats-public-percent"> + {{ totalCommittees() > 0 ? ((publicCommittees() / totalCommittees()) * 100 | number: '1.0-0') : 0 }}% of total + </span> </div> </div> </div> </lfx-card> @@ - <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="stats-active-voting"> + <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full" data-testid="committee-dashboard-stats-active-voting-card"> <div class="p-3"> <div class="text-[10px] text-gray-500 font-normal mb-1">Active Voting</div> <div class="flex items-center gap-2"> - <span class="text-[24px] text-gray-900 font-normal">{{ activeVoting() }}</span> + <span class="text-[24px] text-gray-900 font-normal" data-testid="committee-dashboard-stats-active-voting">{{ activeVoting() }}</span> <div class="inline-flex items-center gap-1 px-1.5 py-0.5 rounded bg-green-100 text-green-600 border border-green-200"> <span class="text-[9px] font-medium">Voting enabled</span> </div> </div> </div> </lfx-card> @@ - <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full items-center" data-testid="stats-upcoming-meeting"> + <lfx-card styleClass="bg-white border border-gray-100 shadow-sm h-full items-center" data-testid="committee-dashboard-stats-upcoming-meeting-card"> <div class="p-3"> <div class="text-[10px] text-gray-500 font-normal mb-1">Upcoming Meeting</div> - <lfx-upcoming-committee-meeting></lfx-upcoming-committee-meeting> + <lfx-upcoming-committee-meeting data-testid="committee-dashboard-stats-upcoming-meeting"></lfx-upcoming-committee-meeting> </div> </lfx-card> @@ - <lfx-input-text + <lfx-input-text + data-testid="committee-dashboard-filters-search" [form]="searchForm" control="search" placeholder="Search committees..." icon="fa-light fa-search" styleClass="w-full" size="small" - data-testid="committee-search-input"></lfx-input-text> + ></lfx-input-text> @@ - <lfx-select + <lfx-select + data-testid="committee-dashboard-filters-type-select" [form]="searchForm" control="category" size="small" [options]="categories()" placeholder="Select Committee Type" [showClear]="true" styleClass="w-full" - (onChange)="onCategoryChange($event.value)" - data-testid="committee-type-filter"></lfx-select> + (onChange)="onCategoryChange($event.value)"></lfx-select> @@ - <lfx-select + <lfx-select + data-testid="committee-dashboard-filters-voting-select" [form]="searchForm" control="votingStatus" size="small" [options]="votingStatusOptions()" placeholder="Select Voting Status" [showClear]="true" styleClass="w-full" - (onChange)="onVotingStatusChange($event.value)" - data-testid="committee-voting-status-filter"></lfx-select> + (onChange)="onVotingStatusChange($event.value)"></lfx-select> @@ - <lfx-button - label="Add First Committee" - icon="fa-light fa-people-group" - severity="secondary" - (onClick)="openCreateDialog()" - data-testid="committee-add-first"></lfx-button> + <lfx-button + label="Add First Committee" + icon="fa-light fa-people-group" + severity="secondary" + (onClick)="openCreateDialog()" + data-testid="committee-dashboard-empty-add-first-btn"></lfx-button>Also applies to: 33-61, 62-73, 74-79, 88-96, 100-110, 114-124, 147-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html(0 hunks)apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html(1 hunks)packages/shared/src/constants/committees.ts(1 hunks)packages/shared/src/interfaces/committee.interface.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{ts,tsx,js,jsx,css,scss}
📄 CodeRabbit Inference Engine (CLAUDE.md)
License headers are required on all source files
Files:
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.tspackages/shared/src/constants/committees.tspackages/shared/src/interfaces/committee.interface.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.tspackages/shared/src/constants/committees.tspackages/shared/src/interfaces/committee.interface.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.tspackages/shared/src/constants/committees.tspackages/shared/src/interfaces/committee.interface.ts
packages/shared/src/{interfaces,constants,enums}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-pcc/shared package
Files:
packages/shared/src/constants/committees.tspackages/shared/src/interfaces/committee.interface.ts
packages/shared/src/interfaces/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Interfaces go into the shared packages
Files:
packages/shared/src/interfaces/committee.interface.ts
apps/**/src/**/*.html
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/**/src/**/*.html: Always add data-testid attributes when creating new components for reliable test targeting
Use data-testid naming convention - '[section]-[component]-[element]' for hierarchical structure
Files:
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
🧠 Learnings (2)
📚 Learning: 2025-08-07T16:35:57.360Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-pcc-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T16:35:57.360Z
Learning: Applies to apps/**/src/**/*.html : Always add data-testid attributes when creating new components for reliable test targeting
Applied to files:
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
📚 Learning: 2025-08-07T16:35:57.360Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-pcc-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T16:35:57.360Z
Learning: Applies to apps/**/src/**/*.html : Use data-testid naming convention - '[section]-[component]-[element]' for hierarchical structure
Applied to files:
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.html
⏰ 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: Code Quality Checks
🔇 Additional comments (3)
packages/shared/src/interfaces/committee.interface.ts (2)
8-8: LGTM:parent_uidnullability enables top-level committeesMaking
parent_uidnullable reflects top-level committees and prevents circular references. This aligns with the subcommittee model.
8-8: Verify APIparent_uidpresence across all endpointsPlease confirm with the backend whether the JSON payload for every
Committeealways includes aparent_uidfield (usingnullfor top-level) rather than omitting it.
- If the API does guarantee
parent_uidis always present (possiblynull), update the interface to:// packages/shared/src/interfaces/committee.interface.ts export interface Committee { // … parent_uid: string | null; // remove the `?` }- Otherwise, leave it optional (
parent_uid?: string | null) and ensure all consumers handle bothnullandundefined.Current code relies on truthiness (
!c.parent_uid,c.parent_uid === parent.id) and the form initializes missing values tonull. Adjust only once you’ve confirmed the API’s behavior.packages/shared/src/constants/committees.ts (1)
52-85: Good coverage and clear mapping to categoriesColor mapping covers all defined committee categories with a sensible default and aligns to meeting types. Nice use of
as constfor type-safe lookup.
Summary
Changes Made
formatDatemethods with Angular's built-indatepipeTest Plan
🤖 Generated with Claude Code