diff --git a/CLAUDE.md b/CLAUDE.md index a925fc2f..ecaa9d39 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,3 +122,5 @@ lfx-pcc-v3/ - All commits and pull requests need to be associated to a JIRA ticket. If there isn't one, we need to create it and reference it moving forward. - Branch names should be following the commit types (feat,fix,docs, etc) followed by the JIRA ticket number. i.e; feat/LFXV2-123 or ci/LFXV2-456 - PR titles must also follow a similar format as conventional commits - `type(scope): description`. The scope has to follow the angular config for conventional commit and not include the JIRA ticket in the title, and everything should be in lowercase. + +- All interfaces, resuable constants, and enums should live in the shared package. diff --git a/apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts b/apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts index e419ee9c..c520e4ae 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts @@ -74,7 +74,7 @@ export class CommitteeDashboardComponent { // Statistics calculations public totalCommittees: Signal = computed(() => this.committees().length); - public publicCommittees: Signal = computed(() => this.committees().filter((c) => c.public_enabled).length); + public publicCommittees: Signal = computed(() => this.committees().filter((c) => c.public).length); public activeVoting: Signal = computed(() => this.committees().filter((c) => c.enable_voting).length); public constructor() { @@ -162,7 +162,7 @@ export class CommitteeDashboardComponent { const committee = this.selectedCommittee(); const projectId = this.project()?.uid; if (committee && projectId) { - this.router.navigate(['/project', this.project()?.slug, 'committees', committee.id]); + this.router.navigate(['/project', this.project()?.slug, 'committees', committee.uid]); } } @@ -188,7 +188,7 @@ export class CommitteeDashboardComponent { private performDelete(committee: Committee): void { this.isDeleting.set(true); - this.committeeService.deleteCommittee(committee.id).subscribe({ + this.committeeService.deleteCommittee(committee.uid).subscribe({ next: () => { this.isDeleting.set(false); // Refresh the committees list by reloading @@ -219,7 +219,7 @@ export class CommitteeDashboardComponent { data: { isEditing: true, committee: committee, - committeeId: committee.id, + committeeId: committee.uid, onCancel: () => this.dialogRef?.close(), }, }); diff --git a/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html b/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html index 9255b139..b215c05e 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html +++ b/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html @@ -25,7 +25,7 @@

