feat(dashboards): add pending action surveys api #164
Conversation
Signed-off-by: Asitha de Silva <[email protected]>
- Convert pending actions component to use required input signal - Remove PersonaService dependency for cleaner architecture - Update all dashboards (core-developer, maintainer, board-member) to pass actions as inputs - Add backend endpoint GET /api/projects/pending-action-surveys - Implement Snowflake query for pending survey responses - Add data transformation from survey rows to PendingActionItem format - Support clickable survey links with buttonLink property - Add empty state display when no pending actions available - Use toSignal pattern for reactive data fetching in board member dashboard 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. WalkthroughAdds end-to-end pending‑action surveys: server route and service to fetch pending surveys, client ProjectService call, new signals/inputs on dashboards to bind pending actions, refactors PendingActionsComponent to accept a required input, and several UI/template adjustments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as BoardMemberDashboard
participant ClientService as ProjectService (client)
participant API as Projects API
participant ServerService as ProjectService (server)
participant DB as Database
User->>Dashboard: open dashboard / select project
Dashboard->>Dashboard: update selectedProject signal
Dashboard->>ClientService: request pending actions (projectSlug)
ClientService->>API: GET /api/projects/pending-action-surveys?projectSlug=X
API->>ServerService: projectController.getPendingActionSurveys(req)
ServerService->>DB: query pending surveys for user+project
DB-->>ServerService: rows
ServerService-->>API: [PendingActionItem...]
API-->>ClientService: HTTP 200 [PendingActionItem...]
ClientService-->>Dashboard: observable emits items
Dashboard->>Dashboard: boardMemberActions signal updated
Dashboard-->>User: render pending actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the pending actions component to use input signals and adds backend support for fetching pending survey responses from a Snowflake analytics database. The changes enable the board member dashboard to display real-time survey actions while maintaining static action items for other dashboard personas.
Key Changes
- Converted pending actions component from computed persona-based logic to input signals for better data flow control
- Added new GET
/api/projects/pending-action-surveysendpoint that queries Snowflake for pending board member surveys - Implemented reactive data fetching in board member dashboard using RxJS patterns (toSignal/toObservable)
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/project.interface.ts | Added PendingSurveyRow interface for Snowflake query results with survey metadata |
| packages/shared/src/interfaces/components.interface.ts | Extended PendingActionItem with optional buttonLink field for external survey links |
| apps/lfx-one/src/server/services/project.service.ts | Implemented getPendingActionSurveys method to query and transform survey data |
| apps/lfx-one/src/server/routes/projects.route.ts | Added route for pending-action-surveys endpoint |
| apps/lfx-one/src/server/controllers/project.controller.ts | Added controller method with OIDC email and projectSlug validation |
| apps/lfx-one/src/app/shared/services/project.service.ts | Added frontend service method to call the API endpoint |
| apps/lfx-one/src/app/modules/dashboards/components/pending-actions/* | Refactored to accept pendingActions as required input signal; added empty state and buttonLink support |
| apps/lfx-one/src/app/modules/dashboards/board-member/* | Integrated reactive survey data fetching with error handling |
| apps/lfx-one/src/app/modules/dashboards/maintainer/* | Updated to pass static MAINTAINER_ACTION_ITEMS via signal |
| apps/lfx-one/src/app/modules/dashboards/core-developer/* | Updated to pass static CORE_DEVELOPER_ACTION_ITEMS via signal |
| apps/lfx-one/src/app/shared/components/meeting-rsvp-details/* | Updated progress bar logic to show attended vs not attended for past meetings |
| apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.ts | Changed summary modal to non-modal, non-closable, non-dismissable |
| apps/lfx-one/src/app/shared/components/header/header.component.ts | Removed unused imports (PersonaSelectorComponent, AutocompleteComponent) |
| .github/workflows/docker-build-tag.yml | Changed AWS IAM role ARN to different account |
💡 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.ts
Outdated
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
Outdated
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/lfx-one/src/app/shared/components/meeting-rsvp-details/meeting-rsvp-details.component.html (1)
51-51: Consider extracting width calculations into component methods.The width calculation
(count / meetingRegistrantCount()) * 100is repeated five times across different bar segments. This duplicates logic and reduces maintainability.For improved clarity and DRY, extract width calculations into component methods:
// In component class: getAttendedWidth(): number { return (this.attendedCount() / this.meetingRegistrantCount()) * 100; } getNotAttendedWidth(): number { return ((this.meetingRegistrantCount() - this.attendedCount()) / this.meetingRegistrantCount()) * 100; } getAcceptedWidth(): number { return (this.acceptedCount() / this.meetingRegistrantCount()) * 100; } getMaybeWidth(): number { return (this.maybeCount() / this.meetingRegistrantCount()) * 100; } getDeclinedWidth(): number { return (this.declinedCount() / this.meetingRegistrantCount()) * 100; }Then update template:
- @if (attendedCount() > 0) { - <div class="h-[5.25px] transition-all duration-300 bg-blue-500" [style.width.%]="(attendedCount() / meetingRegistrantCount()) * 100"></div> + @if (attendedCount() > 0) { + <div class="h-[5.25px] transition-all duration-300 bg-blue-500" [style.width.%]="getAttendedWidth()"></div>This pattern also provides a single place to add null-safety guards if needed.
Also applies to: 56-56, 61-61, 64-64, 67-67
apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.ts (1)
252-265: Confirm UX/close behavior after making SummaryModal non‑modal and non‑closableYou’ve changed the dynamic dialog config for
SummaryModalComponentto:
modal: falseclosable: falsedismissableMask: falseThis removes the PrimeNG‑level close affordances (no X button, no mask click, and potentially no focus trap), and the dialog no longer blocks interaction with the background. Closing now depends entirely on
SummaryModalComponentexposing its own “Close/Cancel/Save” control that callsref.close(...).Please verify both:
SummaryModalComponenthas a clear, keyboard‑accessible way to close the dialog wired toDynamicDialogRef.close.- The switch to non‑modal behavior is intentional from a UX/accessibility perspective, given this component is reused from multiple dashboards.
If the main goal was just to avoid accidental outside‑click closes while keeping it modal, consider instead keeping
modal: true,closable: trueand only settingdismissableMask: false(and optionallycloseOnEscape: false) to preserve a conventional modal with explicit close controls.apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html (1)
7-95: Pending actions template correctly handles input, link behavior, and empty stateThe refactor to drive everything from the
pendingActions()input, including the@if/@forblocks, color‑based styling, and the link vs. button split withtarget="_blank"/rel="noopener noreferrer", looks solid and matches the new API shape. The empty state block with a dashed border and test id is also in line with the “my‑meetings” pattern. Only minor nit (optional):track item.textmay misbehave if texts are duplicated; if that becomes an issue, consider tracking by a more stable key when one becomes available.
📜 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 (17)
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(4 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.ts(1 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(2 hunks)apps/lfx-one/src/app/shared/components/header/header.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/meeting-rsvp-details/meeting-rsvp-details.component.html(1 hunks)apps/lfx-one/src/app/shared/services/project.service.ts(2 hunks)apps/lfx-one/src/server/controllers/project.controller.ts(1 hunks)apps/lfx-one/src/server/routes/projects.route.ts(1 hunks)apps/lfx-one/src/server/services/project.service.ts(2 hunks)packages/shared/src/interfaces/components.interface.ts(1 hunks)packages/shared/src/interfaces/project.interface.ts(1 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/shared/components/meeting-rsvp-details/meeting-rsvp-details.component.htmlapps/lfx-one/src/app/shared/components/meeting-card/meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts
🧬 Code graph analysis (7)
apps/lfx-one/src/app/shared/services/project.service.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(344-359)
apps/lfx-one/src/server/controllers/project.controller.ts (1)
apps/lfx-one/src/server/errors/index.ts (1)
ServiceValidationError(7-7)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
packages/shared/src/constants/action-items.constants.ts (1)
MAINTAINER_ACTION_ITEMS(39-64)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(344-359)
apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts (1)
packages/shared/src/constants/action-items.constants.ts (1)
CORE_DEVELOPER_ACTION_ITEMS(9-34)
apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.ts (4)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
Component(21-86)apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts (1)
Component(13-25)apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
Component(13-25)packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(344-359)
apps/lfx-one/src/server/services/project.service.ts (2)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(344-359)packages/shared/src/interfaces/project.interface.ts (1)
PendingSurveyRow(180-225)
🔇 Additional comments (18)
apps/lfx-one/src/app/shared/components/meeting-rsvp-details/meeting-rsvp-details.component.html (1)
48-69: Refactored progress bar logic looks correct.The restructured conditional for past vs. upcoming meetings is clear and aligns well with the summary text above. Each bar segment correctly guards against empty renders, and the width calculations are consistent across all states.
apps/lfx-one/src/app/shared/components/header/header.component.ts (1)
25-25: Good dependency cleanup; template verified clean.Removing AutocompleteComponent and PersonaSelectorComponent is safe. Template verification confirms no references to the removed components, and the component uses PrimeNG's autocomplete directly, making the custom wrapper unnecessary.
packages/shared/src/interfaces/project.interface.ts (1)
176-225: LGTM!The
PendingSurveyRowinterface is well-documented and comprehensive, covering all necessary fields for survey data from the analytics database. The uppercase naming convention appropriately reflects database column naming.packages/shared/src/interfaces/components.interface.ts (1)
357-359: LGTM!The optional
buttonLinkfield is a clean, non-breaking addition to thePendingActionIteminterface that enables clickable action buttons as described in the PR objectives.apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.html (1)
20-20: LGTM!The binding correctly references the
coreDevActions()signal defined in the TypeScript component (line 24 of core-developer-dashboard.component.ts), maintaining consistency with the reactive pattern applied across other dashboards.apps/lfx-one/src/app/modules/dashboards/core-developer/core-developer-dashboard.component.ts (2)
4-6: LGTM!The imports are correct and align with the new signal-based reactive pattern for providing pending actions to the dashboard.
24-24: LGTM!The
coreDevActionssignal is properly initialized with the constant data. This pattern is consistent with the maintainer dashboard (static data from constants) while differing from the board-member dashboard which fetches data from the API.apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (1)
20-20: No issues found—implementation is correct.The verification confirms:
maintainerActionssignal exists at line 24 of maintainer-dashboard.component.ts and is properly initialized withMAINTAINER_ACTION_ITEMSMAINTAINER_ACTION_ITEMSconstant is properly exported in packages/shared/src/constants/action-items.constants.ts with correct type annotation (PendingActionItem[])- Import statement is correct and references the proper export path
The code is ready as is.
apps/lfx-one/src/server/routes/projects.route.ts (1)
21-22: Controller method verified and OIDC authentication properly implemented.Verification confirms the
getPendingActionSurveyscontroller method exists and correctly extracts user email from OIDC context with proper error handling. The method validates thatreq.oidc?.user?.['email']is present (line 448) and returns aServiceValidationErrorif missing. The route's access toreq.oidcconfirms OIDC authentication middleware is configured at the application level.apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html (1)
40-40: No issues found — signal implementation is properly structured.The
boardMemberActionssignal is correctly implemented with:
- Proper reactive pattern using
toObservable(),switchMap(), andtoSignal()- Appropriate data fetching via
projectService.getPendingActionSurveys()- Comprehensive error handling with logging and fallback to empty array
- Safe initialization with
initialValue: []- Correct null handling for missing project selection
apps/lfx-one/src/app/shared/services/project.service.ts (1)
56-65: Backend endpoint implementation verified and properly authenticated.The verification confirms that the backend endpoint is fully implemented and properly protected. The route exists at
projects.route.ts:21and the controller method atproject.controller.ts:443validates OIDC authentication by requiringreq.oidc?.user?.['email']before processing requests. The endpoint is mounted in the protected API routes section with appropriate error handling for missing authentication context or parameters.apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
4-7: Maintainer actions signal wiring looks consistent and type‑safeUsing
signal(MAINTAINER_ACTION_ITEMS)to exposemaintainerActionsmirrors the core‑developer dashboard pattern and cleanly feeds static data intoPendingActionsComponent. No issues spotted with imports or typing here.Also applies to: 23-25
apps/lfx-one/src/server/controllers/project.controller.ts (1)
440-491: Controller endpoint for pending survey actions is well‑structuredValidation of OIDC email and
projectSlug, use ofServiceValidationError.forField(...), and delegation toprojectService.getPendingActionSurveysall align with existing controller patterns and the stated API contract. Logging includes useful context (email, slug, survey_count), and errors are correctly routed throughnext.apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.ts (1)
5-6: Input/output refactor cleanly decouples pending actions from internal persona logicSwitching to
pendingActions = input.required<PendingActionItem[]>()andactionClick = output<PendingActionItem>()nicely turns this into a pure presentational component that relies entirely on its callers for data, while preserving the actionClick output semantics. The imports and standaloneimports: [CommonModule]list are consistent with this lean surface. Just ensure all dashboard templates now bind[pendingActions]so the required input is always satisfied.Also applies to: 12-13, 18-23
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (4)
5-5: LGTM! Imports properly support reactive pattern.The added imports (toObservable, toSignal, RxJS operators, ProjectService, and PendingActionItem interface) are all necessary for the reactive data fetching implementation.
Also applies to: 8-9, 15-15
30-30: LGTM! Service injection and computed signals properly structured.The ProjectService injection and computed signals follow Angular best practices. The fallback logic in
selectedProjectappropriately handles both project and foundation contexts.Also applies to: 39-40
45-59: LGTM! Constructor properly initializes reactive data flow.The constructor cleanly separates account selection handling from pending actions initialization, with clear comments documenting each responsibility.
61-85: Implementation verified—no issues found.The frontend service correctly abstracts the backend complexity. While the backend service requires both
projectSlugparameters, the frontend service signature properly exposes onlyprojectSlug. The email is correctly extracted from the OIDC context in the backend controller (req.oidc?.user?.['email']) before calling the service. The component's call togetPendingActionSurveys(project.slug)correctly matches the frontend service signature. This follows proper layered architecture.
apps/lfx-one/src/app/shared/components/meeting-rsvp-details/meeting-rsvp-details.component.html
Show resolved
Hide resolved
Signed-off-by: Asitha de Silva <[email protected]>
- Add CommonJS dependencies to allowedCommonJsDependencies list - Increase component style budget from 4kB to 6kB for production and staging - Eliminates CommonJS optimization bailout warnings - Accommodates FullCalendar component styles (5.41 KB) Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html (1)
69-77: Add accessibility indicator for external links.External links opening in a new tab should include an accessibility indicator to inform users. Consider adding an
aria-labelattribute that mentions the link opens in a new tab, or include a visual indicator (icon or text).Apply this diff to add an aria-label:
<a [href]="item.buttonLink" target="_blank" rel="noopener noreferrer" class="w-full h-8 rounded-md px-3 text-sm font-medium border border-gray-300 bg-white text-gray-700 hover:bg-slate-100 transition-colors flex items-center justify-center" - data-testid="dashboard-pending-actions-button"> + data-testid="dashboard-pending-actions-button" + [attr.aria-label]="item.buttonText + ' (opens in new tab)'"> {{ item.buttonText }} </a>
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
74-77: Consider using a logging service instead of console.error.Direct console.error calls may not be appropriate for production environments where structured logging, log aggregation, or log levels are important.
If a logging service exists in the codebase, apply this pattern:
return this.projectService.getPendingActionSurveys(project.slug).pipe( catchError((error) => { - console.error('Failed to fetch survey actions:', error); + this.logger.error('Failed to fetch survey actions', { error, projectSlug: project.slug }); // Return empty array on error return of([]); })
📜 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 (2)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(4 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(344-359)
🔇 Additional comments (1)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
60-84: Reactive pattern implementation looks solid.The implementation correctly uses Angular's signal-observable interop to create a reactive data flow that automatically updates when the selected project changes. The use of
switchMapprevents race conditions, and error handling gracefully degrades to an empty state.Consider whether silent error handling is the desired UX. Currently, errors are logged but users see an empty state with no indication that a fetch failed. If this is intentional to avoid error noise, the current implementation is fine. Otherwise, consider adding user-facing error feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
apps/lfx-one/angular.json(3 hunks)
🔇 Additional comments (1)
apps/lfx-one/angular.json (1)
80-80: Budget increase is justified—FullCalendar component styles require 5.41 KB, exceeding the original 4kB limit.The
anyComponentStylebudget increase (4kB → 6kB) is documented and necessary. The PR integrates the FullCalendar library, whose compiled styles are 5.41 kB—above the previous threshold. The new 6kB limit accommodates this with a reasonable buffer. Style optimization concerns do not apply to external library CSS; this is a required tradeoff for new functionality. The commit message (LFXV2-764) explicitly documents this justification.
Summary
Refactored pending actions component to accept data via input signals and integrated backend API for querying pending survey responses from Snowflake analytics database.
Frontend Changes
Backend Changes
Interfaces
JIRA: LFXV2-764