-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dashboards): foundation project selector and analytics refactoring #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change implements a comprehensive foundation project selector system with hierarchical context management across dashboards and meetings. Related JIRA tickets: - LFXV2-723: Foundation project selector with hierarchical context - LFXV2-724: Analytics API refactoring with required parameters - LFXV2-725: Dashboard foundation context integration - LFXV2-726: Meetings child project selection support Changes included: - New ProjectSelectorComponent with dropdown UI for foundation selection - Enhanced ProjectContextService with separate foundation/sub-project signals - Foundation context display across all dashboard types - Analytics API refactoring to require projectSlug parameter - Removed board meeting attendance feature - Organization involvement component with graceful error handling - Meeting creation for child projects within foundation context - Database schema updates and field name changes Generated with [Claude Code](https://claude.ai/code) 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. WalkthroughAdds foundation-level project selection and UI: new ProjectSelector component, sidebar integration, project/foundation state in ProjectContextService, dashboard bindings to selected foundation, meeting selection support, and analytics/controller/service updates to accept foundation/project slug and remove attendance metrics. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Sidebar as Sidebar Component
participant Selector as ProjectSelector
participant Context as ProjectContextService
participant Dashboard as Dashboard Component
participant Analytics as AnalyticsService
participant Backend
User->>Sidebar: open/choose project selector
Sidebar->>Selector: render projects (projects())
User->>Selector: select foundation
Selector->>Context: setFoundation(selected)
Context-->>Context: persist foundation to storage
Dashboard->>Context: selectedFoundation()
Dashboard->>Dashboard: render foundation header
Dashboard->>Analytics: getBoardMemberDashboard(accountId, foundationSlug)
Analytics->>Backend: request with accountId & projectSlug
Backend-->>Analytics: filtered data (attendance removed)
Analytics-->>Dashboard: return data
Dashboard->>Dashboard: update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
🔇 Additional comments (6)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comprehensive foundation project selector system with hierarchical context management and refactors analytics APIs to use required parameters and project slugs instead of IDs.
Key Changes:
- Added foundation-level project selector with separate state management from sub-project selection
- Refactored analytics APIs to require
accountIdandprojectSlugparameters, removing board meeting attendance tracking - Integrated foundation context display across all dashboards with graceful error handling
- Enhanced meeting creation flow to support child project selection within foundations
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/analytics-data.interface.ts |
Updated interfaces to remove board meeting attendance fields and rename date fields |
apps/lfx-one/src/server/services/organization.service.ts |
Refactored to use projectSlug instead of projectId, updated database schema references |
apps/lfx-one/src/server/controllers/analytics.controller.ts |
Added required parameter validation for accountId and projectSlug |
apps/lfx-one/src/app/shared/services/analytics.service.ts |
Updated API calls to match new required parameter signatures |
apps/lfx-one/src/app/shared/services/project-context.service.ts |
Enhanced with separate foundation and project state management using localStorage |
apps/lfx-one/src/app/shared/components/sidebar/* |
Integrated new project selector component with foundation selection |
apps/lfx-one/src/app/shared/components/project-selector/* |
New component for foundation project selection with PrimeNG Popover UI |
apps/lfx-one/src/app/modules/project/meetings/components/* |
Added child project selector to meeting creation flow |
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/* |
Added create meeting button with foundation context integration |
apps/lfx-one/src/app/modules/dashboards/board-member/* |
Integrated foundation context display and updated data loading |
apps/lfx-one/src/app/modules/dashboards/core-developer/* |
Added foundation header display |
apps/lfx-one/src/app/modules/dashboards/maintainer/* |
Added foundation header display |
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/* |
Added foundation selection validation with empty state handling |
apps/lfx-one/src/app/layouts/main-layout/* |
Enabled project selector in sidebar for both desktop and mobile views |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.html
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (9)
apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html (2)
4-53: Consider adding keyboard navigation support.The interactive dropdown works well with mouse interaction, but the clickable div (line 6-8) may not be fully accessible via keyboard. Consider adding
tabindex="0",role="button", and keyboard event handlers (Enter/Space) to ensure keyboard users can activate the dropdown.Example enhancement:
<div class="box-border content-stretch flex gap-3 items-center overflow-clip p-3 relative w-full cursor-pointer hover:bg-gray-50 transition-colors rounded-lg" (click)="togglePanel($event, popover)" + (keydown.enter)="togglePanel($event, popover)" + (keydown.space)="togglePanel($event, popover); $event.preventDefault()" + tabindex="0" + role="button" + [attr.aria-expanded]="popover.visible" data-testid="project-selector">
33-48: Consider adding keyboard navigation for project options.Similar to the dropdown trigger, the project option divs should support keyboard interaction for full accessibility.
Example enhancement:
<div class="flex items-center gap-3 p-2 hover:bg-gray-100 rounded-lg cursor-pointer transition-colors" (click)="selectProject(project, popover)" + (keydown.enter)="selectProject(project, popover)" + (keydown.space)="selectProject(project, popover); $event.preventDefault()" + tabindex="0" + role="option" [attr.data-testid]="'project-option-' + project.slug">apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html (1)
5-8: Consider handling the no-foundation-selected state.The Foundation Project header displays
{{ selectedFoundation()?.name }}but provides no fallback when a foundation hasn't been selected yet. This results in an empty or incomplete header on initial load.Consider adding conditional rendering or a placeholder message:
- <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> - <h1 class="text-2xl font-serif font-semibold text-gray-900">{{ selectedFoundation()?.name }} Overview</h1> - </div> + @if (selectedFoundation()) { + <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> + <h1 class="text-2xl font-serif font-semibold text-gray-900">{{ selectedFoundation()?.name }} Overview</h1> + </div> + } @else { + <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> + <h1 class="text-2xl font-serif font-semibold text-gray-900">No Foundation Selected</h1> + </div> + }apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.html (1)
5-8: Consider handling the no-foundation-selected state.Similar to the board member dashboard, this header will display an incomplete label when no foundation is selected.
Add conditional rendering or a placeholder:
- <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> - <h1 class="text-2xl font-serif font-semibold text-gray-900">{{ selectedFoundation()?.name }} Overview</h1> - </div> + @if (selectedFoundation()) { + <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> + <h1 class="text-2xl font-serif font-semibold text-gray-900">{{ selectedFoundation()?.name }} Overview</h1> + </div> + } @else { + <div class="mb-6 flex items-center gap-4" data-testid="foundation-project"> + <h1 class="text-2xl font-serif font-semibold text-gray-900">No Foundation Selected</h1> + </div> + }apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
28-28: Consider adding explicit typing for type safety.The
selectedFoundationsignal is implicitly typed, which may result inSignal<any>based on the ProjectContextService return type. For better type safety and IDE support, consider adding an explicit type annotation.- public readonly selectedFoundation = computed(() => this.projectContextService.selectedFoundation()); + public readonly selectedFoundation: Signal<ProjectContext | null> = computed(() => this.projectContextService.selectedFoundation());Note: Ensure
ProjectContextis imported from the appropriate location if not already available.apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts (1)
4-5: Consider adding explicit typing for type safety.The implementation correctly integrates ProjectContextService, but the
selectedFoundationsignal lacks explicit typing.- public readonly selectedFoundation = computed(() => this.projectContextService.selectedFoundation()); + public readonly selectedFoundation: Signal<ProjectContext | null> = computed(() => this.projectContextService.selectedFoundation());Ensure
ProjectContextis imported if not already available.Also applies to: 19-23
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
31-31: Consider adding explicit typing for type safety.The
selectedFoundationsignal lacks explicit type annotation, resulting inSignal<any>based on inference.- public readonly selectedFoundation = computed(() => this.projectContextService.selectedFoundation()); + public readonly selectedFoundation: Signal<ProjectContext | null> = computed(() => this.projectContextService.selectedFoundation());Ensure
ProjectContextis imported from@lfx-one/shared/interfacesif not already available.apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
153-164: Update service method signature to reflect backend requirement.The verification found that the single consumer (organization-involvement.component.ts:129) correctly passes
accountId. However, the service method signature declares the parameter as optional (accountId?: string), creating a mismatch with the backend's now-required validation. Update the service signature in analytics.service.ts:106 fromaccountId?: stringtoaccountId: stringto enforce the requirement at the type level and prevent future callers from accidentally omitting it.apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (1)
34-36: Remove the console logging from the projects signal
tap(console.log)dumps the full project list for every user hitting the sidebar. That’s noisy and leaks data in production logs. Please drop the debug tap before merging.Apply this diff:
- protected readonly projects = toSignal(this.projectService.getProjects().pipe(tap(console.log)), { + protected readonly projects = toSignal(this.projectService.getProjects(), {
📜 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 (24)
apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html(2 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(3 hunks)apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts(1 hunks)apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts(4 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts(2 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.html(1 hunks)apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts(3 hunks)apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.html(1 hunks)apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.scss(1 hunks)apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts(2 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/project-context.service.ts(1 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(3 hunks)apps/lfx-one/src/server/services/organization.service.ts(4 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
🧬 Code graph analysis (8)
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts (1)
packages/shared/src/interfaces/project.interface.ts (1)
ProjectContext(167-174)
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (3)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
Component(25-134)apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts (1)
Component(11-46)packages/shared/src/interfaces/project.interface.ts (2)
Project(54-103)ProjectContext(167-174)
apps/lfx-one/src/app/shared/services/project-context.service.ts (1)
packages/shared/src/interfaces/project.interface.ts (1)
ProjectContext(167-174)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
BoardMemberDashboardResponse(288-316)OrganizationEventsOverviewResponse(324-339)
apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.ts (2)
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (1)
Component(15-73)packages/shared/src/interfaces/project.interface.ts (1)
Project(54-103)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/interfaces/account.interface.ts (1)
Account(8-13)
apps/lfx-one/src/server/services/organization.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (3)
BoardMemberDashboardResponse(288-316)OrganizationEventsOverviewResponse(324-339)BoardMemberDashboardConsolidatedRow(205-219)
apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
packages/shared/src/interfaces/project.interface.ts (1)
Project(54-103)
⏰ 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 (10)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (1)
34-42: LGTM! Well-structured empty state for foundation selection.The new "No Foundation Selected" state provides clear user guidance with an appropriate icon and instructions. The control flow hierarchy (no foundation → loading → content) is logical and user-friendly.
apps/lfx-one/src/app/shared/components/project-selector/project-selector.component.scss (1)
1-25: LGTM! Clean styling for the project selector.The use of
::ng-deepis appropriate for styling PrimeNG popover components, and the Tailwind utility classes maintain consistency with the design system.apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (2)
318-318: LGTM! Enables cross-project meeting creation.The fallback logic
formValue.selectedProjectUid || project.uidproperly supports both the new cross-project meeting creation flow and maintains backward compatibility with the existing single-project workflow.
667-667: LGTM! Form control properly initialized.The
selectedProjectUidform control is correctly initialized as an optional field (empty string default) to support the project selection feature in the meeting creation flow.apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.html (1)
5-20: LGTM! Well-implemented project selector in meeting creation flow.The conditional rendering, clear labeling, and optional selection pattern (
[showClear]="true") align well with the user experience goals. The placement before meeting type selection is logical.apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html (1)
13-18: LGTM! Clean integration of project selector into sidebar.The conditional rendering, input/output binding, and visual separation are well-implemented. The positioning at the top of the sidebar is intuitive for users.
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (1)
5-8: LGTM! Consistent foundation context display.The Foundation Project header uses proper optional chaining and maintains consistency with other dashboards in the PR. The data-testid facilitates testing.
apps/lfx-one/src/app/layouts/main-layout/main-layout.component.html (1)
7-7: LGTM! Consistent project selector integration.The
showProjectSelectorinput is properly wired to both desktop and mobile sidebars, enabling the foundation project selector UI across the application.Also applies to: 26-26
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
227-241: LGTM! Consistent required parameter validation across all consumers.The required
accountIdvalidation follows the same pattern as other updated endpoints. All identified consumers already pass theaccountIdparameter:
- Frontend service layer (
analytics.service.ts) requiresaccountIdas a parameter- Component (
organization-involvement.component.ts) properly suppliesaccountIdfromselectedAccountId$- Route (
analytics.route.ts) correctly binds to the controller methodNo breaking changes detected.
187-208: Add error handling for 400 responses in organization-involvement.component.tsThe endpoint migration to require both
accountIdandprojectSlugis complete and all consumers are using the new signature. However, the component calls the service without error handling—when the endpoint returns a 400 error for missing/invalid parameters, the request fails silently.Update the observable pipe at line 178 to include a
catchErrorhandler to gracefully handle 400 responses:return this.analyticsService.getBoardMemberDashboard(accountId, foundationSlug).pipe( catchError(() => of(/* return empty/default state */)), finalize(() => this.dashboardLoading.set(false)) );
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
Show resolved
Hide resolved
...dules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
Show resolved
Hide resolved
- Replace plain JSON error responses with ServiceValidationError for consistent error handling - Make required parameters non-optional in analytics service methods - Eliminate unnecessary API call in meeting-type-selection when no current project LFXV2-723 Signed-off-by: Asitha de Silva <asithade@gmail.com>
Summary
This PR implements a comprehensive foundation project selector system with hierarchical context management across dashboards and meetings, along with analytics API refactoring and database schema updates.
Related JIRA Tickets
Changes Overview
1. Foundation Project Selector (LFXV2-723)
ProjectSelectorComponentwith PrimeNG dropdown UIProjectContextServicewith separateselectedFoundationsignalselectedProjectsignal functionalityFiles:
apps/lfx-one/src/app/shared/components/project-selector/(NEW)apps/lfx-one/src/app/shared/components/sidebar/apps/lfx-one/src/app/layouts/main-layout/apps/lfx-one/src/app/shared/services/project-context.service.ts2. Analytics API Refactoring (LFXV2-724)
accountIdandprojectSlugnow required in board member dashboard APIPROJECT_SLUGinstead ofPROJECT_IDfor joinsCURRENT_MEMBERSHIP_START_DATE→START_DATE, etc.DEV_ADESILVA_PLATINUM_LFX_ONEFiles:
apps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/app/shared/services/analytics.service.tspackages/shared/src/interfaces/analytics-data.interface.ts3. Dashboard Foundation Context (LFXV2-725)
ProjectContextServiceinto Board Member, Core Developer, and Maintainer dashboardsdata-testid="foundation-project"for E2E testingFiles:
apps/lfx-one/src/app/modules/dashboards/board-member/apps/lfx-one/src/app/modules/dashboards/core-developer/apps/lfx-one/src/app/modules/dashboards/maintainer/apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/4. Meetings Child Project Selection (LFXV2-726)
parent_uidtagselectedProjectUidto meeting type selectionFiles:
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/apps/lfx-one/src/app/modules/project/meetings/components/meeting-type-selection/Testing
Breaking Changes
Analytics APIs:
accountIdANDprojectSlug(previously optional)accountId(was optional)boardMeetingAttendancefrom board member dashboard responseprojectIdfrom events overview responseMigration Required:
boardMeetingAttendanceprojectIdin events responsesCommit Details
24 files changed, 440 insertions(+), 132 deletions(-)
Generated with Claude Code