Committee Not Found

} - @if (committee()?.id && !loading() && !error()) { + @if (committee()?.uid && !loading() && !error()) {
@@ -61,7 +61,7 @@

{{ committee()?.name }}

- @if (committee()?.id) { + @if (committee()?.uid) { {{ committee()?.name }}
- @if (committee()?.id) { - + @if (committee()?.uid) { + } @@ -99,19 +99,19 @@

Description

{{ committee()?.total_voting_reps || 0 }}
- @if (committee()?.committee_website) { + @if (committee()?.website) {
Website - - {{ committee()?.committee_website }} + + {{ committee()?.website }}
} - @if (committee()?.public_name) { + @if (committee()?.display_name) {
Public Name - {{ committee()?.public_name }} + {{ committee()?.display_name }}
} @@ -132,12 +132,12 @@

Description

- - {{ committee()?.public_enabled ? 'Enabled' : 'Disabled' }} + [class.fa-check-circle]="committee()?.public" + [class.text-green-500]="committee()?.public" + [class.fa-times-circle]="!committee()?.public" + [class.text-red-500]="!committee()?.public"> + + {{ committee()?.public ? 'Enabled' : 'Disabled' }}
diff --git a/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts b/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts index 1756a19f..f447adda 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts @@ -113,7 +113,7 @@ export class CommitteeViewComponent { private performDelete(committee: Committee): void { this.isDeleting.set(true); - this.committeeService.deleteCommittee(committee.id).subscribe({ + this.committeeService.deleteCommittee(committee.uid).subscribe({ next: () => { this.isDeleting.set(false); this.goBack(); @@ -138,7 +138,7 @@ export class CommitteeViewComponent { data: { isEditing: true, committee: committee, - committeeId: committee.id, + committeeId: committee.uid, onCancel: () => this.dialogRef?.close(), }, }); diff --git a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html index 257ed9f1..f0852bcc 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html +++ b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html @@ -107,19 +107,19 @@

Public Settings

-
+
@@ -156,14 +156,12 @@

Additional Information

-

- Please enter a valid URL -

+

Please enter a valid URL

diff --git a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts index e61d2e06..6350f664 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts @@ -62,7 +62,16 @@ export class CommitteeFormComponent { if (this.form().valid) { const isEditingMode = this.isEditing(); - const formValue = this.form().value; + const rawFormValue = { + ...this.form().value, + calendar: { + public: this.form().value.public || false, + }, + display_name: this.form().value.display_name || this.form().value.name, + website: this.form().value.website || null, + public: true, + }; + const formValue = this.cleanFormData(rawFormValue); const committeeId = this.committeeId(); this.submitting.set(true); @@ -132,6 +141,23 @@ export class CommitteeFormComponent { }); } + // Helper method to clean form data - convert empty strings to null + private cleanFormData(formData: any): any { + const cleaned: any = {}; + + Object.keys(formData).forEach((key) => { + const value = formData[key]; + // Convert empty strings to null for optional string fields + if (typeof value === 'string' && value.trim() === '') { + cleaned[key] = null; + } else { + cleaned[key] = value; + } + }); + + return cleaned; + } + // Success handler private onSuccess(): void { const isEditing = this.isEditing(); @@ -190,12 +216,12 @@ export class CommitteeFormComponent { // If editing, exclude the current committee const currentCommitteeId = this.committee()?.id; - const availableCommittees = currentCommitteeId ? topLevelCommittees.filter((committee) => committee.id !== currentCommitteeId) : topLevelCommittees; + const availableCommittees = currentCommitteeId ? topLevelCommittees.filter((committee) => committee.uid !== currentCommitteeId) : topLevelCommittees; // Transform to dropdown options const options = availableCommittees.map((committee) => ({ label: committee.name, - value: committee.id, + value: committee.uid, })); // Add "No Parent Committee" option at the beginning @@ -215,11 +241,11 @@ export class CommitteeFormComponent { business_email_required: new FormControl(committee?.business_email_required || false), enable_voting: new FormControl(committee?.enable_voting || false), is_audit_enabled: new FormControl(committee?.is_audit_enabled || false), - public_enabled: new FormControl(committee?.public_enabled || false), - public_name: new FormControl(committee?.public_name || ''), + public: new FormControl(committee?.public || false), + display_name: new FormControl(committee?.display_name || ''), sso_group_enabled: new FormControl(committee?.sso_group_enabled || false), sso_group_name: new FormControl(committee?.sso_group_name || ''), - committee_website: new FormControl(committee?.committee_website || '', [Validators.pattern(/^https?:\/\/.+\..+/)]), + website: new FormControl(committee?.website || '', [Validators.pattern(/^https?:\/\/.+\..+/)]), project_uid: new FormControl(committee?.project_uid || ''), joinable: new FormControl(committee?.joinable || false), }); diff --git a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-members/committee-members.component.ts b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-members/committee-members.component.ts index 077fc0da..428eb239 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-members/committee-members.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-members/committee-members.component.ts @@ -196,7 +196,7 @@ export class CommitteeMembersComponent implements OnInit { private performDelete(member: CommitteeMember): void { const committee = this.committee(); - if (!committee || !committee.id) { + if (!committee || !committee.uid) { this.messageService.add({ severity: 'error', summary: 'Error', @@ -207,7 +207,7 @@ export class CommitteeMembersComponent implements OnInit { this.isDeleting.set(true); - this.committeeService.deleteCommitteeMember(committee.id, member.id).subscribe({ + this.committeeService.deleteCommitteeMember(committee.uid, member.id).subscribe({ next: () => { this.isDeleting.set(false); diff --git a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html index a4968de5..cab4a394 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html +++ b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html @@ -35,7 +35,7 @@ - +
@@ -47,7 +47,7 @@ }
{{ committee.name }} @@ -121,7 +121,7 @@ severity="secondary" styleClass="h-8 w-8 p-0 text-gray-500 hover:text-gray-700" (onClick)="onEdit(committee)" - [attr.data-testid]="'committee-edit-' + committee.id" /> + [attr.data-testid]="'committee-edit-' + committee.uid" /> + [attr.data-testid]="'committee-members-' + committee.uid" /> + [attr.data-testid]="'committee-more-' + committee.uid" />
diff --git a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts index ea2b2340..8a9cf9e4 100644 --- a/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts @@ -46,14 +46,14 @@ export class CommitteeTableComponent { result.push(parent); // Then add any subcommittees for this parent - const subcommittees = allCommittees.filter((c) => c.parent_uid === parent.id); + const subcommittees = allCommittees.filter((c) => c.parent_uid === parent.uid); subcommittees.forEach((sub) => { result.push({ ...sub, level: 1 }); }); }); // Add any orphaned committees (shouldn't happen, but just in case) - const orphaned = allCommittees.filter((c) => c.parent_uid && !allCommittees.find((p) => p.id === c.parent_uid)); + const orphaned = allCommittees.filter((c) => c.parent_uid && !allCommittees.find((p) => p.uid === c.parent_uid)); result.push(...orphaned); return result; diff --git a/apps/lfx-pcc/src/app/modules/project/dashboard/project-dashboard/project.component.ts b/apps/lfx-pcc/src/app/modules/project/dashboard/project-dashboard/project.component.ts index 39c155aa..fce3cdc3 100644 --- a/apps/lfx-pcc/src/app/modules/project/dashboard/project-dashboard/project.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/dashboard/project-dashboard/project.component.ts @@ -239,9 +239,9 @@ export class ProjectComponent { } return committees.map((committee) => ({ - id: committee.id, + id: committee.uid, title: committee.name, - url: `/project/${project.slug}/committees/${committee.id}`, + url: `/project/${project.slug}/committees/${committee.uid}`, status: 'Active', date: committee.updated_at || committee.created_at, })); diff --git a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html index f7a0c83c..03ce7be8 100644 --- a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html +++ b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html @@ -249,9 +249,9 @@

- @for (committee of meeting().meeting_committees!; track committee.id; let isLast = $last) { + @for (committee of meeting().meeting_committees!; track committee.uid; let isLast = $last) { {{ committee.name }} diff --git a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts index 2356b98f..dca99e49 100644 --- a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts @@ -75,7 +75,7 @@ export class MeetingCommitteeModalComponent { public hasVotingEnabledCommittee = computed(() => { const selectedIds = this.selectedCommitteeIds(); const committees = this.committees(); - return committees.some((c) => selectedIds.includes(c.id) && c.enable_voting); + return committees.some((c) => selectedIds.includes(c.uid) && c.enable_voting); }); public tableColspan = computed(() => { @@ -100,7 +100,7 @@ export class MeetingCommitteeModalComponent { // Set initial selected committees if (this.meeting.meeting_committees && this.meeting.meeting_committees.length > 0) { - const committeeIds = this.meeting.meeting_committees.map((c) => c.id); + const committeeIds = this.meeting.meeting_committees.map((c) => c.uid); this.selectedCommitteeIds.set(committeeIds); this.form.patchValue({ committees: committeeIds }); // Load members for initially selected committees @@ -125,7 +125,7 @@ export class MeetingCommitteeModalComponent { const selectedIds = this.form.value.committees as string[]; // If no changes, just close - const currentIds = this.meeting.meeting_committees?.map((c) => c.id) || []; + const currentIds = this.meeting.meeting_committees?.map((c) => c.uid) || []; if (JSON.stringify(selectedIds.sort()) === JSON.stringify(currentIds.sort())) { this.ref.close(); return; @@ -201,7 +201,7 @@ export class MeetingCommitteeModalComponent { // Load members for committees that haven't been loaded yet const memberRequests = committeesToLoad.map((id) => { - const committee = this.committees().find((c) => c.id === id); + const committee = this.committees().find((c) => c.uid === id); return this.committeeService.getCommitteeMembers(id).pipe( map((members) => { const membersWithCommittee = members.map((member) => ({ @@ -237,7 +237,7 @@ export class MeetingCommitteeModalComponent { for (const committeeId of committeeIds) { const cachedMembers = this.committeesMembersCache.get(committeeId) || []; - const committee = this.committees().find((c) => c.id === committeeId); + const committee = this.committees().find((c) => c.uid === committeeId); const committeeName = committee?.name || ''; for (const member of cachedMembers) { diff --git a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts index 2b1c6eb9..bad0706f 100644 --- a/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts @@ -305,7 +305,7 @@ export class MeetingFormComponent { } // Check file type - if (!ALLOWED_FILE_TYPES.includes(file.type as any)) { + if (!ALLOWED_FILE_TYPES.includes(file.type as (typeof ALLOWED_FILE_TYPES)[number])) { const allowedTypes = ALLOWED_FILE_TYPES.map((type) => type.split('/')[1]).join(', '); return `File type "${file.type}" is not supported. Allowed types: ${allowedTypes}.`; } diff --git a/apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts b/apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts index 04f1771b..701b7238 100644 --- a/apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts @@ -236,7 +236,7 @@ export class MeetingDashboardComponent { // Extract unique committees from all meetings meetings.forEach((meeting) => { meeting.meeting_committees?.forEach((committee) => { - committeeMap.set(committee.id, committee.name); + committeeMap.set(committee.uid, committee.name); }); }); @@ -277,7 +277,7 @@ export class MeetingDashboardComponent { // Apply committee filter const committee = this.committeeFilter(); if (committee) { - filtered = filtered.filter((meeting) => meeting.meeting_committees?.some((c) => c.id === committee)); + filtered = filtered.filter((meeting) => meeting.meeting_committees?.some((c) => c.uid === committee)); } return filtered; diff --git a/apps/lfx-pcc/src/app/modules/project/settings/components/user-form/user-form.component.ts b/apps/lfx-pcc/src/app/modules/project/settings/components/user-form/user-form.component.ts index 9324e45f..32b82a01 100644 --- a/apps/lfx-pcc/src/app/modules/project/settings/components/user-form/user-form.component.ts +++ b/apps/lfx-pcc/src/app/modules/project/settings/components/user-form/user-form.component.ts @@ -157,7 +157,7 @@ export class UserFormComponent { map((committees) => committees.map((committee) => ({ label: committee.name, - value: committee.id, + value: committee.uid, })) ) ), @@ -182,7 +182,7 @@ export class UserFormComponent { } else if (user.committeePermissions?.length > 0) { permissionScope = 'committee'; permissionLevel = user.committeePermissions[0].level; - committeeIds = user.committeePermissions.map((cp) => cp.committee.id); + committeeIds = user.committeePermissions.map((cp) => cp.committee.uid); } this.form().patchValue({ diff --git a/apps/lfx-pcc/src/app/shared/components/table/table.component.ts b/apps/lfx-pcc/src/app/shared/components/table/table.component.ts index 8253b949..11d1f193 100644 --- a/apps/lfx-pcc/src/app/shared/components/table/table.component.ts +++ b/apps/lfx-pcc/src/app/shared/components/table/table.component.ts @@ -10,6 +10,9 @@ import { TableModule } from 'primeng/table'; standalone: true, imports: [CommonModule, TableModule], templateUrl: './table.component.html', + host: { + ngSkipHydration: 'true', + }, }) export class TableComponent { // Template references for content projection diff --git a/apps/lfx-pcc/src/app/shared/services/committee.service.ts b/apps/lfx-pcc/src/app/shared/services/committee.service.ts index cbc99113..aedb0e21 100644 --- a/apps/lfx-pcc/src/app/shared/services/committee.service.ts +++ b/apps/lfx-pcc/src/app/shared/services/committee.service.ts @@ -24,7 +24,7 @@ export class CommitteeService { } public getCommitteesByProject(projectId: string, limit?: number, orderBy?: string): Observable { - let params = new HttpParams().set('project_uid', `eq.${projectId}`); + let params = new HttpParams().set('tags', `project_uid:${projectId}`); if (limit) { params = params.set('limit', limit.toString()); diff --git a/apps/lfx-pcc/src/server/controllers/committee.controller.ts b/apps/lfx-pcc/src/server/controllers/committee.controller.ts new file mode 100644 index 00000000..1a7a6276 --- /dev/null +++ b/apps/lfx-pcc/src/server/controllers/committee.controller.ts @@ -0,0 +1,145 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { Request, Response } from 'express'; +import { CommitteeCreateData, CommitteeUpdateData } from '@lfx-pcc/shared/interfaces'; + +import { CommitteeService } from '../services/committee.service'; +import { MicroserviceProxyService } from '../services/microservice-proxy.service'; +import { Logger } from '../helpers/logger'; +import { Responder } from '../helpers/responder'; + +/** + * Controller for handling committee HTTP requests + */ +export class CommitteeController { + private committeeService: CommitteeService; + + public constructor(private microserviceProxy: MicroserviceProxyService) { + this.committeeService = new CommitteeService(microserviceProxy); + } + + /** + * GET /committees + */ + public async getCommittees(req: Request, res: Response): Promise { + const startTime = Logger.start(req, 'get_committees', { + query_params: Logger.sanitize(req.query as Record), + }); + + try { + const committees = await this.committeeService.getCommittees(req, req.query); + + Logger.success(req, 'get_committees', startTime, { + committee_count: committees.length, + }); + + res.json(committees); + } catch (error) { + Logger.error(req, 'get_committees', startTime, error); + Responder.handle(res, error, 'get_committees'); + } + } + + /** + * GET /committees/:id + */ + public async getCommitteeById(req: Request, res: Response): Promise { + const { id } = req.params; + const startTime = Logger.start(req, 'get_committee_by_id', { + committee_id: id, + }); + + try { + const committee = await this.committeeService.getCommitteeById(req, id); + + Logger.success(req, 'get_committee_by_id', startTime, { + committee_id: id, + committee_category: committee.category, + }); + + res.json(committee); + } catch (error) { + Logger.error(req, 'get_committee_by_id', startTime, error, { + committee_id: id, + }); + Responder.handle(res, error, 'get_committee_by_id'); + } + } + + /** + * POST /committees + */ + public async createCommittee(req: Request, res: Response): Promise { + const startTime = Logger.start(req, 'create_committee', { + committee_data: Logger.sanitize(req.body), + }); + + try { + const committeeData: CommitteeCreateData = req.body; + const newCommittee = await this.committeeService.createCommittee(req, committeeData); + + Logger.success(req, 'create_committee', startTime, { + committee_id: newCommittee.uid, + committee_category: newCommittee.category, + }); + + res.status(201).json(newCommittee); + } catch (error) { + Logger.error(req, 'create_committee', startTime, error); + Responder.handle(res, error, 'create_committee'); + } + } + + /** + * PUT /committees/:id + */ + public async updateCommittee(req: Request, res: Response): Promise { + const { id } = req.params; + const startTime = Logger.start(req, 'update_committee', { + committee_id: id, + update_data: Logger.sanitize(req.body), + }); + + try { + const updateData: CommitteeUpdateData = req.body; + const updatedCommittee = await this.committeeService.updateCommittee(req, id, updateData); + + Logger.success(req, 'update_committee', startTime, { + committee_id: id, + }); + + res.json(updatedCommittee); + } catch (error) { + Logger.error(req, 'update_committee', startTime, error, { + committee_id: id, + }); + Responder.handle(res, error, 'update_committee'); + } + } + + /** + * DELETE /committees/:id + */ + public async deleteCommittee(req: Request, res: Response): Promise { + const { id } = req.params; + const startTime = Logger.start(req, 'delete_committee', { + committee_id: id, + }); + + try { + await this.committeeService.deleteCommittee(req, id); + + Logger.success(req, 'delete_committee', startTime, { + committee_id: id, + }); + + res.status(204).send(); + } catch (error) { + Logger.error(req, 'delete_committee', startTime, error, { + committee_id: id, + }); + Responder.handle(res, error, 'delete_committee'); + } + } +} diff --git a/apps/lfx-pcc/src/server/helpers/logger.ts b/apps/lfx-pcc/src/server/helpers/logger.ts new file mode 100644 index 00000000..8cb11e09 --- /dev/null +++ b/apps/lfx-pcc/src/server/helpers/logger.ts @@ -0,0 +1,135 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { Request } from 'express'; +import { SENSITIVE_FIELDS } from '@lfx-pcc/shared/constants'; +import { extractErrorDetails, isValidationApiError } from '@lfx-pcc/shared/interfaces'; + +/** + * Standardized request logging helper for consistent log formatting + */ +export class Logger { + /** + * Logs the start of an operation with timing + */ + public static start(req: Request, operation: string, metadata: Record = {}): number { + const startTime = Date.now(); + + req.log.info( + { + operation, + ...metadata, + request_id: req.id, + user_agent: req.get('User-Agent'), + ip_address: req.ip, + }, + `Starting ${operation.replace('_', ' ')}` + ); + + return startTime; + } + + /** + * Logs successful completion of an operation + */ + public static success(req: Request, operation: string, startTime: number, metadata: Record = {}): void { + const duration = Date.now() - startTime; + + req.log.info( + { + operation, + duration, + status_code: 200, + ...metadata, + request_id: req.id, + }, + `Successfully completed ${operation.replace('_', ' ')}` + ); + } + + /** + * Logs operation failure with error details + */ + public static error(req: Request, operation: string, startTime: number, error: unknown, metadata: Record = {}): void { + const duration = Date.now() - startTime; + const errorDetails = extractErrorDetails(error); + + req.log.error( + { + operation, + duration, + error: errorDetails.message, + stack: error instanceof Error ? error.stack : undefined, + status_code: errorDetails.statusCode, + error_code: errorDetails.code, + validation_errors: isValidationApiError(error) ? error.validationErrors : undefined, + ...metadata, + request_id: req.id, + }, + `Failed to ${operation.replace('_', ' ')}` + ); + } + + /** + * Logs validation errors specifically + */ + public static validation(req: Request, operation: string, validationErrors: any[], metadata: Record = {}): void { + req.log.warn( + { + operation, + validation_errors: validationErrors, + status_code: 400, + ...metadata, + request_id: req.id, + }, + `Validation failed for ${operation.replace('_', ' ')}` + ); + } + + /** + * Logs ETag-related operations + */ + public static etag(req: Request, operation: string, resourceType: string, resourceId: string, etag?: string, metadata: Record = {}): void { + req.log.info( + { + operation, + resource_type: resourceType, + resource_id: resourceId, + etag, + ...metadata, + request_id: req.id, + }, + `ETag operation: ${operation.replace('_', ' ')}` + ); + } + + /** + * Logs warning messages with operation context + */ + public static warning(req: Request, operation: string, message: string, metadata: Record = {}): void { + req.log.warn( + { + operation, + warning_message: message, + ...metadata, + request_id: req.id, + }, + `Warning during ${operation.replace('_', ' ')}: ${message}` + ); + } + + /** + * Sanitizes sensitive data from metadata before logging + */ + public static sanitize(metadata: Record): Record { + const sanitized = { ...metadata }; + + Object.keys(sanitized).forEach((key) => { + if (SENSITIVE_FIELDS.some((field) => key.toLowerCase().includes(field))) { + sanitized[key] = '[REDACTED]'; + } + }); + + return sanitized; + } +} diff --git a/apps/lfx-pcc/src/server/helpers/responder.ts b/apps/lfx-pcc/src/server/helpers/responder.ts new file mode 100644 index 00000000..5e9ec652 --- /dev/null +++ b/apps/lfx-pcc/src/server/helpers/responder.ts @@ -0,0 +1,149 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { Response } from 'express'; +import { ApiErrorResponse, ValidationError, PaginationInfo, extractErrorDetails, isValidationApiError } from '@lfx-pcc/shared/interfaces'; + +/** + * Response helper for consistent API responses + */ +export class Responder { + /** + * Sends an error response with proper formatting + */ + public static error( + res: Response, + message: string, + options: { + statusCode?: number; + code?: string; + errors?: ValidationError[]; + details?: Record; + } = {} + ): void { + const { statusCode = 500, code, errors, details } = options; + + const response: ApiErrorResponse = { + error: message, + ...(code && { code }), + ...(errors && { errors }), + ...(details && { details }), + }; + + res.status(statusCode).json(response); + } + + /** + * Sends a bad request error (400) + */ + public static badRequest(res: Response, message = 'Bad request', details?: Record): void { + this.error(res, message, { statusCode: 400, code: 'BAD_REQUEST', details }); + } + + /** + * Sends an unauthorized error (401) + */ + public static unauthorized(res: Response, message = 'Unauthorized'): void { + this.error(res, message, { statusCode: 401, code: 'UNAUTHORIZED' }); + } + + /** + * Sends a forbidden error (403) + */ + public static forbidden(res: Response, message = 'Forbidden'): void { + this.error(res, message, { statusCode: 403, code: 'FORBIDDEN' }); + } + + /** + * Sends a not found error (404) + */ + public static notFound(res: Response, message = 'Resource not found'): void { + this.error(res, message, { statusCode: 404, code: 'NOT_FOUND' }); + } + + /** + * Sends a conflict error (409) + */ + public static conflict(res: Response, message = 'Resource conflict'): void { + this.error(res, message, { statusCode: 409, code: 'CONFLICT' }); + } + + /** + * Sends a precondition failed error (412) - useful for ETag conflicts + */ + public static preconditionFailed(res: Response, message = 'Resource has been modified. Please refresh and try again.'): void { + this.error(res, message, { statusCode: 412, code: 'PRECONDITION_FAILED' }); + } + + /** + * Sends a validation error (400) with field-specific errors + */ + public static validationError(res: Response, errors: ValidationError[], message = 'Validation failed'): void { + res.status(400).json({ + error: message, + code: 'VALIDATION_ERROR', + errors, + }); + } + + /** + * Sends an internal server error (500) + */ + public static internalError(res: Response, message = 'Internal server error'): void { + this.error(res, message, { statusCode: 500, code: 'INTERNAL_ERROR' }); + } + + /** + * Creates pagination metadata from query parameters and total count + */ + public static createPagination(page: number, limit: number, total: number): PaginationInfo { + const pages = Math.ceil(total / limit); + + return { + page, + limit, + total, + pages, + hasNext: page < pages, + hasPrev: page > 1, + }; + } + + /** + * Handles different types of errors and sends appropriate response + */ + public static handle(res: Response, error: unknown, operation = 'operation'): void { + // Handle validation errors + if (isValidationApiError(error)) { + this.validationError(res, error.validationErrors); + return; + } + + const errorDetails = extractErrorDetails(error); + + // Handle ETag errors + if (errorDetails.code === 'NOT_FOUND') { + this.notFound(res, errorDetails.message); + return; + } + + if (errorDetails.code === 'PRECONDITION_FAILED') { + this.preconditionFailed(res, errorDetails.message); + return; + } + + if (errorDetails.code === 'ETAG_MISSING') { + this.internalError(res, 'ETag header missing from upstream service'); + return; + } + + // Handle HTTP status code errors + const statusCode = errorDetails.statusCode; + const message = errorDetails.message || `Failed to ${operation.replace('_', ' ')}`; + + this.error(res, message, { + statusCode, + code: errorDetails.code || (statusCode >= 500 ? 'INTERNAL_ERROR' : undefined), + }); + } +} diff --git a/apps/lfx-pcc/src/server/routes/committees.ts b/apps/lfx-pcc/src/server/routes/committees.ts index b4a675ba..c10e3656 100644 --- a/apps/lfx-pcc/src/server/routes/committees.ts +++ b/apps/lfx-pcc/src/server/routes/committees.ts @@ -3,281 +3,25 @@ import { NextFunction, Request, Response, Router } from 'express'; +import { ApiClientService } from '../services/api-client.service'; +import { MicroserviceProxyService } from '../services/microservice-proxy.service'; import { SupabaseService } from '../services/supabase.service'; +import { CommitteeController } from '../controllers/committee.controller'; const router = Router(); const supabaseService = new SupabaseService(); +const microserviceProxyService = new MicroserviceProxyService(new ApiClientService()); +const committeeController = new CommitteeController(microserviceProxyService); -router.get('/', async (req: Request, res: Response, next: NextFunction) => { - const startTime = Date.now(); +// Committee CRUD routes - using new controller pattern +router.get('/', (req, res) => committeeController.getCommittees(req, res)); +router.get('/:id', (req, res) => committeeController.getCommitteeById(req, res)); +router.post('/', (req, res) => committeeController.createCommittee(req, res)); +router.put('/:id', (req, res) => committeeController.updateCommittee(req, res)); +router.delete('/:id', (req, res) => committeeController.deleteCommittee(req, res)); - req.log.info( - { - operation: 'fetch_committees', - query_params: req.query, - }, - 'Starting committees fetch request' - ); - - try { - const committees = await supabaseService.getCommittees(req.query as Record); - const duration = Date.now() - startTime; - - req.log.info( - { - operation: 'fetch_committees', - committee_count: committees.length, - duration, - status_code: 200, - }, - 'Successfully fetched committees' - ); - - return res.json(committees); - } catch (error) { - const duration = Date.now() - startTime; - req.log.error( - { - error: error instanceof Error ? error.message : error, - operation: 'fetch_committees', - duration, - query_params: req.query, - }, - 'Failed to fetch committees' - ); - return next(error); - } -}); - -router.get('/:id', async (req: Request, res: Response, next: NextFunction) => { - const startTime = Date.now(); - const committeeId = req.params['id']; - - req.log.info( - { - operation: 'fetch_committee_by_id', - committee_id: committeeId, - }, - 'Starting committee fetch by ID request' - ); - - try { - if (!committeeId) { - req.log.warn( - { - operation: 'fetch_committee_by_id', - error: 'Missing committee ID parameter', - status_code: 400, - }, - 'Bad request: Committee ID validation failed' - ); - - return res.status(400).json({ - error: 'Committee ID is required', - code: 'MISSING_COMMITTEE_ID', - }); - } - - const committee = await supabaseService.getCommitteeById(committeeId); - - if (!committee) { - const duration = Date.now() - startTime; - req.log.warn( - { - operation: 'fetch_committee_by_id', - committee_id: committeeId, - error: 'Committee not found', - duration, - status_code: 404, - }, - 'Committee not found' - ); - - return res.status(404).json({ - error: 'Committee not found', - code: 'COMMITTEE_NOT_FOUND', - }); - } - - const duration = Date.now() - startTime; - req.log.info( - { - operation: 'fetch_committee_by_id', - committee_id: committeeId, - duration, - status_code: 200, - }, - 'Successfully fetched committee' - ); - - return res.json(committee); - } catch (error) { - const duration = Date.now() - startTime; - req.log.error( - { - error: error instanceof Error ? error.message : error, - operation: 'fetch_committee_by_id', - committee_id: committeeId, - duration, - }, - 'Failed to fetch committee' - ); - return next(error); - } -}); - -router.post('/', async (req: Request, res: Response, next: NextFunction) => { - const startTime = Date.now(); - const committeeData = req.body; - - req.log.info( - { - operation: 'create_committee', - committee_category: committeeData?.category, - body_size: JSON.stringify(req.body).length, - }, - 'Starting committee creation request' - ); - - try { - if (!committeeData?.name) { - req.log.warn( - { - operation: 'create_committee', - error: 'Missing committee name', - provided_data: { has_name: !!committeeData?.name, has_category: !!committeeData?.category }, - status_code: 400, - }, - 'Bad request: Committee name validation failed' - ); - - return res.status(400).json({ - error: 'Committee name is required', - code: 'MISSING_COMMITTEE_NAME', - }); - } - - if (!committeeData?.category) { - req.log.warn( - { - operation: 'create_committee', - error: 'Missing committee category', - provided_data: { has_name: !!committeeData?.name, has_category: !!committeeData?.category }, - status_code: 400, - }, - 'Bad request: Committee category validation failed' - ); - - return res.status(400).json({ - error: 'Committee category is required', - code: 'MISSING_COMMITTEE_CATEGORY', - }); - } - - const newCommittee = await supabaseService.createCommittee(committeeData); - const duration = Date.now() - startTime; - - req.log.info( - { - operation: 'create_committee', - committee_id: newCommittee.id, - committee_category: newCommittee.category, - duration, - status_code: 201, - }, - 'Successfully created committee' - ); - - return res.status(201).json(newCommittee); - } catch (error) { - const duration = Date.now() - startTime; - req.log.error( - { - error: error instanceof Error ? error.message : error, - operation: 'create_committee', - committee_category: req.body?.category, - duration, - }, - 'Failed to create committee' - ); - return next(error); - } -}); - -router.put('/:id', async (req: Request, res: Response, next: NextFunction) => { - try { - const committeeId = req.params['id']; - const committeeData = req.body; - - if (!committeeId) { - return res.status(400).json({ - error: 'Committee ID is required', - code: 'MISSING_COMMITTEE_ID', - }); - } - - // Check if committee exists before attempting to update - const existingCommittee = await supabaseService.getCommitteeById(committeeId); - if (!existingCommittee) { - return res.status(404).json({ - error: 'Committee not found', - code: 'COMMITTEE_NOT_FOUND', - }); - } - - const updatedCommittee = await supabaseService.updateCommittee(committeeId, committeeData); - - return res.json(updatedCommittee); - } catch (error) { - req.log.error( - { - error: error instanceof Error ? error.message : error, - committee_id: req.params['id'], - }, - 'Failed to update committee' - ); - return next(error); - } -}); - -router.delete('/:id', async (req: Request, res: Response, next: NextFunction) => { - try { - const committeeId = req.params['id']; - - if (!committeeId) { - return res.status(400).json({ - error: 'Committee ID is required', - code: 'MISSING_COMMITTEE_ID', - }); - } - - // Check if committee exists before attempting to delete - const existingCommittee = await supabaseService.getCommitteeById(committeeId); - if (!existingCommittee) { - return res.status(404).json({ - error: 'Committee not found', - code: 'COMMITTEE_NOT_FOUND', - }); - } - - await supabaseService.deleteCommittee(committeeId); - - return res.status(204).send(); - } catch (error) { - req.log.error( - { - error: error instanceof Error ? error.message : error, - committee_id: req.params['id'], - }, - 'Failed to delete committee' - ); - return next(error); - } -}); - -// Committee Members routes -// GET /api/committees/:id/members +// Committee member routes - still using Supabase service (not migrated to v2 yet) router.get('/:id/members', async (req: Request, res: Response, next: NextFunction) => { try { const committeeId = req.params['id']; @@ -313,7 +57,6 @@ router.get('/:id/members', async (req: Request, res: Response, next: NextFunctio } }); -// GET /api/committees/:id/members/:memberId router.get('/:id/members/:memberId', async (req: Request, res: Response, next: NextFunction) => { try { const committeeId = req.params['id']; @@ -365,7 +108,6 @@ router.get('/:id/members/:memberId', async (req: Request, res: Response, next: N } }); -// POST /api/committees/:id/members router.post('/:id/members', async (req: Request, res: Response, next: NextFunction) => { try { const committeeId = req.params['id']; @@ -417,7 +159,6 @@ router.post('/:id/members', async (req: Request, res: Response, next: NextFuncti } }); -// PUT /api/committees/:id/members/:memberId router.put('/:id/members/:memberId', async (req: Request, res: Response, next: NextFunction) => { try { const committeeId = req.params['id']; @@ -472,7 +213,6 @@ router.put('/:id/members/:memberId', async (req: Request, res: Response, next: N } }); -// DELETE /api/committees/:id/members/:memberId router.delete('/:id/members/:memberId', async (req: Request, res: Response, next: NextFunction) => { try { const committeeId = req.params['id']; diff --git a/apps/lfx-pcc/src/server/routes/meetings.ts b/apps/lfx-pcc/src/server/routes/meetings.ts index feeb22d4..887a9c8f 100644 --- a/apps/lfx-pcc/src/server/routes/meetings.ts +++ b/apps/lfx-pcc/src/server/routes/meetings.ts @@ -1,8 +1,8 @@ // Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT -import { NextFunction, Request, Response, Router } from 'express'; import { ALLOWED_FILE_TYPES, MAX_FILE_SIZE_BYTES, sanitizeFilename } from '@lfx-pcc/shared'; +import { NextFunction, Request, Response, Router } from 'express'; import { SupabaseService } from '../services/supabase.service'; @@ -22,7 +22,7 @@ router.get('/', async (req: Request, res: Response, next: NextFunction) => { ); try { - const meetings = await supabaseService.getMeetings(req.query as Record); + const meetings = await supabaseService.getMeetings(req.query); const duration = Date.now() - startTime; req.log.info( @@ -475,7 +475,7 @@ router.post('/:id/attachments/upload', async (req: Request, res: Response, next: } // Validate file type - if (!ALLOWED_FILE_TYPES.includes(mimeType as any)) { + if (!ALLOWED_FILE_TYPES.includes(mimeType as (typeof ALLOWED_FILE_TYPES)[number])) { req.log.warn( { operation: 'upload_meeting_attachment', @@ -656,7 +656,7 @@ router.post('/storage/upload', async (req: Request, res: Response, next: NextFun } // Validate file type - if (!ALLOWED_FILE_TYPES.includes(mimeType as any)) { + if (!ALLOWED_FILE_TYPES.includes(mimeType as (typeof ALLOWED_FILE_TYPES)[number])) { return res.status(400).json({ error: 'File type not supported', code: 'UNSUPPORTED_FILE_TYPE', diff --git a/apps/lfx-pcc/src/server/services/api-client.service.ts b/apps/lfx-pcc/src/server/services/api-client.service.ts index 85dc3a3d..51541a2c 100644 --- a/apps/lfx-pcc/src/server/services/api-client.service.ts +++ b/apps/lfx-pcc/src/server/services/api-client.service.ts @@ -1,7 +1,7 @@ // Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT -import { ApiClientConfig, ApiResponse } from '@lfx-pcc/shared/interfaces'; +import { ApiClientConfig, ApiResponse, extractErrorDetails } from '@lfx-pcc/shared/interfaces'; import { serverLogger } from '../server'; import { createHttpError, createNetworkError, createTimeoutError } from '../utils/api-error'; @@ -17,33 +17,34 @@ export class ApiClientService { }; } - public async get(url: string, bearerToken: string, params?: Record): Promise> { + public async get(url: string, bearerToken: string, params?: Record, customHeaders?: Record): Promise> { const queryString = params ? new URLSearchParams(params).toString() : ''; const fullUrl = queryString ? `${url}?${queryString}` : url; - return this.makeRequest('GET', fullUrl, bearerToken); + return this.makeRequest('GET', fullUrl, bearerToken, undefined, customHeaders); } - public async post(url: string, bearerToken: string, data?: any): Promise> { - return this.makeRequest('POST', url, bearerToken, data); + public async post(url: string, bearerToken: string, data?: any, customHeaders?: Record): Promise> { + return this.makeRequest('POST', url, bearerToken, data, customHeaders); } - public async put(url: string, bearerToken: string, data?: any): Promise> { - return this.makeRequest('PUT', url, bearerToken, data); + public async put(url: string, bearerToken: string, data?: any, customHeaders?: Record): Promise> { + return this.makeRequest('PUT', url, bearerToken, data, customHeaders); } - public async patch(url: string, bearerToken: string, data?: any): Promise> { - return this.makeRequest('PATCH', url, bearerToken, data); + public async patch(url: string, bearerToken: string, data?: any, customHeaders?: Record): Promise> { + return this.makeRequest('PATCH', url, bearerToken, data, customHeaders); } - public async delete(url: string, bearerToken: string): Promise> { - return this.makeRequest('DELETE', url, bearerToken); + public async delete(url: string, bearerToken: string, customHeaders?: Record): Promise> { + return this.makeRequest('DELETE', url, bearerToken, undefined, customHeaders); } - private async makeRequest(method: string, url: string, bearerToken: string, data?: any): Promise> { + private async makeRequest(method: string, url: string, bearerToken: string, data?: any, customHeaders?: Record): Promise> { const requestInit: RequestInit = { method, headers: { + ...customHeaders, Authorization: `Bearer ${bearerToken}`, Accept: 'application/json', ['Content-Type']: 'application/json', @@ -69,15 +70,16 @@ export class ApiClientService { lastError = error instanceof Error ? error : new Error(String(error)); if (attempt === this.config.retryAttempts) { + const errorDetails = extractErrorDetails(error); serverLogger.error( { url, method: requestInit.method, attempt, max_attempts: this.config.retryAttempts, - error: lastError.message, - error_code: (error as any)?.code, - status: (error as any)?.status, + error: errorDetails.message, + error_code: errorDetails.code, + status: errorDetails.statusCode, }, 'API request failed after all retry attempts' ); @@ -102,14 +104,15 @@ export class ApiClientService { continue; } + const errorDetails = extractErrorDetails(error); serverLogger.error( { url, method: requestInit.method, attempt, - error: lastError.message, - error_code: (error as any)?.code, - status: (error as any)?.status, + error: errorDetails.message, + error_code: errorDetails.code, + status: errorDetails.statusCode, will_retry: false, }, 'API request failed with non-retryable error' @@ -170,29 +173,40 @@ export class ApiClientService { } } - private isRetryableError(error: any): boolean { + private isRetryableError(error: unknown): boolean { + const errorDetails = extractErrorDetails(error); + // Timeout errors - if (error.code === 'TIMEOUT' || error.name === 'AbortError') { + if (errorDetails.code === 'TIMEOUT' || (error instanceof Error && error.name === 'AbortError')) { return true; } - // Network errors - if (error.code === 'ECONNRESET' || error.code === 'ENOTFOUND' || error.code === 'ECONNREFUSED') { - return true; - } + // Network errors - check if error has these specific codes + if (error && typeof error === 'object') { + const errorObj = error as Record; + const errorCode = errorObj['code']; - // Fetch network errors - if (error.cause?.code === 'ECONNRESET' || error.cause?.code === 'ENOTFOUND' || error.cause?.code === 'ECONNREFUSED') { - return true; + if (errorCode === 'ECONNRESET' || errorCode === 'ENOTFOUND' || errorCode === 'ECONNREFUSED') { + return true; + } + + // Fetch network errors + const cause = errorObj['cause'] as Record | undefined; + if (cause && typeof cause === 'object') { + const causeCode = cause['code']; + if (causeCode === 'ECONNRESET' || causeCode === 'ENOTFOUND' || causeCode === 'ECONNREFUSED') { + return true; + } + } } // Server errors (5xx) - if (error.status >= 500 && error.status < 600) { + if (errorDetails.statusCode >= 500 && errorDetails.statusCode < 600) { return true; } // Rate limiting - if (error.status === 429) { + if (errorDetails.statusCode === 429) { return true; } diff --git a/apps/lfx-pcc/src/server/services/committee.service.ts b/apps/lfx-pcc/src/server/services/committee.service.ts new file mode 100644 index 00000000..561dccfc --- /dev/null +++ b/apps/lfx-pcc/src/server/services/committee.service.ts @@ -0,0 +1,392 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { + Committee, + CommitteeCreateData, + CommitteeSettingsData, + CommitteeUpdateData, + CommitteeValidationError, + CommitteeValidationResult, + ETagError, + QueryServiceResponse, + ValidationApiError, +} from '@lfx-pcc/shared/interfaces'; +import { getValidCommitteeCategories } from '@lfx-pcc/shared/constants'; +import { Request } from 'express'; + +import { ETagService } from './etag.service'; +import { MicroserviceProxyService } from './microservice-proxy.service'; +import { createApiError } from '../utils/api-error'; + +/** + * Service for handling committee business logic + */ +export class CommitteeService { + private etagService: ETagService; + + public constructor(private microserviceProxy: MicroserviceProxyService) { + this.etagService = new ETagService(microserviceProxy); + } + + /** + * Fetches all committees based on query parameters + */ + public async getCommittees(req: Request, query: Record = {}): Promise { + const params = { + ...query, + type: 'committee', + }; + + const { resources } = await this.microserviceProxy.proxyRequest>(req, 'LFX_V2_SERVICE', '/query/resources', 'GET', params); + + return resources.map((resource) => resource.data); + } + + /** + * Fetches a single committee by ID + */ + public async getCommitteeById(req: Request, committeeId: string): Promise { + const params = { + type: 'committee', + parent: `committee:${committeeId}`, + }; + + const { resources } = await this.microserviceProxy.proxyRequest>(req, 'LFX_V2_SERVICE', '/query/resources', 'GET', params); + + if (!resources || resources.length === 0) { + const error: ETagError = { + code: 'NOT_FOUND', + message: 'Committee not found', + statusCode: 404, + }; + throw error; + } + + return resources[0].data; + } + + /** + * Creates a new committee with optional settings + */ + public async createCommittee(req: Request, data: CommitteeCreateData): Promise { + // Validate input data + const validation = this.validateCommitteeData(data); + if (!validation.isValid) { + const validationError: ValidationApiError = createApiError({ + message: `Validation failed: ${validation.errors.map((e) => e.message).join(', ')}`, + statusCode: 400, + code: 'VALIDATION_ERROR', + }) as ValidationApiError; + validationError.validationErrors = validation.errors; + throw validationError; + } + + // Extract settings fields + const { business_email_required, is_audit_enabled, ...committeeData } = data; + + // Step 1: Create committee + const newCommittee = await this.microserviceProxy.proxyRequest(req, 'LFX_V2_SERVICE', '/committees', 'POST', {}, committeeData); + + req.log.info( + { + operation: 'create_committee', + committee_id: newCommittee.uid, + committee_category: newCommittee.category, + }, + 'Committee created successfully' + ); + + // Step 2: Update settings if provided + if (business_email_required !== undefined || is_audit_enabled !== undefined) { + try { + await this.updateCommitteeSettings(req, newCommittee.uid, { business_email_required, is_audit_enabled }); + } catch (error) { + req.log.warn( + { + operation: 'create_committee', + committee_id: newCommittee.uid, + error: error instanceof Error ? error.message : error, + }, + 'Failed to update committee settings, but committee was created successfully' + ); + } + } + + return { + ...newCommittee, + ...(business_email_required !== undefined && { business_email_required }), + ...(is_audit_enabled !== undefined && { is_audit_enabled }), + }; + } + + /** + * Updates an existing committee using ETag for concurrency control + */ + public async updateCommittee(req: Request, committeeId: string, data: CommitteeUpdateData): Promise { + // Validate input data + const validation = this.validateCommitteeData(data, true); + if (!validation.isValid) { + const validationError: ValidationApiError = createApiError({ + message: `Validation failed: ${validation.errors.map((e) => e.message).join(', ')}`, + statusCode: 400, + code: 'VALIDATION_ERROR', + }) as ValidationApiError; + validationError.validationErrors = validation.errors; + throw validationError; + } + + // Extract settings fields + const { business_email_required, is_audit_enabled, ...committeeData } = data; + + // Step 1: Fetch committee with ETag + const { etag } = await this.etagService.fetchWithETag(req, 'LFX_V2_SERVICE', `/committees/${committeeId}`, 'update_committee'); + + // Step 2: Update committee with ETag + const updatedCommittee = await this.etagService.updateWithETag( + req, + 'LFX_V2_SERVICE', + `/committees/${committeeId}`, + etag, + committeeData, + 'update_committee' + ); + + req.log.info( + { + operation: 'update_committee', + committee_id: committeeId, + }, + 'Committee updated successfully' + ); + + // Step 3: Update settings if provided + if (business_email_required !== undefined || is_audit_enabled !== undefined) { + try { + await this.updateCommitteeSettings(req, committeeId, { business_email_required, is_audit_enabled }); + } catch (error) { + req.log.warn( + { + operation: 'update_committee', + committee_id: committeeId, + error: error instanceof Error ? error.message : error, + }, + 'Failed to update committee settings, but committee was updated successfully' + ); + } + } + + return { + ...updatedCommittee, + ...(business_email_required !== undefined && { business_email_required }), + ...(is_audit_enabled !== undefined && { is_audit_enabled }), + }; + } + + /** + * Deletes a committee using ETag for concurrency control + */ + public async deleteCommittee(req: Request, committeeId: string): Promise { + // Step 1: Fetch committee with ETag + const { etag } = await this.etagService.fetchWithETag(req, 'LFX_V2_SERVICE', `/committees/${committeeId}`, 'delete_committee'); + + // Step 2: Delete committee with ETag + await this.etagService.deleteWithETag(req, 'LFX_V2_SERVICE', `/committees/${committeeId}`, etag, 'delete_committee'); + + req.log.info( + { + operation: 'delete_committee', + committee_id: committeeId, + }, + 'Committee deleted successfully' + ); + } + + /** + * Updates committee settings (business_email_required, is_audit_enabled) + */ + private async updateCommitteeSettings(req: Request, committeeId: string, settings: CommitteeSettingsData): Promise { + const settingsData = { + ...(settings.business_email_required !== undefined && { + business_email_required: settings.business_email_required, + }), + ...(settings.is_audit_enabled !== undefined && { + is_audit_enabled: settings.is_audit_enabled, + }), + }; + + await this.microserviceProxy.proxyRequest(req, 'LFX_V2_SERVICE', `/committees/${committeeId}/settings`, 'PUT', {}, settingsData); + + req.log.info( + { + operation: 'update_committee_settings', + committee_id: committeeId, + settings_data: settingsData, + }, + 'Committee settings updated successfully' + ); + } + + /** + * Validates committee data for create or update operations + */ + private validateCommitteeData(data: CommitteeCreateData | CommitteeUpdateData, isUpdate = false): CommitteeValidationResult { + const errors: CommitteeValidationError[] = []; + + // Required field validation for create operations + if (!isUpdate) { + if (!data.name || data.name.trim().length === 0) { + errors.push({ + field: 'name', + message: 'Committee name is required', + code: 'REQUIRED_FIELD_MISSING', + }); + } + + if (!data.category || data.category.trim().length === 0) { + errors.push({ + field: 'category', + message: 'Committee category is required', + code: 'REQUIRED_FIELD_MISSING', + }); + } + } + + // Name validation + if (data.name !== undefined) { + if (typeof data.name !== 'string') { + errors.push({ + field: 'name', + message: 'Committee name must be a string', + code: 'INVALID_TYPE', + }); + } else if (data.name.trim().length === 0) { + errors.push({ + field: 'name', + message: 'Committee name cannot be empty', + code: 'INVALID_VALUE', + }); + } else if (data.name.length > 255) { + errors.push({ + field: 'name', + message: 'Committee name cannot exceed 255 characters', + code: 'VALUE_TOO_LONG', + }); + } + } + + // Category validation + if (data.category !== undefined) { + const validCategories = getValidCommitteeCategories(); + if (typeof data.category !== 'string') { + errors.push({ + field: 'category', + message: 'Committee category must be a string', + code: 'INVALID_TYPE', + }); + } else if (!validCategories.includes(data.category)) { + errors.push({ + field: 'category', + message: `Committee category must be one of: ${validCategories.join(', ')}`, + code: 'INVALID_VALUE', + }); + } + } + + // Description validation + if (data.description !== undefined) { + if (typeof data.description !== 'string') { + errors.push({ + field: 'description', + message: 'Committee description must be a string', + code: 'INVALID_TYPE', + }); + } else if (data.description.length > 2000) { + errors.push({ + field: 'description', + message: 'Committee description cannot exceed 2000 characters', + code: 'VALUE_TOO_LONG', + }); + } + } + + // Display name validation + if (data.display_name !== undefined) { + if (typeof data.display_name !== 'string') { + errors.push({ + field: 'display_name', + message: 'Display name must be a string', + code: 'INVALID_TYPE', + }); + } else if (data.display_name.length > 255) { + errors.push({ + field: 'display_name', + message: 'Display name cannot exceed 255 characters', + code: 'VALUE_TOO_LONG', + }); + } + } + + // Website validation + if (data.website !== undefined) { + if (typeof data.website !== 'string') { + errors.push({ + field: 'website', + message: 'Website must be a string', + code: 'INVALID_TYPE', + }); + } else if (data.website.trim().length > 0 && !this.isValidUrl(data.website)) { + errors.push({ + field: 'website', + message: 'Website must be a valid URL', + code: 'INVALID_FORMAT', + }); + } + } + + // SSO group validation + if (data.sso_group_enabled === true && (!data.sso_group_name || data.sso_group_name.trim().length === 0)) { + errors.push({ + field: 'sso_group_name', + message: 'SSO group name is required when SSO group is enabled', + code: 'CONDITIONAL_FIELD_MISSING', + }); + } + + // Boolean field validation + const booleanFields: (keyof CommitteeCreateData)[] = [ + 'enable_voting', + 'public', + 'sso_group_enabled', + 'business_email_required', + 'is_audit_enabled', + 'joinable', + ]; + booleanFields.forEach((field) => { + if (data[field] !== undefined && typeof data[field] !== 'boolean') { + errors.push({ + field, + message: `${field} must be a boolean value`, + code: 'INVALID_TYPE', + }); + } + }); + + return { + isValid: errors.length === 0, + errors, + }; + } + + /** + * Validates if a string is a valid URL + */ + private isValidUrl(url: string): boolean { + try { + new URL(url); + return true; + } catch { + return false; + } + } +} diff --git a/apps/lfx-pcc/src/server/services/etag.service.ts b/apps/lfx-pcc/src/server/services/etag.service.ts new file mode 100644 index 00000000..19d9aac8 --- /dev/null +++ b/apps/lfx-pcc/src/server/services/etag.service.ts @@ -0,0 +1,140 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { ETagError, ETagResult, extractErrorDetails } from '@lfx-pcc/shared/interfaces'; +import { HTTP_HEADERS } from '@lfx-pcc/shared/constants'; +import { Request } from 'express'; + +import { MicroserviceProxyService } from './microservice-proxy.service'; + +/** + * Service for handling ETag-based operations with microservices + */ +export class ETagService { + public constructor(private microserviceProxy: MicroserviceProxyService) {} + + /** + * Fetches a resource with ETag header for safe operations + */ + public async fetchWithETag(req: Request, service: 'LFX_V2_SERVICE', path: string, operation: string): Promise> { + req.log.info( + { + operation, + step: 'fetch_with_etag', + path, + }, + 'Fetching resource to obtain ETag header' + ); + + try { + const response = await this.microserviceProxy.proxyRequestWithResponse(req, service, path, 'GET'); + + if (!response.data) { + const error: ETagError = { + code: 'NOT_FOUND', + message: 'Resource not found', + statusCode: 404, + }; + throw error; + } + + const etag = response.headers[HTTP_HEADERS.ETAG.toLowerCase()] || response.headers[HTTP_HEADERS.ETAG]; + + if (!etag) { + req.log.warn( + { + operation, + path, + error: 'ETag header not found in response', + available_headers: Object.keys(response.headers), + }, + 'ETag header missing from resource response' + ); + + const error: ETagError = { + code: 'ETAG_MISSING', + message: 'Unable to obtain ETag header for safe operation', + statusCode: 500, + headers: response.headers, + }; + throw error; + } + + req.log.info( + { + operation, + step: 'resource_fetched', + path, + has_etag: true, + }, + 'Resource fetched successfully with ETag' + ); + + return { + data: response.data, + etag, + headers: response.headers, + }; + } catch (error) { + if (this.isETagError(error)) { + throw error; + } + + const errorDetails = extractErrorDetails(error); + const etagError: ETagError = { + code: 'NETWORK_ERROR', + message: errorDetails.message, + statusCode: errorDetails.statusCode, + }; + throw etagError; + } + } + + /** + * Performs a safe update operation using If-Match header + */ + public async updateWithETag(req: Request, service: 'LFX_V2_SERVICE', path: string, etag: string, data: any, operation: string): Promise { + req.log.info( + { + operation, + step: 'update_with_etag', + path, + etag_value: etag, + }, + 'Attempting to update resource with If-Match header' + ); + + return await this.microserviceProxy.proxyRequest(req, service, path, 'PUT', {}, data, { [HTTP_HEADERS.IF_MATCH]: etag }); + } + + /** + * Performs a safe delete operation using If-Match header + */ + public async deleteWithETag(req: Request, service: 'LFX_V2_SERVICE', path: string, etag: string, operation: string): Promise { + req.log.info( + { + operation, + step: 'delete_with_etag', + path, + etag_value: etag, + }, + 'Attempting to delete resource with If-Match header' + ); + + await this.microserviceProxy.proxyRequest(req, service, path, 'DELETE', {}, undefined, { [HTTP_HEADERS.IF_MATCH]: etag }); + } + + /** + * Type guard for ETag errors + */ + private isETagError(error: unknown): error is ETagError { + if (error === null || typeof error !== 'object') return false; + + const errorObj = error as Record; + return ( + typeof errorObj['code'] === 'string' && + typeof errorObj['statusCode'] === 'number' && + ['NOT_FOUND', 'ETAG_MISSING', 'NETWORK_ERROR', 'PRECONDITION_FAILED'].includes(errorObj['code'] as string) + ); + } +} diff --git a/apps/lfx-pcc/src/server/services/microservice-proxy.service.ts b/apps/lfx-pcc/src/server/services/microservice-proxy.service.ts index 988bb9f1..683a125a 100644 --- a/apps/lfx-pcc/src/server/services/microservice-proxy.service.ts +++ b/apps/lfx-pcc/src/server/services/microservice-proxy.service.ts @@ -2,11 +2,12 @@ // SPDX-License-Identifier: MIT import { DEFAULT_QUERY_PARAMS } from '@lfx-pcc/shared/constants'; -import { MicroserviceUrls } from '@lfx-pcc/shared/interfaces'; +import { ApiResponse, MicroserviceUrls, ApiError, extractErrorDetails } from '@lfx-pcc/shared/interfaces'; import { Request } from 'express'; import { serverLogger } from '../server'; import { ApiClientService } from './api-client.service'; +import { createApiError } from '../utils/api-error'; export class MicroserviceProxyService { private apiClient: ApiClientService; @@ -21,7 +22,8 @@ export class MicroserviceProxyService { path: string, method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' = 'GET', params?: Record, - data?: any + data?: any, + customHeaders?: Record ): Promise { try { if (!req.bearerToken) { @@ -41,7 +43,7 @@ export class MicroserviceProxyService { const defaultParams = DEFAULT_QUERY_PARAMS; const mergedParams = { ...params, ...defaultParams }; - const response = await this.executeRequest(method, endpoint, token, data, mergedParams); + const response = await this.executeRequest(method, endpoint, token, data, mergedParams, customHeaders); return response.data; } catch (error) { serverLogger.error( @@ -61,38 +63,84 @@ export class MicroserviceProxyService { } } + public async proxyRequestWithResponse( + req: Request, + service: keyof MicroserviceUrls, + path: string, + method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' = 'GET', + params?: Record, + data?: any, + customHeaders?: Record + ): Promise> { + try { + if (!req.bearerToken) { + throw new Error('Bearer token not available on request'); + } + + const MICROSERVICE_URLS: MicroserviceUrls = { + LFX_V2_SERVICE: process.env['LFX_V2_SERVICE'] || 'http://lfx-api.k8s.orb.local', + }; + + const baseUrl = MICROSERVICE_URLS[service]; + const endpoint = `${baseUrl}${path}`; + const token = req.bearerToken; + + // Merge query parameters with defaults taking precedence + // This ensures that default params cannot be overridden by the caller + const defaultParams = DEFAULT_QUERY_PARAMS; + const mergedParams = { ...params, ...defaultParams }; + + const response = await this.executeRequest(method, endpoint, token, data, mergedParams, customHeaders); + return response; + } catch (error) { + serverLogger.error( + { + service, + path, + method, + error: error instanceof Error ? error.message : error, + stack: error instanceof Error && process.env['NODE_ENV'] !== 'production' ? error.stack : undefined, + endpoint: `${service}${path}`, + has_bearer_token: !!req.bearerToken, + }, + 'Microservice request failed' + ); + + throw this.transformError(error, service, path); + } + } + private async executeRequest( method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE', endpoint: string, bearerToken: string, data?: any, - params?: Record + params?: Record, + customHeaders?: Record ) { switch (method) { case 'GET': - return await this.apiClient.get(endpoint, bearerToken, params); + return await this.apiClient.get(endpoint, bearerToken, params, customHeaders); case 'POST': - return await this.apiClient.post(endpoint, bearerToken, data); + return await this.apiClient.post(endpoint, bearerToken, data, customHeaders); case 'PUT': - return await this.apiClient.put(endpoint, bearerToken, data); + return await this.apiClient.put(endpoint, bearerToken, data, customHeaders); case 'PATCH': - return await this.apiClient.patch(endpoint, bearerToken, data); + return await this.apiClient.patch(endpoint, bearerToken, data, customHeaders); case 'DELETE': - return await this.apiClient.delete(endpoint, bearerToken); + return await this.apiClient.delete(endpoint, bearerToken, customHeaders); default: throw new Error(`Unsupported HTTP method: ${method}`); } } - private transformError(error: any, service: string, path: string): Error { - const originalMessage = error instanceof Error ? error.message : String(error); - - const statusCode = error.status || 500; + private transformError(error: unknown, service: string, path: string): ApiError { + const errorDetails = extractErrorDetails(error); let userMessage: string; let errorCode: string; - switch (statusCode) { + switch (errorDetails.statusCode) { case 400: userMessage = 'Invalid request. Please check your input and try again.'; errorCode = 'BAD_REQUEST'; @@ -132,10 +180,10 @@ export class MicroserviceProxyService { errorCode = 'SERVICE_UNAVAILABLE'; break; default: - if (originalMessage.includes('timeout')) { + if (errorDetails.message.includes('timeout')) { userMessage = 'Request timeout. Please try again.'; errorCode = 'TIMEOUT'; - } else if (originalMessage.includes('Network')) { + } else if (errorDetails.message.includes('Network')) { userMessage = 'Network error. Please check your connection and try again.'; errorCode = 'NETWORK_ERROR'; } else { @@ -144,13 +192,14 @@ export class MicroserviceProxyService { } } - const transformedError = new Error(userMessage); - (transformedError as any).code = errorCode; - (transformedError as any).status = statusCode; - (transformedError as any).service = service; - (transformedError as any).path = path; - (transformedError as any).originalMessage = originalMessage; - - return transformedError; + return createApiError({ + message: userMessage, + statusCode: errorDetails.statusCode, + code: errorCode, + service, + path, + originalMessage: errorDetails.message, + originalError: error instanceof Error ? error : undefined, + }); } } diff --git a/apps/lfx-pcc/src/server/services/supabase.service.ts b/apps/lfx-pcc/src/server/services/supabase.service.ts index ae5d1ac8..c4227c87 100644 --- a/apps/lfx-pcc/src/server/services/supabase.service.ts +++ b/apps/lfx-pcc/src/server/services/supabase.service.ts @@ -129,8 +129,8 @@ export class SupabaseService { const committeesWithCounts = await Promise.all( committees.map(async (committee: Committee) => { const [totalMembers, totalVotingReps] = await Promise.all([ - this.getCommitteeMemberCountByCommitteeId(committee.id), - this.getCommitteeVotingRepsCount(committee.id), + this.getCommitteeMemberCountByCommitteeId(committee.uid), + this.getCommitteeVotingRepsCount(committee.uid), ]); return { diff --git a/docs/architecture/backend/README.md b/docs/architecture/backend/README.md index 78b84290..a600e5a9 100644 --- a/docs/architecture/backend/README.md +++ b/docs/architecture/backend/README.md @@ -2,23 +2,50 @@ ## 🖥 Overview -The LFX PCC backend consists of an Express.js server handling SSR, authentication, and API services with integrated logging and monitoring. +The LFX PCC backend follows a modern **Controller-Service pattern** with Express.js server handling SSR, authentication, and API services. The architecture emphasizes separation of concerns, maintainability, and integration with microservices. ## 🏗 Architecture Components ### Server Stack - **Express.js** server with Angular Universal SSR -- **Auth0** authentication integration -- **Winston** logging with structured logs -- **PM2** process management for production +- **Auth0** authentication integration with express-openid-connect +- **Pino** structured JSON logging with sensitive data redaction +- **PM2** process management for production deployment +- **Controller-Service Pattern** for clean architecture separation -### Services +### Core Architecture Patterns -- **Authentication Service** with Auth0 integration -- **User Service** for user profile management -- **Logging Service** with Winston and custom transports -- **Health Check** endpoints for monitoring +#### Controller-Service Pattern + +```text +Request → Controller → Service → Microservice/Data Layer + ↓ ↓ + Validation Business Logic + Logging Data Transformation + Response Error Handling +``` + +#### Helper Classes + +- **Logger Helper**: Standardized request logging with timing and context +- **Responder Helper**: Consistent error response formatting +- **ETag Service**: Concurrency control for safe data operations + +### API Services + +#### Committee Management Service + +- **CommitteeController**: HTTP request handling and validation +- **CommitteeService**: Business logic and microservice integration +- **CRUD Operations**: Create, Read, Update, Delete with ETag concurrency control +- **Query Service Integration**: Integration with LFX Query Service microservice + +#### Authentication & Session Management + +- **Auth0 Integration**: OpenID Connect with session-based authentication +- **Token Management**: Automatic token refresh and validation +- **User Context**: Request-scoped user information and permissions ## 📋 Documentation Sections @@ -40,14 +67,219 @@ Discover PM2 configuration, production deployment, and server management. ## 🚀 Key Features -- **Server-Side Rendering**: Angular Universal with Express.js -- **Authentication**: Auth0 integration with secure token handling -- **Structured Logging**: Winston with custom formatters and transports -- **Process Management**: PM2 for production deployment -- **Health Monitoring**: Built-in health check endpoints +### Architecture & Design Patterns + +- **Controller-Service Pattern**: Clean separation between HTTP handling and business logic +- **Helper Classes**: Reusable utilities for logging, response formatting, and data validation +- **Microservice Integration**: Seamless integration with LFX Query Service and other microservices +- **ETag Concurrency Control**: Safe concurrent operations with optimistic locking + +### Core Services + +- **Server-Side Rendering**: Angular Universal with Express.js for optimal SEO and performance +- **Authentication**: Auth0 integration with OpenID Connect and session management +- **Structured Logging**: Pino with request correlation, timing, and sensitive data redaction +- **Process Management**: PM2 for production deployment with health monitoring +- **Health Monitoring**: Built-in health check endpoints with detailed system status + +### Development & Quality + +- **TypeScript**: Full type safety with shared interfaces across frontend and backend +- **Validation**: Comprehensive input validation with detailed error responses +- **Error Handling**: Consistent error formatting and microservice compatibility +- **Testing**: E2E testing with Playwright and comprehensive test coverage + +## 🔧 Implementation Details + +### Helper Classes Architecture + +#### Logger Helper (`/server/helpers/logger.ts`) + +```typescript +export class Logger { + static start(req: Request, operation: string, metadata?: Record): number; + static success(req: Request, operation: string, startTime: number, metadata?: Record): void; + static error(req: Request, operation: string, startTime: number, error: any, metadata?: Record): void; + static validation(req: Request, operation: string, validationErrors: any[], metadata?: Record): void; + static etag(req: Request, operation: string, resourceType: string, resourceId: string, etag?: string, metadata?: Record): void; + static warning(req: Request, operation: string, message: string, metadata?: Record): void; + static sanitize(metadata: Record): Record; +} +``` + +**Features:** + +- Request correlation with unique IDs +- Operation timing and performance metrics +- Sensitive data sanitization (passwords, tokens, secrets) +- Structured JSON logging with context + +#### Responder Helper (`/server/helpers/responder.ts`) + +```typescript +export class Responder { + static error(res: Response, message: string, options?: ErrorOptions): void; + static badRequest(res: Response, message?: string, details?: Record): void; + static unauthorized(res: Response, message?: string): void; + static forbidden(res: Response, message?: string): void; + static notFound(res: Response, message?: string): void; + static conflict(res: Response, message?: string): void; + static preconditionFailed(res: Response, message?: string): void; + static validationError(res: Response, errors: ValidationError[], message?: string): void; + static internalError(res: Response, message?: string): void; + static handle(res: Response, error: any, operation?: string): void; + static createPagination(page: number, limit: number, total: number): PaginationInfo; +} +``` + +**Features:** + +- Consistent error response format across all endpoints +- HTTP status code standardization +- Validation error aggregation +- ETag conflict handling +- Microservice response compatibility + +#### ETag Service (`/server/services/etag.service.ts`) + +```typescript +export class ETagService { + constructor(private microserviceProxy: MicroserviceProxyService); + async fetchWithETag(req: Request, service: string, path: string, operation: string): Promise>; + async updateWithETag(req: Request, service: string, path: string, etag: string, data: any, operation: string): Promise; + async deleteWithETag(req: Request, service: string, path: string, etag: string, operation: string): Promise; +} +``` + +**Features:** + +- Optimistic concurrency control using HTTP If-Match headers +- ETag extraction and validation from response headers +- Conflict detection and resolution (412 Precondition Failed) +- Integration with microservice ETag headers following HTTP standards + +### Controller-Service Pattern Implementation + +#### Example: Committee Management + +**Controller Layer** (`/server/controllers/committee.controller.ts`): + +```typescript +export class CommitteeController { + public async getCommittees(req: Request, res: Response): Promise { + const startTime = Logger.start(req, 'get_committees', { + query_params: Logger.sanitize(req.query as Record), + }); + + try { + const committees = await this.committeeService.getCommittees(req, req.query); + + Logger.success(req, 'get_committees', startTime, { + committee_count: committees.length, + }); + + res.json(committees); + } catch (error) { + Logger.error(req, 'get_committees', startTime, error); + Responder.handle(res, error, 'get_committees'); + } + } +} +``` + +**Service Layer** (`/server/services/committee.service.ts`): + +```typescript +export class CommitteeService { + public async getCommittees(req: Request, queryParams: any): Promise { + // Business logic and microservice integration + const { resources } = await this.microserviceProxy.proxyRequest>(req, 'LFX_V2_SERVICE', '/query/resources', 'GET', { + ...queryParams, + type: 'committee', + }); + + return resources.map((resource) => resource.data); + } +} +``` + +**Benefits:** + +- **Separation of Concerns**: Controllers handle HTTP, Services handle business logic +- **Testability**: Each layer can be unit tested independently +- **Maintainability**: Clear boundaries and responsibilities +- **Reusability**: Services can be reused across different controllers +- **Consistency**: Standardized logging, error handling, and response formatting + +## 📁 Directory Structure + +```text +apps/lfx-pcc/src/server/ +├── controllers/ # HTTP request handling layer +│ └── committee.controller.ts +├── services/ # Business logic layer +│ ├── committee.service.ts +│ ├── etag.service.ts +│ ├── api-client.service.ts +│ └── microservice-proxy.service.ts +├── helpers/ # Utility classes +│ ├── logger.ts # Standardized logging +│ ├── responder.ts # Response formatting +│ └── url-validation.ts # Input validation +├── middleware/ # Express middleware +│ ├── auth-token.middleware.ts +│ ├── error-handler.middleware.ts +│ └── token-refresh.middleware.ts +├── routes/ # Route definitions +│ ├── committees.ts +│ ├── meetings.ts +│ ├── permissions.ts +│ └── projects.ts +└── utils/ # Shared utilities + └── api-error.ts +``` + +## 🎯 Best Practices + +### Development Guidelines + +1. **Controller Responsibilities**: + - HTTP request/response handling + - Input validation and sanitization + - Request logging and timing + - Error response formatting + +2. **Service Responsibilities**: + - Business logic implementation + - Microservice integration + - Data transformation and validation + - Complex error handling + +3. **Helper Usage**: + - Use Logger for all operations with timing + - Use Responder for consistent error responses + - Sanitize sensitive data before logging + - Handle ETags for concurrency control + +4. **Error Handling**: + - Always log errors with context + - Use appropriate HTTP status codes + - Provide meaningful error messages + - Maintain microservice compatibility + +### Code Quality Standards + +- **TypeScript**: Strict type checking enabled +- **Interfaces**: All interfaces in shared package +- **Validation**: Comprehensive input validation +- **Testing**: Unit tests for services, E2E for controllers +- **Documentation**: JSDoc comments for public methods +- **Linting**: ESLint with Prettier formatting ## 🔗 Quick Links - [Server Configuration](../../CLAUDE.md#backend-stack) - [Development Commands](../../CLAUDE.md#development) - [Production Deployment](../../CLAUDE.md#production) +- [Shared Interfaces](../shared/package-architecture.md) +- [Frontend Integration](../frontend/README.md) diff --git a/packages/shared/src/constants/committees.ts b/packages/shared/src/constants/committees.ts index e934e854..2b85892a 100644 --- a/packages/shared/src/constants/committees.ts +++ b/packages/shared/src/constants/committees.ts @@ -89,6 +89,13 @@ export const COMMITTEE_TYPE_COLORS = { */ export const DEFAULT_COMMITTEE_TYPE_COLOR = 'text-gray-500'; +/** + * Get valid committee category values for validation + */ +export function getValidCommitteeCategories(): string[] { + return COMMITTEE_CATEGORIES.map((category) => category.value); +} + /** * Get color class for a committee type */ diff --git a/packages/shared/src/constants/index.ts b/packages/shared/src/constants/index.ts index 2ca9e9bf..e24f11df 100644 --- a/packages/shared/src/constants/index.ts +++ b/packages/shared/src/constants/index.ts @@ -6,4 +6,5 @@ export * from './colors'; export * from './committees'; export * from './file-upload'; export * from './font-sizes'; +export * from './server'; export * from './timezones'; diff --git a/packages/shared/src/constants/server.ts b/packages/shared/src/constants/server.ts new file mode 100644 index 00000000..0418d4d2 --- /dev/null +++ b/packages/shared/src/constants/server.ts @@ -0,0 +1,52 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +/** + * Sensitive field names for data sanitization in logging + * These fields will be redacted when logging request/response data + */ +export const SENSITIVE_FIELDS = [ + 'password', + 'token', + 'secret', + 'key', + 'authorization', + 'cookie', + 'jwt', + 'bearer', + 'auth', + 'credentials', + 'apikey', + 'api_key', + 'access_token', + 'refresh_token', +] as const; + +/** + * Standard HTTP header names with correct casing + */ +export const HTTP_HEADERS = { + ETAG: 'ETag', + IF_MATCH: 'If-Match', + CONTENT_TYPE: 'Content-Type', + AUTHORIZATION: 'Authorization', + USER_AGENT: 'User-Agent', + ACCEPT: 'Accept', + CACHE_CONTROL: 'Cache-Control', +} as const; + +/** + * Common error codes used across the application + */ +export const ERROR_CODES = { + NOT_FOUND: 'NOT_FOUND', + PRECONDITION_FAILED: 'PRECONDITION_FAILED', + ETAG_MISSING: 'ETAG_MISSING', + NETWORK_ERROR: 'NETWORK_ERROR', + VALIDATION_ERROR: 'VALIDATION_ERROR', + BAD_REQUEST: 'BAD_REQUEST', + UNAUTHORIZED: 'UNAUTHORIZED', + FORBIDDEN: 'FORBIDDEN', + CONFLICT: 'CONFLICT', + INTERNAL_ERROR: 'INTERNAL_ERROR', +} as const; diff --git a/packages/shared/src/interfaces/api.ts b/packages/shared/src/interfaces/api.ts index 4ac9b181..cbca71c5 100644 --- a/packages/shared/src/interfaces/api.ts +++ b/packages/shared/src/interfaces/api.ts @@ -20,21 +20,25 @@ export interface MicroserviceUrls { export interface ApiError extends Error { status?: number; + statusCode?: number; code?: string; service?: string; path?: string; originalMessage?: string; + cause?: { code?: string }; + response?: ApiResponse; } export interface ApiErrorOptions { message: string; status?: number; + statusCode?: number; code?: string; service?: string; path?: string; originalMessage?: string; originalError?: Error; - response?: any; + response?: ApiResponse; } export interface QueryServiceItem { @@ -46,3 +50,107 @@ export interface QueryServiceItem { export interface QueryServiceResponse { resources: QueryServiceItem[]; } + +export interface ETagResult { + data: T; + etag: string; + headers: Record; +} + +export interface ETagError { + code: 'NOT_FOUND' | 'ETAG_MISSING' | 'NETWORK_ERROR' | 'PRECONDITION_FAILED'; + message: string; + statusCode: number; + headers?: Record; +} + +/** + * Standard API error response interface + */ +export interface ApiErrorResponse { + error: string; + code?: string; + errors?: ValidationError[]; + details?: Record; +} + +export interface ValidationError { + field: string; + message: string; + code: string; +} + +export interface PaginationInfo { + page: number; + limit: number; + total: number; + pages: number; + hasNext: boolean; + hasPrev: boolean; +} + +/** + * Enhanced validation error interface with proper status code + */ +export interface ValidationApiError extends ApiError { + statusCode: 400; + code: 'VALIDATION_ERROR'; + validationErrors: ValidationError[]; +} + +/** + * Type guard to check if error is an ApiError + */ +export function isApiError(error: unknown): error is ApiError { + if (!(error instanceof Error)) return false; + + return ( + ('status' in error && typeof error.status === 'number') || + ('statusCode' in error && typeof error.statusCode === 'number') || + ('code' in error && typeof error.code === 'string') + ); +} + +/** + * Type guard to check if error is a ValidationApiError + */ +export function isValidationApiError(error: unknown): error is ValidationApiError { + if (!isApiError(error) || error.code !== 'VALIDATION_ERROR') return false; + + return 'validationErrors' in error && Array.isArray(error.validationErrors); +} + +/** + * Utility to safely extract error properties with proper typing + */ +export function extractErrorDetails(error: unknown): { + message: string; + statusCode: number; + code: string; + service?: string; + path?: string; +} { + if (isApiError(error)) { + return { + message: error.message, + statusCode: error.statusCode || error.status || 500, + code: error.code || 'UNKNOWN_ERROR', + service: error.service, + path: error.path, + }; + } + + if (error instanceof Error) { + return { + message: error.message, + statusCode: 500, + code: 'INTERNAL_ERROR', + }; + } + + return { + message: String(error), + statusCode: 500, + code: 'UNKNOWN_ERROR', + }; +} diff --git a/packages/shared/src/interfaces/committee.interface.ts b/packages/shared/src/interfaces/committee.interface.ts index 9b14f684..5869d7bf 100644 --- a/packages/shared/src/interfaces/committee.interface.ts +++ b/packages/shared/src/interfaces/committee.interface.ts @@ -2,32 +2,80 @@ // SPDX-License-Identifier: MIT export interface Committee { - id: string; + uid: string; name: string; + display_name?: string; category: string; - parent_uid?: string | null; description?: string; - business_email_required: boolean; + parent_uid?: string; enable_voting: boolean; - is_audit_enabled: boolean; - public_enabled: boolean; - public_name?: string; + public: boolean; sso_group_enabled: boolean; sso_group_name?: string; - committee_website?: string; - created_by?: string; + website?: string; + requires_review?: boolean; created_at: string; - updated_at?: string; - updated_by?: string; - system_mod_stamp: string; + updated_at: string; total_members: number; total_voting_reps: number; project_uid: string; - joinable?: boolean; + project_name?: string; + calendar?: { + public: boolean; + }; + // These fields will be populated from the settings endpoint + business_email_required?: boolean; + is_audit_enabled?: boolean; +} + +export interface CommitteeSettings { + uid: string; + business_email_required: boolean; + last_reviewed_at?: string; + last_reviewed_by?: string; + writers: string[]; + auditors: string[]; + created_at: string; + updated_at: string; } export interface CommitteeSummary { - id: string; + uid: string; + name: string; + category: string; +} + +export interface CommitteeCreateData { name: string; category: string; + description?: string; + parent_uid?: string; + business_email_required?: boolean; + enable_voting?: boolean; + is_audit_enabled?: boolean; + public?: boolean; + display_name?: string; + sso_group_enabled?: boolean; + sso_group_name?: string; + website?: string; + project_uid?: string; + joinable?: boolean; +} + +export interface CommitteeUpdateData extends Partial {} + +export interface CommitteeSettingsData { + business_email_required?: boolean; + is_audit_enabled?: boolean; +} + +export interface CommitteeValidationError { + field: string; + message: string; + code: string; +} + +export interface CommitteeValidationResult { + isValid: boolean; + errors: CommitteeValidationError[]; } diff --git a/packages/shared/src/interfaces/meeting.interface.ts b/packages/shared/src/interfaces/meeting.interface.ts index bc40352d..88897752 100644 --- a/packages/shared/src/interfaces/meeting.interface.ts +++ b/packages/shared/src/interfaces/meeting.interface.ts @@ -15,7 +15,7 @@ export interface MeetingRecurrence { } export interface MeetingCommittee { - id: string; + uid: string; name: string; committee_total_members: number; }