feat(committees): add BFF endpoints for meetings and surveys#341
Closed
manishdixitlfx wants to merge 2 commits intomainfrom
Closed
feat(committees): add BFF endpoints for meetings and surveys#341manishdixitlfx wants to merge 2 commits intomainfrom
manishdixitlfx wants to merge 2 commits intomainfrom
Conversation
github-actions
bot
requested review from
andrest50,
asithade,
dealako,
jordane and
mauriciozanettisalomao
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughAdded endpoints and service methods for fetching committee meetings and surveys; refactored frontend upcoming-committee-meeting component to use CommitteeService; introduced controller subResourceHandler helper; added server-side survey/committee meeting service logic (including a lazy MeetingService import); removed committee-application interface and its re-export. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant CommitCtrl as CommitteeController
participant CommitSvc as CommitteeService
participant MeetSvc as MeetingService
participant Backend as BackendAPI
Client->>CommitCtrl: GET /api/committees/:id/meetings
CommitCtrl->>CommitCtrl: validate committeeId
CommitCtrl->>CommitSvc: getCommitteeMeetings(req, id, query)
CommitSvc->>CommitSvc: whitelist + add committee_uid
CommitSvc->>MeetSvc: getMeetings(augmented query)
MeetSvc->>Backend: HTTP GET /meetings
Backend-->>MeetSvc: PaginatedResponse<Meeting>
MeetSvc-->>CommitSvc: PaginatedResponse<Meeting>
CommitSvc-->>CommitCtrl: PaginatedResponse<Meeting>
CommitCtrl-->>Client: JSON response
sequenceDiagram
actor Client
participant CommitCtrl as CommitteeController
participant SurveySvc as SurveyService
participant Proxy as MicroserviceProxy
participant External as ExternalService
Client->>CommitCtrl: GET /api/committees/:id/surveys
CommitCtrl->>CommitCtrl: validate committeeId
CommitCtrl->>SurveySvc: getCommitteeSurveys(req, id, query)
SurveySvc->>SurveySvc: whitelist query + build type=survey & tags=committee_uid:<id>
SurveySvc->>Proxy: proxyRequest /query/resources
Proxy->>External: forward request
External-->>Proxy: resources[]
Proxy-->>SurveySvc: resources[]
SurveySvc->>SurveySvc: map resources -> Survey[]
SurveySvc-->>CommitCtrl: Survey[]
CommitCtrl-->>Client: JSON response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
This was referenced Mar 19, 2026
6 tasks
9 tasks
- Add subResourceHandler factory for standardized committee sub-resource endpoints - Add GET /api/committees/:id/meetings endpoint with query param whitelisting - Add GET /api/committees/:id/surveys endpoint with tag-based filtering - Add getCommitteeMeetings to backend CommitteeService (lazy MeetingService import) - Add getCommitteeSurveys to backend SurveyService with sanitized query params - Add getCommitteeMeetings FE method to CommitteeService - Add getSurveysByCommittee FE method to SurveyService - Switch upcoming-committee-meeting to use CommitteeService.getCommitteeMeetings - Remove unused CommitteeJoinApplication interface - Remove mailing_list/chat_channel PATCH from updateCommittee (not supported upstream) LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
force-pushed
the
feat/LFXV2-committees-bff-endpoints
branch
from
975d5a9 to
a438738
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/lfx-one/src/app/shared/services/survey.service.ts (1)
38-40: Consider adding error handling for consistency.Unlike other methods in this service (
getSurveys,getSurvey), this method has nocatchErrorhandling. While propagating errors is acceptable (per project learnings, frontend HTTP errors are visible in devtools), the inconsistency could be confusing. If the consuming component doesn't handle errors, this could result in unhandled observable errors.♻️ Optional: Add catchError for consistency
public getSurveysByCommittee(committeeUid: string): Observable<Survey[]> { - return this.http.get<Survey[]>(`/api/committees/${committeeUid}/surveys`); + return this.http.get<Survey[]>(`/api/committees/${committeeUid}/surveys`).pipe( + catchError((error) => { + console.error('Failed to load committee surveys:', error); + return of([]); + }) + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/shared/services/survey.service.ts` around lines 38 - 40, The getSurveysByCommittee method lacks the same catchError handling used by getSurveys and getSurvey; update getSurveysByCommittee to pipe the HTTP Observable and add catchError which logs the error (using the same logger used elsewhere in this service) and rethrows it (use throwError(() => err)) so behavior is consistent and consumers still receive the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/lfx-one/src/server/controllers/committee.controller.ts`:
- Around line 596-602: The logging currently sets [countKey] based on
Array.isArray(result) which fails for PaginatedResponse objects (e.g.,
getCommitteeMeetings returns PaginatedResponse<Meeting>), causing meeting_count
to be a boolean; update the handler around serviceFn invocation so
logger.success checks for a PaginatedResponse shape (e.g., result &&
Array.isArray(result.data)) and uses result.data.length when present, otherwise
falls back to Array.isArray(result) ? result.length : !!result; keep references
to serviceFn, countKey, logger.success, and getCommitteeMeetings when making the
change.
- Around line 603-605: The catch block in subResourceHandler currently calls
logger.error(req, operation, startTime, error, { committee_id: committeeId })
and then next(error), causing duplicate logs since the Express error middleware
also logs errors; remove the manual logger.error call from the
subResourceHandler catch block so only next(error) is called (mirroring the
pattern used in getCommitteeSurveys), leaving error handling to the centralized
middleware.
---
Nitpick comments:
In `@apps/lfx-one/src/app/shared/services/survey.service.ts`:
- Around line 38-40: The getSurveysByCommittee method lacks the same catchError
handling used by getSurveys and getSurvey; update getSurveysByCommittee to pipe
the HTTP Observable and add catchError which logs the error (using the same
logger used elsewhere in this service) and rethrows it (use throwError(() =>
err)) so behavior is consistent and consumers still receive the error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b56d16cf-e4aa-4836-86b7-1725f233aef2
📒 Files selected for processing (9)
apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-one/src/app/shared/services/committee.service.tsapps/lfx-one/src/app/shared/services/survey.service.tsapps/lfx-one/src/server/controllers/committee.controller.tsapps/lfx-one/src/server/routes/committees.route.tsapps/lfx-one/src/server/services/committee.service.tsapps/lfx-one/src/server/services/survey.service.tspackages/shared/src/interfaces/committee-application.interface.tspackages/shared/src/interfaces/index.ts
💤 Files with no reviewable changes (2)
- packages/shared/src/interfaces/index.ts
- packages/shared/src/interfaces/committee-application.interface.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Adds committee-scoped BFF endpoints for meetings and surveys, plus corresponding frontend service plumbing, to support committee detail views and upcoming-meeting rendering.
Changes:
- Add
GET /api/committees/:id/meetingsandGET /api/committees/:id/surveysendpoints, including a shared sub-resource handler pattern. - Add FE service methods for committee meetings/surveys and update the upcoming committee meeting component to use the new committee meetings endpoint.
- Remove the unused
CommitteeJoinApplicationshared interface export/file.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/index.ts | Stops exporting the removed committee application interface. |
| packages/shared/src/interfaces/committee-application.interface.ts | Removes unused CommitteeJoinApplication types. |
| apps/lfx-one/src/server/services/survey.service.ts | Adds service method to query surveys by committee tag with param whitelisting. |
| apps/lfx-one/src/server/services/committee.service.ts | Adds committee meetings fetch with lazy MeetingService import; adjusts updateCommittee flow. |
| apps/lfx-one/src/server/routes/committees.route.ts | Wires new /meetings and /surveys committee sub-resource routes. |
| apps/lfx-one/src/server/controllers/committee.controller.ts | Adds meetings handler via factory + manual surveys handler + subResourceHandler helper. |
| apps/lfx-one/src/app/shared/services/survey.service.ts | Adds getSurveysByCommittee() client method. |
| apps/lfx-one/src/app/shared/services/committee.service.ts | Adds getCommitteeMeetings() client method for the new endpoint. |
| apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts | Switches upcoming meeting lookup to use committee meetings API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const allowedParams = ['page_size', 'page_token', 'order_by', 'committee_uid']; | ||
| const sanitizedQuery: Record<string, string> = {}; | ||
| for (const key of allowedParams) { | ||
| if (query[key]) sanitizedQuery[key] = String(query[key]); |
| }); | ||
| res.json(result); | ||
| } catch (error) { | ||
| logger.error(req, operation, startTime, error, { committee_id: committeeId }); |
Comment on lines
+539
to
+540
| * Manual handler (not using subResourceHandler) because this endpoint needs req.query passthrough for survey filtering | ||
| */ |
| const allowedParams = ['page_size', 'page_token', 'order_by', 'status']; | ||
| const sanitizedQuery: Record<string, string> = {}; | ||
| for (const key of allowedParams) { | ||
| if (query[key]) sanitizedQuery[key] = String(query[key]); |
Comment on lines
149
to
154
| // Extract settings fields from core committee data | ||
| const { business_email_required, is_audit_enabled, show_meeting_attendees, member_visibility, ...committeeData } = data; | ||
|
|
||
| const hasSettingsUpdate = | ||
| business_email_required !== undefined || is_audit_enabled !== undefined || show_meeting_attendees !== undefined || member_visibility !== undefined; | ||
| const hasChannelsUpdate = mailing_list !== undefined || chat_channel !== undefined; | ||
| const hasCoreUpdate = Object.keys(committeeData).length > 0; |
| const result = await serviceFn(req, committeeId); | ||
| logger.success(req, operation, startTime, { | ||
| committee_id: committeeId, | ||
| [countKey]: Array.isArray(result) ? result.length : !!result, |
|
|
||
| const startTime = logger.startOperation(req, 'get_committee_surveys', { | ||
| committee_id: id, | ||
| query_params: logger.sanitize(req.query as Record<string, any>), |
Comment on lines
+100
to
+124
| // Whitelist allowed query params to prevent unexpected parameters from reaching downstream | ||
| const allowedParams = ['page_size', 'page_token', 'order_by', 'status']; | ||
| const sanitizedQuery: Record<string, string> = {}; | ||
| for (const key of allowedParams) { | ||
| if (query[key]) sanitizedQuery[key] = String(query[key]); | ||
| } | ||
|
|
||
| // Use tags parameter for server-side filtering — committee_uid is not a supported query param | ||
| const params = { | ||
| ...sanitizedQuery, | ||
| type: 'survey', | ||
| tags: `committee_uid:${committeeId}`, | ||
| }; | ||
|
|
||
| const { resources } = await this.microserviceProxy.proxyRequest<QueryServiceResponse<Survey>>(req, 'LFX_V2_SERVICE', '/query/resources', 'GET', params); | ||
|
|
||
| const surveys: Survey[] = (resources ?? []).map((resource) => resource.data); | ||
|
|
||
| logger.debug(req, 'get_committee_surveys', 'Completed committee survey fetch', { | ||
| committee_uid: committeeId, | ||
| count: surveys.length, | ||
| }); | ||
|
|
||
| return surveys; | ||
| } |
| } | ||
|
|
||
| public getSurveysByCommittee(committeeUid: string): Observable<Survey[]> { | ||
| return this.http.get<Survey[]>(`/api/committees/${committeeUid}/surveys`); |
Comment on lines
+90
to
+91
| public getCommitteeMeetings(committeeId: string): Observable<Meeting[]> { | ||
| return this.http.get<PaginatedResponse<Meeting>>(`/api/committees/${committeeId}/meetings`).pipe(map((response) => response.data)); |
- Fix subResourceHandler countKey to handle PaginatedResponse shapes (was logging boolean instead of count for meetings endpoint) - Remove duplicate logger.error from subResourceHandler catch (error middleware handles it) - Fix query param whitelisting to use !== undefined (was dropping falsy values) - Strip mailing_list/chat_channel from updateCommittee PUT payload (upstream rejects these fields) - Reset cached meetingServicePromise on failure (was caching rejected promise forever) - Fix misleading comment on getCommitteeSurveys handler - Fix Record<string, any> → Record<string, unknown> on logger.sanitize call - Add catchError to FE getSurveysByCommittee for consistency LFXV2-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/lfx-one/src/server/controllers/committee.controller.ts (2)
603-605:⚠️ Potential issue | 🟡 MinorAvoid duplicate error logging in
subResourceHandler.Line 604 logs the error and Line 605 forwards it; this commonly duplicates logs when centralized error middleware also logs failures.
🔧 Suggested fix
} catch (error) { - logger.error(req, operation, startTime, error, { committee_id: committeeId }); next(error); }Based on learnings: In controllers under
apps/lfx-one/src/server/controllers, error logging is handled by Express middleware vianext(error), and explicitlogger.errorin catch blocks should not be required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/server/controllers/committee.controller.ts` around lines 603 - 605, In the catch block of subResourceHandler (and similar controller handlers in controllers under apps/lfx-one/src/server/controllers) remove the explicit logger.error(req, operation, startTime, error, { committee_id: committeeId }) call and only call next(error) so centralized Express error middleware handles logging; locate the logger.error usage in the catch within the function named subResourceHandler (and any other controller catch blocks using logger.error followed by next(error)) and delete the logger.error invocation, leaving next(error) intact.
598-601:⚠️ Potential issue | 🟡 Minor
countKeylogging can collapse to boolean for paginated results.Line 600 logs
!!resultfor non-array payloads, which turns counts intotrue/falseinstead of item totals when a service returns an object payload.🔧 Suggested fix
try { const result = await serviceFn(req, committeeId); + const count = Array.isArray(result) + ? result.length + : result && + typeof result === 'object' && + 'data' in (result as Record<string, unknown>) && + Array.isArray((result as { data: unknown[] }).data) + ? (result as { data: unknown[] }).data.length + : !!result; logger.success(req, operation, startTime, { committee_id: committeeId, - [countKey]: Array.isArray(result) ? result.length : !!result, + [countKey]: count, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/server/controllers/committee.controller.ts` around lines 598 - 601, The logger.success call is collapsing non-array paginated payloads into a boolean via "!!result"; update the count calculation around the logger.success call (where countKey is used) to detect common paginated shapes: if result is an array use result.length; else if result.data is an array use result.data.length; else if result.total or result.count is a number use that; otherwise fall back to 0 or !!result as a last resort. Apply this logic where countKey is computed (in the controller method that calls logger.success) so logged counts reflect the actual item totals for objects returned by services.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts`:
- Around line 40-54: upcomingMeeting currently reads this.committeeId() once so
it never reacts to later changes; fix by deriving upcomingMeeting from the
committeeId signal via toObservable(this.committeeId) and switchMap: when
committeeId emits null return of(null), otherwise switchMap to
committeeService.getCommitteeMeetings(committeeId) and keep the existing
map/filter/sort logic to pick upcoming[0]; then wrap that observable with
toSignal(..., { initialValue: null }) so upcomingMeeting updates whenever
committeeId changes. Reference: upcomingMeeting, committeeId,
committeeService.getCommitteeMeetings.
---
Duplicate comments:
In `@apps/lfx-one/src/server/controllers/committee.controller.ts`:
- Around line 603-605: In the catch block of subResourceHandler (and similar
controller handlers in controllers under apps/lfx-one/src/server/controllers)
remove the explicit logger.error(req, operation, startTime, error, {
committee_id: committeeId }) call and only call next(error) so centralized
Express error middleware handles logging; locate the logger.error usage in the
catch within the function named subResourceHandler (and any other controller
catch blocks using logger.error followed by next(error)) and delete the
logger.error invocation, leaving next(error) intact.
- Around line 598-601: The logger.success call is collapsing non-array paginated
payloads into a boolean via "!!result"; update the count calculation around the
logger.success call (where countKey is used) to detect common paginated shapes:
if result is an array use result.length; else if result.data is an array use
result.data.length; else if result.total or result.count is a number use that;
otherwise fall back to 0 or !!result as a last resort. Apply this logic where
countKey is computed (in the controller method that calls logger.success) so
logged counts reflect the actual item totals for objects returned by services.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e9bbd27-ae2a-4328-85cb-8602f80f2bef
📒 Files selected for processing (9)
apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsapps/lfx-one/src/app/shared/services/committee.service.tsapps/lfx-one/src/app/shared/services/survey.service.tsapps/lfx-one/src/server/controllers/committee.controller.tsapps/lfx-one/src/server/routes/committees.route.tsapps/lfx-one/src/server/services/committee.service.tsapps/lfx-one/src/server/services/survey.service.tspackages/shared/src/interfaces/committee-application.interface.tspackages/shared/src/interfaces/index.ts
💤 Files with no reviewable changes (2)
- packages/shared/src/interfaces/index.ts
- packages/shared/src/interfaces/committee-application.interface.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/lfx-one/src/server/routes/committees.route.ts
- apps/lfx-one/src/app/shared/services/survey.service.ts
- apps/lfx-one/src/app/shared/services/committee.service.ts
- apps/lfx-one/src/server/services/committee.service.ts
Comment on lines
+40
to
+54
| const committeeId = this.committeeId(); | ||
| if (!committeeId) return toSignal(of(null), { initialValue: null }); | ||
|
|
||
| return toSignal( | ||
| this.committeeService.getCommitteeMeetings(committeeId).pipe( | ||
| map((meetings: Meeting[]) => { | ||
| const now = new Date().getTime(); | ||
| const upcoming = meetings | ||
| .filter((m) => m.start_time && new Date(m.start_time).getTime() > now) | ||
| .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime()); | ||
| return upcoming[0] ?? null; | ||
| }) | ||
| ), | ||
| { initialValue: null } | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether committeeId is bound in places where it can change after init.
# Expectation: if bindings are async/late, current one-time read is risky.
rg -n --type=html --type=ts -C2 '<lfx-upcoming-committee-meeting|\[committeeId\]|committeeId='Repository: linuxfoundation/lfx-v2-ui
Length of output: 1563
🏁 Script executed:
cat -n apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts | head -80Repository: linuxfoundation/lfx-v2-ui
Length of output: 2882
🏁 Script executed:
# Also check the component class definition and input signals
rg -n "input\(" apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts -A 2Repository: linuxfoundation/lfx-v2-ui
Length of output: 51
🏁 Script executed:
# Check how upcomingMeeting signal is declared and used
rg -n "upcomingMeeting|initializeUpcomingMeeting" apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 289
🏁 Script executed:
# Check the file size first
wc -l apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.tsRepository: linuxfoundation/lfx-v2-ui
Length of output: 189
Make upcomingMeeting reactive to committeeId updates.
The input signal committeeId is read only once at line 40 during ngOnInit(). If committeeId is null initially and updated later by the parent binding, the upcomingMeeting signal will not re-initialize and the fetch will never trigger. This breaks reactivity when the input changes after component initialization.
Use toObservable() with switchMap() to make the signal properly reactive:
Suggested fix
-import { toSignal } from '@angular/core/rxjs-interop';
+import { toObservable, toSignal } from '@angular/core/rxjs-interop';
...
-import { map, of } from 'rxjs';
+import { map, of, switchMap } from 'rxjs';
...
private initializeUpcomingMeeting(): Signal<Meeting | null> {
- const committeeId = this.committeeId();
- if (!committeeId) return toSignal(of(null), { initialValue: null });
-
- return toSignal(
- this.committeeService.getCommitteeMeetings(committeeId).pipe(
- map((meetings: Meeting[]) => {
- const now = new Date().getTime();
- const upcoming = meetings
- .filter((m) => m.start_time && new Date(m.start_time).getTime() > now)
- .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime());
- return upcoming[0] ?? null;
- })
- ),
- { initialValue: null }
- );
+ return toSignal(
+ toObservable(this.committeeId).pipe(
+ switchMap((committeeId) => {
+ if (!committeeId) return of<Meeting | null>(null);
+ return this.committeeService.getCommitteeMeetings(committeeId).pipe(
+ map((meetings: Meeting[]) => {
+ const now = Date.now();
+ const upcoming = meetings
+ .filter((m) => m.start_time && new Date(m.start_time).getTime() > now)
+ .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime());
+ return upcoming[0] ?? null;
+ })
+ );
+ })
+ ),
+ { initialValue: null }
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const committeeId = this.committeeId(); | |
| if (!committeeId) return toSignal(of(null), { initialValue: null }); | |
| return toSignal( | |
| this.committeeService.getCommitteeMeetings(committeeId).pipe( | |
| map((meetings: Meeting[]) => { | |
| const now = new Date().getTime(); | |
| const upcoming = meetings | |
| .filter((m) => m.start_time && new Date(m.start_time).getTime() > now) | |
| .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime()); | |
| return upcoming[0] ?? null; | |
| }) | |
| ), | |
| { initialValue: null } | |
| ); | |
| return toSignal( | |
| toObservable(this.committeeId).pipe( | |
| switchMap((committeeId) => { | |
| if (!committeeId) return of<Meeting | null>(null); | |
| return this.committeeService.getCommitteeMeetings(committeeId).pipe( | |
| map((meetings: Meeting[]) => { | |
| const now = Date.now(); | |
| const upcoming = meetings | |
| .filter((m) => m.start_time && new Date(m.start_time).getTime() > now) | |
| .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime()); | |
| return upcoming[0] ?? null; | |
| }) | |
| ); | |
| }) | |
| ), | |
| { initialValue: null } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/lfx-one/src/app/modules/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts`
around lines 40 - 54, upcomingMeeting currently reads this.committeeId() once so
it never reacts to later changes; fix by deriving upcomingMeeting from the
committeeId signal via toObservable(this.committeeId) and switchMap: when
committeeId emits null return of(null), otherwise switchMap to
committeeService.getCommitteeMeetings(committeeId) and keep the existing
map/filter/sort logic to pick upcoming[0]; then wrap that observable with
toSignal(..., { initialValue: null }) so upcomingMeeting updates whenever
committeeId changes. Reference: upcomingMeeting, committeeId,
committeeService.getCommitteeMeetings.
Contributor
Author
|
Deferring this PR — the current effort focuses on filling in the members tab and polishing the committee view. Votes, surveys, and meetings tab content will come in a follow-up. The BFF endpoints aren't needed until those tabs are wired up. |
asithade
requested changes
Mar 19, 2026
|
|
||
| const params = { | ||
| ...sanitizedQuery, | ||
| committee_uid: committeeId, |
Contributor
There was a problem hiding this comment.
committee_uid is not a valid upstream parameter for meetings.
Problem: The params object includes committee_uid: committeeId, but the upstream meeting service does not accept committee_uid as a query parameter. The meeting service lists meetings by project_uid only — committee association is embedded within the meeting object's committees array. This param is either silently ignored (returning unfiltered results) or could cause an error.
Why it's a problem: If ignored, the endpoint returns all project meetings, not just committee-scoped ones — the frontend would show meetings from other committees. If the query service is used instead, it supports tags filtering like the survey endpoint does.
Fix: Verify which upstream service this actually hits. If it goes through the query service, use tags: \committee_uid:${committeeId}`for server-side filtering (matching thegetCommitteeSurveyspattern in this same PR). If it goes through the meeting service directly, filter client-side by checkingmeeting.committees?.some(c => c.uid === committeeId)`, and document why server-side filtering isn't possible.
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
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
subResourceHandlerfactory for standardized committee sub-resource endpointsGET /api/committees/:id/meetingsendpoint with query param whitelisting and lazyMeetingServiceimport (avoids circular dependency)GET /api/committees/:id/surveysendpoint with tag-based filtering and sanitized query paramsgetCommitteeMeetingsandgetSurveysByCommitteeFE service methodsupcoming-committee-meetingcomponent to useCommitteeService.getCommitteeMeetingsCommitteeJoinApplicationinterfacemailing_list/chat_channelPATCH fromupdateCommitteeNo votes BFF endpoint — the votes-list component uses
VoteServicedirectly, avoiding type mapping issues.Review Fixes Applied
Record<string, string>type safety on query params (notRecord<string, any>)logger.startOperationin surveys handlerlogger.errorin catch blocks (error middleware handles it)Split Context
This is PR 1 of 4 splitting #335 into focused PRs:
Test Plan
GET /api/committees/:id/meetingsreturns paginated meetingsGET /api/committees/:id/surveysreturns surveys filtered by committeeyarn lint && yarn buildpasses ✅LFXV2-1190
🤖 Generated with Claude Code