Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import { toSignal } from '@angular/core/rxjs-interop';
import { RouterLink } from '@angular/router';
import { Meeting } from '@lfx-one/shared/interfaces';
import { MeetingTimePipe } from '@pipes/meeting-time.pipe';
import { MeetingService } from '@services/meeting.service';
import { CommitteeService } from '@services/committee.service';
import { ProjectService } from '@services/project.service';
import { TooltipModule } from 'primeng/tooltip';
import { filter, map, of } from 'rxjs';
import { map, of } from 'rxjs';

@Component({
selector: 'lfx-upcoming-committee-meeting',
imports: [RouterLink, MeetingTimePipe, TooltipModule],
templateUrl: './upcoming-committee-meeting.component.html',
})
export class UpcomingCommitteeMeetingComponent implements OnInit {
private readonly committeeService = inject(CommitteeService);
private readonly projectService = inject(ProjectService);
private readonly meetingService = inject(MeetingService);
private readonly injector = inject(Injector);

public readonly committeeId = input<string | null>(null);
Expand All @@ -37,9 +37,21 @@ export class UpcomingCommitteeMeetingComponent implements OnInit {
}

private initializeUpcomingMeeting(): Signal<Meeting | null> {
return toSignal(this.project() ? this.getNextUpcomingCommitteeMeeting(this.project()!.uid, this.committeeId()) : of(null), {
initialValue: 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 }
);
Comment on lines +40 to +54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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 -80

Repository: 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 2

Repository: 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.ts

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.ts

Repository: 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.

}

private initializeCommittees() {
Expand All @@ -50,36 +62,4 @@ export class UpcomingCommitteeMeetingComponent implements OnInit {
.join(', ') ?? ''
);
}

private getNextUpcomingCommitteeMeeting(uid: string, committeeId: string | null = null) {
return this.meetingService.getMeetingsByProject(uid).pipe(
filter((meetings: Meeting[]) => {
// Return only meetings that have a start time in the future and has a committee value regardless of the committee id
return (
meetings.filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime() && meeting.committees && meeting.committees?.length > 0)
.length > 0
);
}),
map((meetings: Meeting[]) => {
if (meetings.length > 0) {
if (committeeId) {
// Find the earliest upcoming meeting that has the committee id and return it
const committeeMeetings = meetings.filter(
(meeting) =>
new Date(meeting.start_time).getTime() > new Date().getTime() &&
meeting.committees &&
meeting.committees?.length > 0 &&
meeting.committees.some((c) => c.uid === committeeId)
);

return committeeMeetings.sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime())[0];
}

// Return the next upcoming meeting by date in the future
return meetings.filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime())[0];
}
return null;
})
);
}
}
14 changes: 13 additions & 1 deletion apps/lfx-one/src/app/shared/services/committee.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@

import { HttpClient, HttpParams } from '@angular/common/http';
import { inject, Injectable, signal, WritableSignal } from '@angular/core';
import { Committee, CommitteeMember, CreateCommitteeMemberRequest, MyCommittee, QueryServiceCountResponse } from '@lfx-one/shared/interfaces';
import {
Committee,
CommitteeMember,
CreateCommitteeMemberRequest,
Meeting,
MyCommittee,
PaginatedResponse,
QueryServiceCountResponse,
} from '@lfx-one/shared/interfaces';
import { catchError, map, Observable, of, take, tap, throwError } from 'rxjs';

@Injectable({
Expand Down Expand Up @@ -79,6 +87,10 @@ export class CommitteeService {
return this.http.delete<void>(`/api/committees/${committeeId}/members/${memberId}`).pipe(take(1));
}

public getCommitteeMeetings(committeeId: string): Observable<Meeting[]> {
return this.http.get<PaginatedResponse<Meeting>>(`/api/committees/${committeeId}/meetings`).pipe(map((response) => response.data));
Comment on lines +90 to +91
}

// ── Join / Leave Methods ──────────────────────────────────────────────────

/** Self-join an open group */
Expand Down
4 changes: 4 additions & 0 deletions apps/lfx-one/src/app/shared/services/survey.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export class SurveyService {
return this.getSurveys(params);
}

public getSurveysByCommittee(committeeUid: string): Observable<Survey[]> {
return this.http.get<Survey[]>(`/api/committees/${committeeUid}/surveys`);
}

public getSurvey(surveyUid: string, projectId?: string): Observable<Survey> {
let params = new HttpParams();
if (projectId) {
Expand Down
85 changes: 85 additions & 0 deletions apps/lfx-one/src/server/controllers/committee.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,24 @@ import { NextFunction, Request, Response } from 'express';
import { ServiceValidationError } from '../errors';
import { logger } from '../services/logger.service';
import { CommitteeService } from '../services/committee.service';
import { SurveyService } from '../services/survey.service';

/**
* Controller for handling committee HTTP requests
*/
export class CommitteeController {
private committeeService: CommitteeService = new CommitteeService();
// Cross-domain: surveys are accessed via committee context for the surveys tab
private readonly surveyService = new SurveyService();

// ── Dashboard Sub-Resource Handlers (via factory) ─────────────────────────

/** GET /committees/:id/meetings */
public getCommitteeMeetings = this.subResourceHandler(
'get_committee_meetings',
(req, id) => this.committeeService.getCommitteeMeetings(req, id, req.query as Record<string, string>),
'meeting_count'
);

/**
* GET /committees
Expand Down Expand Up @@ -521,4 +533,77 @@ export class CommitteeController {
next(error);
}
}

/**
* GET /committees/:id/surveys
* Manual handler (not using subResourceHandler) because this endpoint needs req.query passthrough for survey filtering
*/
public async getCommitteeSurveys(req: Request, res: Response, next: NextFunction): Promise<void> {
const { id } = req.params;

if (!id) {
next(
ServiceValidationError.forField('id', 'Committee ID is required', {
operation: 'get_committee_surveys',
service: 'committee_controller',
path: req.path,
})
);
return;
}

const startTime = logger.startOperation(req, 'get_committee_surveys', {
committee_id: id,
query_params: logger.sanitize(req.query as Record<string, any>),
});

try {
const surveys = await this.surveyService.getCommitteeSurveys(req, id, req.query as Record<string, string>);

logger.success(req, 'get_committee_surveys', startTime, {
committee_id: id,
survey_count: surveys.length,
});

res.json(surveys);
} catch (error) {
next(error);
}
}

// ── Private helpers ──────────────────────────────────────────────────────

/**
* Factory that produces a standard sub-resource handler.
* Validates the committee ID, starts an operation, calls the service,
* logs success, and delegates errors to Express error middleware.
*/
private subResourceHandler(operation: string, serviceFn: (req: Request, id: string) => Promise<unknown>, countKey: string) {
return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
const committeeId = req.params['id'];
if (!committeeId) {
next(
ServiceValidationError.forField('id', 'Committee ID is required', {
operation,
service: 'committee_controller',
path: req.path,
})
);
return;
}

const startTime = logger.startOperation(req, operation, { committee_id: committeeId });
try {
const result = await serviceFn(req, committeeId);
logger.success(req, operation, startTime, {
committee_id: committeeId,
[countKey]: Array.isArray(result) ? result.length : !!result,
});
res.json(result);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} catch (error) {
logger.error(req, operation, startTime, error, { committee_id: committeeId });
next(error);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
};
}
}
5 changes: 5 additions & 0 deletions apps/lfx-one/src/server/routes/committees.route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ router.post('/:id/members', (req, res, next) => committeeController.createCommit
router.put('/:id/members/:memberId', (req, res, next) => committeeController.updateCommitteeMember(req, res, next));
router.delete('/:id/members/:memberId', (req, res, next) => committeeController.deleteCommitteeMember(req, res, next));

// Meeting routes
router.get('/:id/meetings', (req, res, next) => committeeController.getCommitteeMeetings(req, res, next));

// ── Join / Leave routes ────────────────────────────────────────────────────
router.post('/:id/join', (req, res, next) => committeeController.joinCommittee(req, res, next));
router.delete('/:id/leave', (req, res, next) => committeeController.leaveCommittee(req, res, next));

// Survey routes
router.get('/:id/surveys', (req, res, next) => committeeController.getCommitteeSurveys(req, res, next));
export default router;
82 changes: 54 additions & 28 deletions apps/lfx-one/src/server/services/committee.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
CommitteeSettingsData,
CommitteeUpdateData,
CreateCommitteeMemberRequest,
Meeting,
PaginatedResponse,
MyCommittee,
QueryServiceCountResponse,
QueryServiceResponse,
Expand All @@ -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();
Expand Down Expand Up @@ -138,12 +146,11 @@ export class CommitteeService {
* Updates an existing committee using ETag for concurrency control
*/
public async updateCommittee(req: Request, committeeId: string, data: CommitteeUpdateData): Promise<Committee> {
// Extract settings and channel fields from core committee data
const { business_email_required, is_audit_enabled, show_meeting_attendees, member_visibility, mailing_list, chat_channel, ...committeeData } = data;
// 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;

let updatedCommittee: Committee;
Expand All @@ -166,30 +173,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, {
Expand Down Expand Up @@ -390,6 +374,48 @@ 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]) sanitizedQuery[key] = String(query[key]);
}

const params = {
...sanitizedQuery,
committee_uid: committeeId,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

};

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 {
logger.warning(req, 'get_committee_meetings', 'Failed to fetch committee meetings, returning empty', {
committee_uid: committeeId,
});
return { data: [], page_token: undefined };
}
}

// ── My Committees ─────────────────────────────────────────────────────────

public async getMyCommittees(req: Request, projectUid?: string): Promise<MyCommittee[]> {
Expand Down
Loading
Loading