refactor(committees): migrate to global architecture #165
+369
−1,129
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Migrate committees functionality from project-scoped to global architecture: - Remove project-specific committee routes and components - Add global committee routes with new view page - Migrate committee components to global scope - Update services for global committee context - Add committee type colors to Tailwind safelist - Update related components and documentation LFXV2-765 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
|
Caution Review failedThe pull request is closed. 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. WalkthroughThis pull request performs a large-scale refactoring to migrate from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommitteeDashboard
participant CommitteeTable
participant Router
participant CommitteeService
Note over User,CommitteeService: Old Flow (Event Emitters)
User->>CommitteeTable: Click View
CommitteeTable->>CommitteeTable: onViewCommittee()
CommitteeTable->>CommitteeDashboard: emit viewCommittee
CommitteeDashboard->>CommitteeDashboard: onViewCommittee()
CommitteeDashboard->>Router: navigate()
User->>CommitteeTable: Click Edit
CommitteeTable->>CommitteeTable: onEditCommittee()
CommitteeTable->>CommitteeDashboard: emit editCommittee
CommitteeDashboard->>CommitteeDashboard: onEditCommittee()
CommitteeDashboard->>Router: navigate()
User->>CommitteeTable: Click Delete
CommitteeTable->>CommitteeDashboard: emit deleteCommittee
CommitteeDashboard->>CommitteeDashboard: onDeleteCommittee()
CommitteeDashboard->>CommitteeService: deleteCommittee()
CommitteeDashboard->>CommitteeDashboard: refreshCommittees()
sequenceDiagram
participant User
participant CommitteeTable
participant Router
participant ConfirmationService
participant CommitteeService
participant CommitteeTable2 as CommitteeTable<br/>(refresh)
Note over User,CommitteeTable2: New Flow (Route-Based Navigation & Refresh)
User->>CommitteeTable: Click View (anchor)
CommitteeTable->>Router: routerLink /groups/{uid}
Router->>Router: navigate
User->>CommitteeTable: Click Edit (routerLink)
CommitteeTable->>Router: routerLink /groups/{uid}/edit
Router->>Router: navigate
User->>CommitteeTable: Click Delete (button)
CommitteeTable->>ConfirmationService: confirm()
ConfirmationService->>User: Show confirmation dialog
User->>ConfirmationService: Accept
ConfirmationService->>CommitteeService: deleteCommittee()
CommitteeService->>CommitteeTable2: success
CommitteeTable2->>CommitteeTable2: refresh()
CommitteeTable2->>CommitteeDashboard: emit refresh
CommitteeDashboard->>CommitteeDashboard: refreshCommittees()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
Comment |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request migrates committees functionality from project-scoped to global architecture. The main changes include renaming projectId to uid across interfaces and services for consistency, removing 33 project-specific committee files, and adding new global committee routes and components.
Key Changes:
- Renamed
projectIdtouidacross interfaces, services, and components for consistency - Removed project-specific committee routes (
/project/:slug/committees) - Added global committee routes (
/groups/:id) with new view page - Updated committee components to work in global context
- Added committee type colors to Tailwind safelist for dynamic styling
Reviewed Changes
Copilot reviewed 42 out of 52 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/project.interface.ts | Renamed projectId to uid in ProjectSlugToIdResponse and ProjectContext interfaces |
| packages/shared/src/interfaces/analytics-data.interface.ts | Renamed projectId to uid in analytics interfaces |
| packages/shared/src/constants/committees.constants.ts | Changed finance committee color from amber to emerald |
| docs/architecture/backend/*.md | Updated documentation to reflect uid naming |
| apps/lfx-one/tailwind.config.js | Added committee type colors (emerald, pink, orange) to safelist |
| apps/lfx-one/src/server/services/*.ts | Updated services to use uid parameter name |
| apps/lfx-one/src/app/shared/services/*.service.ts | Renamed methods and parameters from projectId to uid |
| apps/lfx-one/src/app/modules/meetings/*.ts | Updated meeting components to use uid and improved SSR handling |
| apps/lfx-one/src/app/modules/committees/* | Major refactor of committee components for global scope |
| apps/lfx-one/src/app/modules/project/committees/* | Removed 33 project-scoped committee files |
| apps/lfx-one/src/app/modules/project/project.routes.ts | Removed project-specific committee routes |
Comments suppressed due to low confidence (1)
apps/lfx-one/src/app/modules/committees/components/member-form/member-form.component.ts:127
- [nitpick] The error message 'Member already exists' could be more helpful by indicating what action the user should take. Consider changing to 'This member is already part of the committee' or 'A member with this email already exists in the committee' for better clarity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.html
Outdated
Show resolved
Hide resolved
Remove afterNextRender wrapper from meeting-join component as toSignal already handles SSR properly with initialValue options. LFXV2-765 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/project.service.ts (1)
69-103: Critical bug: getProjectById uses incorrect tags formatThe code at line 72 passes a bare
uidto the query service, but the rest of the codebase—including other calls in the same controller—uses the namespaced formatproject_uid:${uid}:project.service.ts:72 tags: uid ← BARE UUID (WRONG) project.controller.ts:36-37 tags: `project_uid:${project.uid}` ← NAMESPACED (CORRECT) project.controller.ts:87-88 tags: `project_uid:${project.uid}` ← NAMESPACED (CORRECT) Consistent pattern across all services: - committee.service.ts:84 tags: `committee_uid:${committeeId}` - meeting.service.ts:261 tags: `meeting_uid:${meetingUid}` - meeting.service.ts:333 tags: `email:${email}`The query service expects namespaced tags. The bare
uidformat will not match the indexing and likely causes silent failures or empty results.Fix: Change line 72 to
tags: \project_uid:${uid}``apps/lfx-one/src/app/modules/committees/committees.routes.ts (1)
20-29: Critical route ordering issue::id/editmust come before:id.Angular's router matches routes sequentially. With the current order, a navigation to
/committees/<uid>/editwill incorrectly match the:idroute (treating 'edit' as theidparameter) before reaching:id/edit. This breaks the edit functionality.Apply this diff to fix the route order:
{ path: 'create', loadComponent: () => import('./committee-manage/committee-manage.component').then((m) => m.CommitteeManageComponent), canActivate: [authGuard], }, + { + path: ':id/edit', + loadComponent: () => import('./committee-manage/committee-manage.component').then((m) => m.CommitteeManageComponent), + canActivate: [authGuard], + }, { path: ':id', loadComponent: () => import('./committee-view/committee-view.component').then((m) => m.CommitteeViewComponent), canActivate: [authGuard], }, - { - path: ':id/edit', - loadComponent: () => import('./committee-manage/committee-manage.component').then((m) => m.CommitteeManageComponent), - canActivate: [authGuard], - },
🧹 Nitpick comments (5)
apps/lfx-one/src/app/modules/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (1)
61-77: <!-- [scratchpad_start]
[task verify_modal_opening_context 6/7]
Final key evidence gathered:ProjectContextService analysis:
- selectedProject and selectedFoundation are WritableSignals (can change at any time)
- No mechanism to lock context while modal is open
- Can be changed via setProject() or setFoundation() methods
Dashboard validation:
- Checks !project?.uid before loading meetings (lines 88-89)
- Meetings only shown when valid project exists
- BUT: Nothing prevents context from changing after modal opens
Route structure:
- Only authGuard, no project context guard
- Project context is globally mutable, not route-protected
[observations]
- Modal opens from meeting-card component (which is rendered by dashboard)
- toSignal subscribes immediately on MeetingCommitteeModalComponent init
- If projectContextService.getProjectUid() becomes falsy after modal opens, API will be called with empty UID
- This is an edge case but technically possible: user could clear project context while modal is opening
- Meeting data passed to modal (line 209 in meeting-card) could theoretically be used as fallback
[analysis]
- Original review concern is VALID but describes an edge case
- In normal UX flow, unlikely to occur (modal is blocking)
- However, projectContextService is globally mutable with no guards
- Suggestion to guard against empty UID is reasonable defensive programming
- Alternative: Could extract projectUid from meeting data if meeting is tied to project
[conclusion]
Original review is accurate. The suggestion to guard against missing project context is valid and represents reasonable defensive programming. This is an OPTIONAL refactoring (not critical bug, but good practice).[done]
Verification complete. Original review is correct and justified.
...lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
Remove redundant loading.set(false) calls in catchError blocks and rely on finalize as the single cleanup point for both upcoming and past meetings initialization. LFXV2-765 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
jordane
approved these changes
Nov 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Related JIRA
LFXV2-765
Changes
Architecture
/modules/project/committees//modules/committees/(global scope)/:id)Components
Services
Impact
Generated with Claude Code