-
Notifications
You must be signed in to change notification settings - Fork 0
feat(committees): add BFF endpoints for meetings and surveys #341
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ import { | |
| CommitteeSettingsData, | ||
| CommitteeUpdateData, | ||
| CreateCommitteeMemberRequest, | ||
| Meeting, | ||
| PaginatedResponse, | ||
| MyCommittee, | ||
| QueryServiceCountResponse, | ||
| QueryServiceResponse, | ||
|
|
@@ -17,18 +19,24 @@ import { Request } from 'express'; | |
| import { getUsernameFromAuth } from '../utils/auth-helper'; | ||
|
|
||
| import { ResourceNotFoundError } from '../errors'; | ||
| import { logger } from '../services/logger.service'; | ||
| import { logger } from './logger.service'; | ||
| import { AccessCheckService } from './access-check.service'; | ||
| import { ETagService } from './etag.service'; | ||
| import { MicroserviceProxyService } from './microservice-proxy.service'; | ||
|
|
||
| // MeetingService is dynamically imported to avoid circular dependency. | ||
| // Use import('...') type to reference the class without a static import. | ||
| type MeetingServiceType = InstanceType<typeof import('./meeting.service').MeetingService>; | ||
|
|
||
| /** | ||
| * Service for handling committee business logic | ||
| */ | ||
| export class CommitteeService { | ||
| private accessCheckService: AccessCheckService; | ||
| private etagService: ETagService; | ||
| private microserviceProxy: MicroserviceProxyService; | ||
| // Promise-based lazy initializer to avoid concurrent imports creating duplicate instances | ||
| private meetingServicePromise?: Promise<MeetingServiceType>; | ||
|
|
||
| public constructor() { | ||
| this.accessCheckService = new AccessCheckService(); | ||
|
|
@@ -139,11 +147,12 @@ export class CommitteeService { | |
| */ | ||
| public async updateCommittee(req: Request, committeeId: string, data: CommitteeUpdateData): Promise<Committee> { | ||
| // Extract settings and channel fields from core committee data | ||
| // mailing_list/chat_channel are stripped because upstream PUT does not accept them | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const { business_email_required, is_audit_enabled, show_meeting_attendees, member_visibility, mailing_list, chat_channel, ...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; | ||
|
|
||
| let updatedCommittee: Committee; | ||
|
|
@@ -166,30 +175,7 @@ export class CommitteeService { | |
| updatedCommittee = await this.microserviceProxy.proxyRequest<Committee>(req, 'LFX_V2_SERVICE', `/committees/${committeeId}`, 'GET'); | ||
| } | ||
|
|
||
| // Step 3: Update channels via PATCH (mailing_list/chat_channel are not accepted by PUT) | ||
| if (hasChannelsUpdate) { | ||
| try { | ||
| const channelsPayload: Record<string, any> = {}; | ||
| if (mailing_list !== undefined) channelsPayload['mailing_list'] = mailing_list; | ||
| if (chat_channel !== undefined) channelsPayload['chat_channel'] = chat_channel; | ||
|
|
||
| logger.debug(req, 'update_committee_channels', 'Updating committee channels via PATCH', { | ||
| committee_uid: committeeId, | ||
| fields: Object.keys(channelsPayload), | ||
| }); | ||
|
|
||
| const patched = await this.microserviceProxy.proxyRequest<Committee>(req, 'LFX_V2_SERVICE', `/committees/${committeeId}`, 'PATCH', {}, channelsPayload); | ||
|
|
||
| updatedCommittee = { ...updatedCommittee, ...patched }; | ||
| } catch (error) { | ||
| logger.warning(req, 'update_committee_channels', 'PATCH failed for channels, returning current committee data', { | ||
| committee_uid: committeeId, | ||
| error: error instanceof Error ? error.message : 'Unknown error', | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Step 5: Update settings if provided | ||
| // Step 3: Update settings if provided | ||
| if (hasSettingsUpdate) { | ||
| try { | ||
| await this.updateCommitteeSettings(req, committeeId, { | ||
|
|
@@ -390,6 +376,51 @@ export class CommitteeService { | |
| return userMemberships; | ||
| } | ||
|
|
||
| /** | ||
| * Fetches meetings associated with a committee. | ||
| */ | ||
| public async getCommitteeMeetings(req: Request, committeeId: string, query: Record<string, string> = {}): Promise<PaginatedResponse<Meeting>> { | ||
| try { | ||
| // Whitelist allowed query params to prevent unexpected parameters from reaching downstream | ||
| const allowedParams = ['page_size', 'page_token', 'order_by', 'committee_uid']; | ||
| const sanitizedQuery: Record<string, string> = {}; | ||
| for (const key of allowedParams) { | ||
| if (query[key] !== undefined) sanitizedQuery[key] = String(query[key]); | ||
| } | ||
|
|
||
| const params = { | ||
| ...sanitizedQuery, | ||
| committee_uid: committeeId, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Problem: The 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 Fix: Verify which upstream service this actually hits. If it goes through the query service, use |
||
| }; | ||
|
|
||
| logger.debug(req, 'get_committee_meetings', 'Fetching meetings for committee', { | ||
| committee_uid: committeeId, | ||
| }); | ||
|
|
||
| // Lazy import to avoid circular dependency — Promise ensures only one instance is created | ||
| if (!this.meetingServicePromise) { | ||
| this.meetingServicePromise = import('./meeting.service').then((m) => new m.MeetingService()); | ||
| } | ||
| const meetingService = await this.meetingServicePromise; | ||
| const result = await meetingService.getMeetings(req, params); | ||
|
|
||
| logger.debug(req, 'get_committee_meetings', 'Fetched committee meetings', { | ||
| committee_uid: committeeId, | ||
| count: result.data.length, | ||
| }); | ||
|
|
||
| return result; | ||
| } catch (error) { | ||
| // Reset cached promise so a transient import failure doesn't stick forever | ||
| this.meetingServicePromise = undefined; | ||
| logger.warning(req, 'get_committee_meetings', 'Failed to fetch committee meetings, returning empty', { | ||
| committee_uid: committeeId, | ||
| error: error instanceof Error ? error.message : 'Unknown error', | ||
| }); | ||
| return { data: [], page_token: undefined }; | ||
| } | ||
| } | ||
|
|
||
| // ── My Committees ───────────────────────────────────────────────────────── | ||
|
|
||
| public async getMyCommittees(req: Request, projectUid?: string): Promise<MyCommittee[]> { | ||
|
|
||
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.
🧩 Analysis chain
🏁 Script executed:
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:
Repository: linuxfoundation/lfx-v2-ui
Length of output: 51
🏁 Script executed:
Repository: 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
upcomingMeetingreactive tocommitteeIdupdates.The input signal
committeeIdis read only once at line 40 duringngOnInit(). IfcommitteeIdis null initially and updated later by the parent binding, theupcomingMeetingsignal will not re-initialize and the fetch will never trigger. This breaks reactivity when the input changes after component initialization.Use
toObservable()withswitchMap()to make the signal properly reactive:Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents