feat(ui): rename application from LFX PCC to LFX One #87
Conversation
- Rename apps/lfx-pcc/ to apps/lfx-one/ for cleaner branding - Update package references from @lfx-pcc/shared to @lfx-one/shared - Update all user-facing text from "LFX Projects"/"LFX PCC" to "LFX One" - Update page titles, tooltips, and UI components - Update comprehensive documentation across all architecture docs - Update CI/CD workflows and Docker image labels - Update Helm chart documentation and configuration - Update PM2 process management configuration LFXV2-489 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRepository-wide rebrand from "LFX PCC" to "LFX One": package and workspace names, TypeScript path aliases, app directories, import paths, Docker/PM2 identifiers, CI/E2E workflow paths and image labels, UI text/titles, and documentation updated. No functional runtime logic changes introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR renames the application from "LFX PCC" (Project Control Center) to "LFX One" across the entire codebase for improved branding and consistency. The changes include updating package names, application references, and user-facing text while maintaining all functionality.
- Renamed folder structure from
apps/lfx-pcc/toapps/lfx-one/ - Updated shared package references from
@lfx-pcc/sharedto@lfx-one/shared - Changed all user-facing text, documentation, and configuration files
Reviewed Changes
Copilot reviewed 117 out of 283 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/package.json | Updated package name to @lfx-one/shared |
| package.json | Renamed root package and PM2 script references |
| Multiple TypeScript files | Updated import statements to use new shared package name |
| Documentation files | Updated references from LFX PCC to LFX One |
| Configuration files | Updated Angular config, Docker files, and CI/CD references |
| E2E tests | Updated page title expectations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.../lfx-one/src/app/modules/project/settings/settings-dashboard/settings-dashboard.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
Show resolved
Hide resolved
...s/project/meetings/components/agenda-template-selector/agenda-template-selector.component.ts
Show resolved
Hide resolved
.../lfx-one/src/app/modules/project/settings/settings-dashboard/settings-dashboard.component.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
apps/lfx-one/src/server/services/ai.service.ts (3)
13-23: Remove hardcoded API key fallback; enforce env var.
Defaulting to a fake key risks accidental prod usage and weakens secrets handling.Apply:
- private readonly aiKey = process.env['AI_API_KEY'] || 'sk-proj-1234567890'; + private readonly aiKey: string; ... - this.aiProxyUrl = process.env['AI_PROXY_URL'] || 'https://api.openai.com/v1/chat/completions'; - if (!this.aiProxyUrl) { - throw new Error('AI_PROXY_URL environment variable is required'); - } - - if (!this.aiKey) { - throw new Error('AI_API_KEY environment variable is required'); - } + this.aiProxyUrl = this.requireEnv('AI_PROXY_URL'); + this.aiKey = this.requireEnv('AI_API_KEY'); + } + + private requireEnv(name: string): string { + const v = process.env[name]; + if (!v) throw new Error(`${name} environment variable is required`); + return v; }
123-139: Add request timeout and abort; set Accept header.
External call without timeout can hang request threads.Apply:
private async makeAiRequest(request: OpenAIChatRequest): Promise<OpenAIChatResponse> { - const response = await fetch(this.aiProxyUrl, { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 30000); // 30s + const response = await fetch(this.aiProxyUrl, { method: 'POST', headers: { ['Content-Type']: 'application/json', + ['Accept']: 'application/json', ['Authorization']: `Bearer ${this.aiKey}`, }, - body: JSON.stringify(request), + body: JSON.stringify(request), + signal: controller.signal, }); + clearTimeout(timeout);
172-175: Avoid logging raw model output (possible sensitive content).
Trim or removecontentfrom logs to prevent PII leakage.Apply:
- serverLogger.warn('Failed to parse JSON response, falling back to text extraction', { - content: content.substring(0, 100), - error: parseError, - }); + serverLogger.warn('Failed to parse JSON response; falling back to text extraction', { + content_preview_len: Math.min(content.length, 100), + error: (parseError as Error).message, + });apps/lfx-one/src/server/helpers/logger.ts (1)
17-26: Sanitize metadata before logging (sanitize() is never applied).
Current logs spread raw metadata; sensitive keys may leak. Apply sanitize() in all helpers before spreading metadata.public static start(req: Request, operation: string, metadata: Record<string, any> = {}): number { const startTime = Date.now(); - req.log.info( + const meta = Logger.sanitize(metadata); + req.log.info( { operation, - ...metadata, + ...meta, request_id: req.id, user_agent: req.get('User-Agent'), ip_address: req.ip, }, `Starting ${operation.replace(/_/g, ' ')}` );public static success(req: Request, operation: string, startTime: number, metadata: Record<string, any> = {}): void { const duration = Date.now() - startTime; - req.log.info( + const meta = Logger.sanitize(metadata); + req.log.info( { operation, duration, - status_code: metadata['status_code'] || 200, - ...metadata, + status_code: meta['status_code'] ?? 200, + ...meta, request_id: req.id, }, `Successfully completed ${operation.replace(/_/g, ' ')}` );public static error(req: Request, operation: string, startTime: number, error: unknown, metadata: Record<string, any> = {}): void { const duration = Date.now() - startTime; - req.log.error( + const meta = Logger.sanitize(metadata); + req.log.error( { operation, duration, error: error instanceof Error ? error.message : String(error), - ...metadata, + ...meta, request_id: req.id, }, `Failed to ${operation.replace(/_/g, ' ')}` );public static validation(req: Request, operation: string, validationErrors: any[], metadata: Record<string, any> = {}): void { - req.log.warn( + const meta = Logger.sanitize(metadata); + req.log.warn( { operation, validation_errors: validationErrors, status_code: 400, - ...metadata, + ...meta, request_id: req.id, }, `Validation failed for ${operation.replace(/_/g, ' ')}` );public static etag(req: Request, operation: string, resourceType: string, resourceId: string, etag?: string, metadata: Record<string, any> = {}): void { - req.log.info( + const meta = Logger.sanitize(metadata); + req.log.info( { operation, resource_type: resourceType, resource_id: resourceId, etag, - ...metadata, + ...meta, request_id: req.id, }, `ETag operation: ${operation.replace(/_/g, ' ')}` );public static warning(req: Request, operation: string, message: string, metadata: Record<string, any> = {}): void { - req.log.warn( + const meta = Logger.sanitize(metadata); + req.log.warn( { operation, warning_message: message, - ...metadata, + ...meta, request_id: req.id, }, `Warning during ${operation.replace(/_/g, ' ')}: ${message}` );Also applies to: 37-46, 55-64, 71-80, 87-97, 104-112, 118-128
apps/lfx-one/src/server/services/api-client.service.ts (1)
38-44: Update User-Agent to LFX One.
Leftover brand string in a public header.Accept: 'application/json', ['Content-Type']: 'application/json', - ['User-Agent']: 'LFX-PCC-Server/1.0', + ['User-Agent']: 'LFX-One-Server/1.0',apps/lfx-one/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts (1)
93-109: Guard against undefined startDate to prevent runtime errors.Several paths call
.getDate()/.getDay()on a possibly undefined start date (common during initial form render), which will throw.Apply this diff:
@@ - public onMonthlyTypeChange(monthlyType: string): void { + public onMonthlyTypeChange(monthlyType: string): void { const recurrenceForm = this.recurrenceForm(); if (!recurrenceForm) return; if (monthlyType === 'dayOfMonth') { recurrenceForm.patchValue({ - monthly_day: this.startDate().getDate(), + monthly_day: (this.startDate()?.getDate() ?? new Date().getDate()), monthly_week: null, monthly_week_day: null, }); } else { // dayOfWeek - const startDate = this.startDate(); + const startDate = this.startDate() ?? new Date(); const { weekOfMonth } = getWeekOfMonth(startDate); recurrenceForm.patchValue({ monthly_day: null, monthly_week: weekOfMonth, monthly_week_day: startDate.getDay() + 1, // Convert 0-6 to 1-7 }); } } @@ recurrenceForm.patchValue({ type: type, // Clear pattern-specific fields when changing pattern type weekly_days: patternType === 'weekly' ? this.getDefaultWeeklyDays() : null, - monthly_day: patternType === 'monthly' ? this.startDate().getDate() : null, + monthly_day: patternType === 'monthly' ? (this.startDate()?.getDate() ?? new Date().getDate()) : null, monthly_week: null, monthly_week_day: null, }); @@ - private getDefaultWeeklyDays(): string { - // Default to the current start date's day of week - const startDate = this.startDate(); - return String(startDate.getDay() + 1); // Convert 0-6 to 1-7 - } + private getDefaultWeeklyDays(): string { + // Default to start date's DOW, or today if start date not yet set + const base = this.startDate() ?? new Date(); + return String(base.getDay() + 1); // Convert 0-6 to 1-7 + }Also applies to: 174-182, 296-300
apps/lfx-one/src/server/routes/permissions.route.ts (1)
79-83: Avoid logging PII (names/emails) in structured logs.
userDataincludes email and names; emitting it to logs is a privacy risk. Log only non-PII metadata.Apply this diff:
- req.log.info({ projectId, userData }, 'Creating user with permissions'); + req.log.info( + { + projectId, + permission_scope: userData.permission_scope, + permission_level: userData.permission_level, + committee_ids_count: userData.committee_ids?.length ?? 0, + }, + 'Creating user with permissions' + );apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
113-143: Bug: unsafe signal initialization and null access; possible runtime error when meeting is undefined.
- toSignal has no initialValue; yet code accesses this.meeting().uid (and others) without null checks → crash before data arrives.
- Error path returns an empty object then spreads res.meeting, which can be undefined → TypeError on spread.
Fix by making meeting nullable, providing an initialValue, guarding accesses, and avoiding spread on possibly undefined.
Apply this diff:
- public meeting: Signal<Meeting & { project: Project }>; + public meeting: Signal<(Meeting & { project: Project }) | null>; @@ - public returnTo: Signal<string | undefined>; + public returnTo: Signal<string | undefined>; @@ - private initializeMeeting() { - return toSignal<Meeting & { project: Project }>( + private initializeMeeting() { + return toSignal<(Meeting & { project: Project }) | null>( combineLatest([this.activatedRoute.paramMap, this.activatedRoute.queryParamMap]).pipe( switchMap(([params, queryParams]) => { const meetingId = params.get('id'); this.password.set(queryParams.get('password')); if (meetingId) { return this.meetingService.getPublicMeeting(meetingId, this.password()).pipe( catchError((error) => { // If 404, navigate to not found page if ([404, 403, 400].includes(error.status)) { this.router.navigate(['/meetings/not-found']); - return of({} as { meeting: Meeting; project: Project }); + return of(null); } // Re-throw other errors throw error; }) ); } // If no meeting ID, redirect to not found this.router.navigate(['/meetings/not-found']); - return of({} as { meeting: Meeting; project: Project }); + return of(null); }), - map((res) => ({ ...res.meeting, project: res.project })), - tap((res) => { - this.project.set(res.project); + map((res) => (res && res.meeting && res.project ? ({ ...res.meeting, project: res.project }) : null)), + tap((res) => { + if (res?.project) this.project.set(res.project); }) ) - ) as Signal<Meeting & { project: Project }>; + , { initialValue: null }); } @@ public onJoinMeeting(): void { - if (!this.canJoinMeeting()) { + if (!this.canJoinMeeting() || !this.meeting()) { this.messageService.add({ severity: 'warn', summary: 'Meeting Not Available', detail: 'The meeting has not started yet.', }); return; } @@ - .getPublicMeetingJoinUrl(this.meeting().uid, this.meeting().password, { + .getPublicMeetingJoinUrl(this.meeting()!.uid, this.meeting()!.password, { email: this.authenticated() ? this.user()?.email : this.joinForm.get('email')?.value, }) @@ - private initializeReturnTo(): Signal<string | undefined> { + private initializeReturnTo(): Signal<string | undefined> { return computed(() => { - return `${environment.urls.home}/meetings/${this.meeting().uid}?password=${this.password()}`; + const m = this.meeting(); + if (!m) return undefined; + const pw = this.password(); + return pw ? `${environment.urls.home}/meetings/${m.uid}?password=${pw}` : `${environment.urls.home}/meetings/${m.uid}`; }); } @@ - private initializeJoinUrlWithParams(): Signal<string | undefined> { + private initializeJoinUrlWithParams(): Signal<string | undefined> { return computed(() => { - const meeting = this.meeting(); + const meeting = this.meeting(); const joinUrl = meeting?.join_url; if (!joinUrl) { return undefined; } // Access form values to trigger reactivity const formValues = this.formValues(); return this.buildJoinUrlWithParams(joinUrl, formValues); }); }Also applies to: 207-211, 84-111, 229-242
104-106: Security: prevent reverse‑tabnabbing when opening external windows.Use noopener/noreferrer and clear opener.
Apply this diff:
- window.open(joinUrlWithParams, '_blank'); + const win = window.open(joinUrlWithParams, '_blank', 'noopener,noreferrer'); + if (win) win.opener = null;apps/lfx-one/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (1)
216-219: Bug: self‑parent exclusion compares uid to id (mismatch)Current committee detection uses
idbut list usesuid, so the item isn’t excluded and users can set a committee as its own parent.Apply this diff:
- const currentCommitteeId = this.committee()?.id; - const availableCommittees = currentCommitteeId ? topLevelCommittees.filter((committee) => committee.uid !== currentCommitteeId) : topLevelCommittees; + const currentCommitteeUid = this.committee()?.uid; + const availableCommittees = currentCommitteeUid + ? topLevelCommittees.filter((committee) => committee.uid !== currentCommitteeUid) + : topLevelCommittees;apps/lfx-one/src/app/modules/project/settings/settings-dashboard/settings-dashboard.component.ts (1)
41-54: Bug:usersnever loads ifproject()?.uidis initially null.
The source fortoSignalis chosen once in the constructor; later project updates don’t re-subscribe. Bind the stream to the project signal (and refresh) so it reacts when the project arrives/changes.- public constructor() { - // Initialize userPermissions signal from service - this.users = toSignal( - this.project()?.uid - ? this.refresh.pipe( - tap(() => this.loading.set(true)), - switchMap(() => this.permissionsService.getProjectPermissions(this.project()?.uid as string).pipe(tap(() => this.loading.set(false)))) - ) - : of([]), - { - initialValue: [], - } - ); - } + public constructor() { + const project$ = toObservable(this.project); + this.users = toSignal( + combineLatest([project$, this.refresh]).pipe( + switchMap(([project]) => { + if (!project?.uid) return of([]); + this.loading.set(true); + return this.permissionsService.getProjectPermissions(project.uid).pipe( + tap(() => this.loading.set(false)) + ); + }) + ), + { initialValue: [] } + ); + }Add required imports:
- import { toSignal } from '@angular/core/rxjs-interop'; + import { toObservable, toSignal } from '@angular/core/rxjs-interop'; - import { BehaviorSubject, of, switchMap, take, tap } from 'rxjs'; + import { BehaviorSubject, combineLatest, of, switchMap, take, tap } from 'rxjs';
🧹 Nitpick comments (66)
docs/architecture/testing/e2e-testing.md (1)
156-165: Replace remaining “Projects Self-Service” strings with “LFX One” in examplesTwo example blocks still reference the old brand; update to avoid confusion and keep docs consistent with the rebrand.
- await expect(page.getByText('Projects Self-Service')).toBeHidden(); + await expect(page.getByText('LFX One')).toBeHidden(); ... - await expect(page.getByText('Projects Self-Service')).toBeVisible(); + await expect(page.getByText('LFX One')).toBeVisible();- await expect(page.getByText('Projects Self-Service')).toBeHidden(); + await expect(page.getByText('LFX One')).toBeHidden(); ... - await expect(page.getByText('Projects Self-Service')).toBeVisible(); + await expect(page.getByText('LFX One')).toBeVisible();Also applies to: 218-231
apps/lfx-one/src/app/modules/project/meetings/components/meeting-platform-features/meeting-platform-features.component.ts (3)
11-11: Prefer LFX wrapper over direct PrimeNG Tooltip import (policy carry‑over).Per past guidance we learned for this codebase, use LFX wrapper components instead of importing PrimeNG directly. If an LFX tooltip wrapper exists, switch to it for consistency and theming; otherwise ignore.
Note: Referencing retrieved learning from earlier reviews; please confirm whether this convention still applies post‑rename.
29-36: Externalize “(Coming Soon)” for i18n.Avoid hard‑coded user‑facing strings; move to translations/util string helper.
64-66: Tighten typing for featureKey.Use a keyed type from the form model (e.g., keyof MeetingFeaturesFormControls) instead of string to catch template typos at compile time.
apps/lfx-one/src/server/services/microservice-proxy.service.ts (3)
30-33: Guard path and join safely to avoid double slashes and invalid inputs.Ensure
pathis a relative path starting with/and normalize the join.+ private static joinEndpoint(baseUrl: string, path: string): string { + if (!path.startsWith('/')) { + throw new Error(`Expected path to start with '/': ${path}`); + } + return `${baseUrl.replace(/\/+$/, '')}${path}`; + } @@ - const endpoint = `${baseUrl}${path}`; + const endpoint = MicroserviceProxyService.joinEndpoint(baseUrl, path); @@ - const endpoint = `${baseUrl}${path}`; + const endpoint = MicroserviceProxyService.joinEndpoint(baseUrl, path);Also applies to: 69-72
25-41: Deduplicate endpoint/operation/query assembly.The two methods repeat URL/env/query logic. Extract a small helper to reduce drift.
export class MicroserviceProxyService { private apiClient: ApiClientService = new ApiClientService(); + private readonly MICROSERVICE_URLS: MicroserviceUrls = { + LFX_V2_SERVICE: process.env['LFX_V2_SERVICE'] || 'http://lfx-api.k8s.orb.local', + }; + + private prepare<T>( + req: Request, + service: keyof MicroserviceUrls, + path: string, + method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE', + query?: Record<string, any> + ) { + const operation = `${method.toLowerCase()}_${path.replace(/\//g, '_')}`; + const baseUrl = this.MICROSERVICE_URLS[service]; + const endpoint = MicroserviceProxyService.joinEndpoint(baseUrl, path); + const token = (req as any).bearerToken; + const mergedQuery = { ...query, ...DEFAULT_QUERY_PARAMS }; + return { operation, endpoint, token, mergedQuery }; + } @@ - const operation = `${method.toLowerCase()}_${path.replace(/\//g, '_')}`; - try { - 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; - const mergedQuery = { ...query, ...DEFAULT_QUERY_PARAMS }; + const { operation, endpoint, token, mergedQuery } = this.prepare<T>(req, service, path, method, query); + try { req.log.debug({ path: endpoint, method, mergedQuery, data, customHeaders }, 'Proxy request'); @@ - const operation = `${method.toLowerCase()}_${path.replace(/\//g, '_')}`; - try { - 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; - const mergedQuery = { ...query, ...DEFAULT_QUERY_PARAMS }; + const { operation, endpoint, token, mergedQuery } = this.prepare<T>(req, service, path, method, query); + try {Also applies to: 64-78
44-47: Broaden error guard to common HTTP client shapes.If
ApiClientServicesometimes wraps Axios/fetch errors, consider derivingstatus/errorBodyfromerror.responseas a fallback.- if (error.status && error.code) { - throw MicroserviceError.fromMicroserviceResponse(error.status, error.message, error.errorBody, service, path, operation); + const status = error?.status ?? error?.response?.status; + const errorBody = error?.errorBody ?? error?.response?.data; + if (typeof status === 'number') { + throw MicroserviceError.fromMicroserviceResponse(status, error.message, errorBody, service, path, operation); }Also applies to: 81-83
apps/lfx-one/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts (2)
33-37: Guard against missing dialog data (possible undefined access).
this.dialogConfig.data?.meetingcan be undefined but is typed asMeeting, and subsequent fields dereference it. Add a hard guard and assign in ctor.Apply:
- public readonly meeting: Meeting = this.dialogConfig.data?.meeting; + public readonly meeting!: Meeting; ... - public readonly registrantCount: number = this.meeting.individual_registrants_count + this.meeting.committee_members_count; + public readonly registrantCount: number = this.meeting.individual_registrants_count + this.meeting.committee_members_count;And add a constructor (after injections):
+ public constructor() { + if (!this.dialogConfig.data?.meeting) { + throw new Error('Meeting data is required to delete.'); + } + this.meeting = this.dialogConfig.data.meeting as Meeting; + }
99-103: Strongly type the form control to avoidanyfromAbstractControl.value.
Prevents accidental invalid delete types.Apply:
- public readonly deleteForm: FormGroup = this.initializeDeleteForm(); + public readonly deleteForm: FormGroup<{ deleteType: FormControl<'single' | 'series' | 'future'> }> = this.initializeDeleteForm(); ... - private initializeDeleteForm(): FormGroup { - return new FormGroup({ - deleteType: new FormControl('single'), - }); - } + private initializeDeleteForm(): FormGroup<{ deleteType: FormControl<'single' | 'series' | 'future'> }> { + return new FormGroup<{ deleteType: FormControl<'single' | 'series' | 'future'> }>({ + deleteType: new FormControl<'single' | 'series' | 'future'>('single', { nonNullable: true }), + }); + }Also applies to: 42-43
apps/lfx-one/src/server/services/ai.service.ts (1)
82-86: Preserve original error as cause.
Improves observability without leaking details to callers.Apply:
- throw new Error('Failed to generate meeting agenda'); + throw new Error('Failed to generate meeting agenda', { cause: error as Error });apps/lfx-one/src/app/shared/components/header/header.component.ts (1)
122-125: Drop console noise in production.
Replace with a UI action or remove.- console.info('User avatar clicked'); + // TODO: open user menu / navigate to profileapps/lfx-one/src/app/shared/pipes/linkify.pipe.ts (2)
6-6: Avoid barrel import from '@lfx-one/shared' if possible.
Prefer direct path import to prevent unintended deps and tree-shaking issues.If available, switch to the concrete module path (e.g.,
@lfx-one/shared/utils/extract-urls).
12-38: Return type mismatch: function returns string, signature says SafeHtml.
Either returnSafeHtmlor change the signature tostring.Apply one of:
Option A (return SafeHtml):
- public transform(value: string | null | undefined): SafeHtml { + public transform(value: string | null | undefined): SafeHtml { ... - return this.sanitizer.sanitize(SecurityContext.HTML, linkedText) || ''; + const sanitized = this.sanitizer.sanitize(SecurityContext.HTML, linkedText) || ''; + return this.sanitizer.bypassSecurityTrustHtml(sanitized);Option B (simpler, return string):
- public transform(value: string | null | undefined): SafeHtml { + public transform(value: string | null | undefined): string { ... - return this.sanitizer.sanitize(SecurityContext.HTML, linkedText) || ''; + return this.sanitizer.sanitize(SecurityContext.HTML, linkedText) || '';Please pick one based on template usage.
apps/lfx-one/src/server/services/supabase.service.ts (1)
22-22: Optional: avoid barrel-style interface imports for consistency.If
@lfx-one/shared/interfacesis a barrel, prefer direct paths (e.g.,@lfx-one/shared/interfaces/user) to reduce import cycles and improve tree-shaking. This follows past guidance captured in learnings.apps/lfx-one/src/app/shared/services/project.service.ts (1)
6-6: Optional: prefer direct type imports over interface barrel.If available, import
Projectfrom its concrete module (e.g.,@lfx-one/shared/interfaces/project) to avoid barrel coupling.apps/lfx-one/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (2)
11-11: Align VOTING_STATUSES import with shared constants path.Elsewhere we import constants from
@lfx-one/shared/constants. For consistency, importVOTING_STATUSESfrom the constants module rather than the package root.Apply:
-import { VOTING_STATUSES } from '@lfx-one/shared'; +import { VOTING_STATUSES } from '@lfx-one/shared/constants';
16-18: PrimeNG direct imports — consider using LFX wrappers.Per prior team guidance, prefer LFX wrapper components/services over direct PrimeNG usage where wrappers exist (DynamicDialog, Tooltip, MessageService).
Do wrappers exist for these in this repo? If yes, we should switch in a follow-up to keep consistency.
apps/lfx-one/src/app/shared/components/header/header.component.html (1)
12-12: Rename brittle test-id to a brand-agnostic token.Avoid future churn by using a neutral id.
Apply:
-<span class="text-xl font-display text-gray-900 hidden md:block" data-testid="header-projects-text">One</span> +<span class="text-xl font-display text-gray-900 hidden md:block" data-testid="header-brand-text">One</span>Remember to update E2E selectors accordingly.
docs/architecture/backend/README.md (1)
74-76: Inconsistent logger mention: use Pino, not Winston.
Doc earlier cites Pino; fix this section for consistency.-Explore Winston logging configuration, structured logging, and monitoring strategies. +Explore Pino logging configuration, structured logging, and monitoring strategies.apps/lfx-one/src/app/shared/components/avatar/avatar.component.ts (1)
22-22: Avoid null default for style input.
Prefer undefined to prevent “null” leaking into DOM bindings.- public readonly style = input<AvatarProps['style']>(null); + public readonly style = input<AvatarProps['style']>(undefined as unknown as AvatarProps['style']);apps/lfx-one/src/server/services/etag.service.ts (1)
19-31: Consider HEAD for ETag fetch when supported.
Fetching via HEAD can reduce payload when only the ETag is needed.apps/lfx-one/src/app/shared/pipes/recurrence-summary.pipe.ts (1)
52-55: Harden parsing of weekly_days to avoid NaN entries.If malformed data sneaks in (e.g., stray commas/spaces),
parseInt(...) - 1can produceNaN. Filter and bounds-check before subtracting.Apply this diff:
- let weeklyDaysArray: number[] = []; - if (recurrence.weekly_days) { - weeklyDaysArray = recurrence.weekly_days.split(',').map((d) => parseInt(d.trim()) - 1); - } + let weeklyDaysArray: number[] = []; + if (recurrence.weekly_days) { + weeklyDaysArray = recurrence.weekly_days + .split(',') + .map((d) => Number(d.trim())) + .filter((n) => Number.isInteger(n) && n >= 1 && n <= 7) + .map((n) => n - 1); + }apps/lfx-one/src/app/modules/project/meetings/components/meeting-recurrence-pattern/meeting-recurrence-pattern.component.ts (1)
71-73: Use numeric comparator with sort (nit).Explicit comparator avoids lexicographic surprises and future regressions if values exceed single digits.
Apply this diff:
- newDays = [...currentDays, dayIndex].sort(); + newDays = [...currentDays, dayIndex].sort((a, b) => a - b);apps/lfx-one/src/server/routes/permissions.route.ts (2)
52-56: Incorrect operation label in validation error payload.This is a create call, not get. Fixing improves traceability.
Apply this diff:
- const validationError = ServiceValidationError.forField('projectId', 'Project ID is required', { - operation: 'get_project_permissions', + const validationError = ServiceValidationError.forField('projectId', 'Project ID is required', { + operation: 'create_user_permissions', service: 'permissions_route', path: req.path, });
61-75: Consider using ServiceValidationError for 400 responses to keep error format consistent.Current 400s return ad-hoc JSON. Using
ServiceValidationError.fromFieldErrors(...)would standardize client handling.If you want, I can provide a small refactor patch to replace these blocks with
ServiceValidationErrorfor uniform API errors. Confirm preference.Also applies to: 106-121
.github/workflows/e2e-tests.yml (1)
288-291: Correct the skipped‑secrets message for AI config.The workflow uses GitHub secrets AI_API_KEY and AI_PROXY_URL, not an AWS “ai_config” secret. Update the message to avoid confusion.
Apply this diff:
- echo " - /cloudops/managed-secrets/ai/ai_config (AI configuration)" + echo " - AI_API_KEY (GitHub Secret)" + echo " - AI_PROXY_URL (GitHub Secret)"charts/lfx-v2-ui/README.md (1)
1-3: Header/intro rebrand — LGTM; add note about PCC_ env vars being legacy but retained.*Most of the doc still uses PCC_* variables and “PCC Application Configuration.” Add a brief note near the top clarifying these names are legacy and intentionally preserved for compatibility, plus any deprecation plan.
docs/architecture/backend/ai-service.md (1)
5-6: Model/proxy note — clarify compatibility with Helm defaultsHelm README defaults AI_PROXY_URL to 'https://api.openai.com/v1/chat/completions' (charts/lfx-v2-ui/README.md lines 44–46) and the params note says "OpenAI compatible" (lines 155–156). Since docs/architecture/backend/ai-service.md describes Claude Sonnet 4 via LiteLLM (lines 5–6), add a single sentence to that doc and update the Helm README: state explicitly that an OpenAI‑compatible proxy (e.g., LiteLLM) is required for Anthropic models and AI_PROXY_URL must point to that proxy (not api.openai.com); replace the README default or show a proxy example/warning.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-resources-summary/meeting-resources-summary.component.ts (1)
182-185: Harden filename validation (reject slashes/control chars).Prevents odd paths like “a/b.pdf”, backslashes, and control chars.
- if (file.name.includes('..') || file.name.startsWith('.')) { + if (/[\/\\\x00-\x1F]/.test(file.name) || file.name.includes('..') || file.name.startsWith('.')) { return `Invalid filename "${file.name}". Filename cannot contain path traversal characters or start with a dot.`; }docs/architecture/shared/package-architecture.md (1)
65-71: Docs promote barrel imports; align with repo guidance (prefer direct paths).Switch examples to category paths to avoid barrels.
-// Main export (all interfaces) -import { User, AuthContext, AvatarProps } from '@lfx-one/shared'; +// Prefer category exports +import { User, AuthContext, AvatarProps } from '@lfx-one/shared/interfaces'; -// Specific exports -import { User, AuthContext } from '@lfx-one/shared/interfaces'; -import { lfxColors } from '@lfx-one/shared/constants'; +// Specific exports +import { User, AuthContext } from '@lfx-one/shared/interfaces'; +import { lfxColors } from '@lfx-one/shared/constants'; -// Angular component -import { User, AvatarProps } from '@lfx-one/shared/interfaces'; +// Angular component +import { User, AvatarProps } from '@lfx-one/shared/interfaces'; -// Express server -import { User, AuthContext } from '@lfx-one/shared/interfaces'; +// Express server +import { User, AuthContext } from '@lfx-one/shared/interfaces';Also applies to: 226-228, 243-243
apps/lfx-one/src/app/app.component.ts (1)
40-40: Fix comment typo.“auth s tate” → “auth state”.
- // Hydrate the auth s tate from the server, if it exists, otherwise set it to false and null + // Hydrate the auth state from the server, if it exists; otherwise set it to false and nullapps/lfx-one/src/app/modules/project/meetings/components/recurring-meeting-edit-options/recurring-meeting-edit-options.component.ts (1)
46-50: Optional: strongly type the reactive form.Improves safety when reading
editType.- public readonly editForm: FormGroup = this.initializeEditForm(); + public readonly editForm: FormGroup<{ editType: FormControl<'single' | 'future'> }> = this.initializeEditForm(); - private initializeEditForm(): FormGroup { - return new FormGroup({ - editType: new FormControl('single'), - }); - } + private initializeEditForm(): FormGroup<{ editType: FormControl<'single' | 'future'> }> { + return new FormGroup({ + editType: new FormControl<'single' | 'future'>('single', { nonNullable: true }), + }); + }apps/lfx-one/src/app/shared/components/time-picker/time-picker.component.ts (1)
8-11: Use LFX wrapper Button/InputText in this component; leave Popover as PrimeNG.Replace primeng ButtonModule and InputTextModule in apps/lfx-one/src/app/shared/components/time-picker/time-picker.component.ts (import lines ~8–11 and the imports array at ~26) with the existing ButtonComponent and InputTextComponent to preserve LFX theming/consistency. No shared Popover wrapper was found in the repo — keep PopoverModule/Popover usage.
apps/lfx-one/src/app/shared/services/activity.service.ts (1)
6-6: Import rename is fine; consider using a logging service instead of console.error.Swapping console.error for the project’s logger improves consistency and observability.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
261-265: Avoid deprecated unescape and ensure Unicode‑safe Base64.Replace unescape/encodeURIComponent/btoa combo with TextEncoder-based encoding.
Apply this diff:
- const encodedName = btoa(unescape(encodeURIComponent(displayName))); + const utf8 = new TextEncoder().encode(displayName); + const encodedName = btoa(String.fromCharCode(...utf8));
17-17: Prefer subpath imports over the package barrel for tree‑shaking and clarity.Import types from @lfx-one/shared/interfaces and utilities from their subpath (e.g., validators/utils) instead of '@lfx-one/shared'.
Apply this diff:
-import { extractUrlsWithDomains, Meeting, Project, User } from '@lfx-one/shared'; +import { Meeting, Project, User } from '@lfx-one/shared/interfaces'; +import { extractUrlsWithDomains } from '@lfx-one/shared/validators' /* or the correct subpath where it lives */;Please confirm the correct subpath for extractUrlsWithDomains within the shared package.
docs/architecture/shared/development-workflow.md (2)
56-61: Align Yarn/Node versions with root package.json.Docs show packageManager [email protected] and engines.node >=18, but root uses [email protected] and node >=22. Update examples to avoid on‑boarding confusion.
Apply:
// package.json { - "workspaces": ["apps/*", "packages/*"], - "packageManager": "[email protected]" + "workspaces": ["apps/*", "packages/*"], + "packageManager": "[email protected]" }// Root package.json (example block) "engines": { - "node": ">=18" + "node": ">=22" }Also applies to: 259-267
367-373: Use the correct Turbo cache-clearing command.“npx turbo prune” is for workspace pruning, not cache clearing. Use “turbo clean” here.
-# Clear Turborepo cache -npx turbo prune +# Clear Turborepo cache +npx turbo cleandocs/architecture.md (3)
176-179: Resolve guidance conflict: direct PrimeNG vs wrapper components.Top section says “Direct PrimeNG integration,” while later sections mandate LFX wrapper components (and past learnings prefer wrappers). Make the guidance consistent: recommend wrappers everywhere and reserve direct PrimeNG only inside wrappers.
- - **Direct PrimeNG integration** with custom LFX theming + - **Use LFX wrapper components**; avoid importing PrimeNG directly in app codeAlso applies to: 556-666
914-925: Environment variable prefix still “PCC_*”. Consider alias or rename plan.Rebrand is complete elsewhere, but envs remain PCC_ (e.g., PCC_BASE_URL). Either:
- keep PCC_* for backward compatibility and document it, or
- add support for LFX_ONE_* with PCC_* as fallback and set a deprecation window.
// server.ts (example) const BASE_URL = process.env['LFX_ONE_BASE_URL'] ?? process.env['PCC_BASE_URL'] ?? 'http://localhost:4200';Also applies to: 1063-1066
1051-1079: Port consistency: 4000 vs 4200.Docs and PM2 examples often use 4200, but code samples default to 4000. Align examples or add a note that PORT controls the runtime port and default is 4000.
docs/troubleshooting.md (1)
174-182: Forward Turbo flags correctly through Yarn.To pass flags to Turbo via Yarn script, use “--”.
-# Force rebuild without cache -yarn build --force +# Force rebuild without cache +yarn build -- --forcedocs/deployment.md (2)
255-267: Repo path after clone likely incorrect.“cd lfx-one” probably should be the repo directory (e.g., lfx-v2-ui). Update to avoid copy-paste errors.
-git clone <your-repo> -cd lfx-one +git clone <your-repo> +cd lfx-v2-ui
97-112: Port defaults vs examples.PM2 env sets PORT=4200 while startServer defaults to 4000. Add a one-liner clarifying that PORT controls runtime and examples use 4200.
Also applies to: 121-126
docs/architecture/backend/authentication.md (1)
11-25: PCC_ env variables under new branding—document compatibility or add alias.*Keep PCC_* for now but document the decision and (optionally) support LFX_ONE_* with PCC_* fallback.
Also applies to: 31-33
apps/lfx-one/e2e/project-dashboard.spec.ts (1)
42-42: Title assertion updated — OK; consider making it case-insensitive (optional)Keeps tests resilient if minor casing/prefix changes happen.
Apply this diff if you agree:
- await expect(page).toHaveTitle('LFX One'); + await expect(page).toHaveTitle(/LFX One/);apps/lfx-one/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
15-15: Imports migrated — consider typing visibility with enum to avoid string literalsUsing MeetingVisibility improves safety where 'public'/'private' strings are used.
Minimal changes:
+import { MeetingVisibility } from '@lfx-one/shared/enums';Example updates elsewhere in this file (illustrative):
// options const VISIBILITY_OPTIONS: { label: string; value: MeetingVisibility | null }[] = [ { label: 'All Visibilities', value: null }, { label: 'Public', value: MeetingVisibility.PUBLIC }, { label: 'Private', value: MeetingVisibility.PRIVATE }, ]; // filters filtered = filtered.filter((m) => m.visibility === MeetingVisibility.PUBLIC); // colors backgroundColor: m.visibility === MeetingVisibility.PUBLIC ? '#3b82f6' : '#6b7280',apps/lfx-one/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
13-13: Import Committee from its concrete interface file (avoid barrel).Committee is declared in packages/shared/src/interfaces/committee.interface.ts — update apps/lfx-one/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (line 13) to:
import { Committee } from '@lfx-one/shared/interfaces/committee.interface'apps/lfx-one/src/app/modules/project/meetings/components/meeting-registrants/meeting-registrants.component.ts (1)
16-17: Prefer LFX wrapper over direct PrimeNG where available.If a wrapper exists for ConfirmDialog/ConfirmationService, use it for consistency; otherwise ignore.
Do we have LFX wrappers for ConfirmDialog in this repo?
apps/lfx-one/src/server/server.ts (2)
153-168: Env var names still use PCC_ — confirm intent; optionally support LFX_ONE_ fallbacks.**If branding requires envs to change, add backward‑compatible fallbacks now to avoid breaking deployments.
Proposed minimal shim:
-const authConfig: ConfigParams = { +const BASE_URL = process.env['LFX_ONE_BASE_URL'] ?? process.env['PCC_BASE_URL'] ?? 'http://localhost:4000'; +const AUTH0_CLIENT_ID = process.env['LFX_ONE_AUTH0_CLIENT_ID'] ?? process.env['PCC_AUTH0_CLIENT_ID'] ?? '1234'; +const AUTH0_ISSUER_BASE_URL = process.env['LFX_ONE_AUTH0_ISSUER_BASE_URL'] ?? process.env['PCC_AUTH0_ISSUER_BASE_URL'] ?? 'https://example.com'; +const AUTH0_SECRET = process.env['LFX_ONE_AUTH0_SECRET'] ?? process.env['PCC_AUTH0_SECRET'] ?? 'sufficiently-long-string'; +const AUTH0_CLIENT_SECRET = process.env['LFX_ONE_AUTH0_CLIENT_SECRET'] ?? process.env['PCC_AUTH0_CLIENT_SECRET'] ?? 'bar'; +const AUTH0_AUDIENCE = process.env['LFX_ONE_AUTH0_AUDIENCE'] ?? process.env['PCC_AUTH0_AUDIENCE'] ?? 'https://example.com'; +const authConfig: ConfigParams = { authRequired: false, auth0Logout: true, - baseURL: process.env['PCC_BASE_URL'] || 'http://localhost:4000', - clientID: process.env['PCC_AUTH0_CLIENT_ID'] || '1234', - issuerBaseURL: process.env['PCC_AUTH0_ISSUER_BASE_URL'] || 'https://example.com', - secret: process.env['PCC_AUTH0_SECRET'] || 'sufficiently-long-string', + baseURL: BASE_URL, + clientID: AUTH0_CLIENT_ID, + issuerBaseURL: AUTH0_ISSUER_BASE_URL, + secret: AUTH0_SECRET, authorizationParams: { response_type: 'code', - audience: process.env['PCC_AUTH0_AUDIENCE'] || 'https://example.com', + audience: AUTH0_AUDIENCE, scope: 'openid email profile access:api offline_access', }, - clientSecret: process.env['PCC_AUTH0_CLIENT_SECRET'] || 'bar', + clientSecret: AUTH0_CLIENT_SECRET, routes: { login: false }, }; @@ - const validatedReturnTo = validateAndSanitizeUrl(returnTo, [process.env['PCC_BASE_URL'] as string]); + const validatedReturnTo = validateAndSanitizeUrl(returnTo, [BASE_URL]); @@ - { provide: APP_BASE_HREF, useValue: process.env['PCC_BASE_URL'] }, + { provide: APP_BASE_HREF, useValue: BASE_URL },Also update docs to state supported env names.
Also applies to: 247-247, 172-190, 175-176, 183-185
126-147: Nit: autoLogging ignore includes /api/health but only /health is defined here.Either add /api/health route or remove it from the ignore for clarity.
apps/lfx-one/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
21-21: Prefer LFX wrapper for Tooltip if available.If a wrapper exists for primeng/tooltip, use it for consistency across the app.
Confirm whether an LFX Tooltip wrapper exists in this workspace.
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts (3)
11-11: Avoid barrel import; use subpath for interfacesPrefer direct subpath to match repo conventions and aid treeshaking.
-import { CreateUserPermissionRequest, PermissionLevel, PermissionScope, UserPermissionSummary } from '@lfx-one/shared'; +import { CreateUserPermissionRequest, PermissionLevel, PermissionScope, UserPermissionSummary } from '@lfx-one/shared/interfaces';
153-167: Committee list can fetch with undefined project UID; make it reactive to project signalInitialize after project is available to avoid calling the service with "undefined" and keep it reactive.
- private initCommittees(): Signal<{ label: string; value: string }[]> { - return toSignal( - this.committeeService.getCommitteesByProject(this.project()?.uid as string).pipe( - take(1), - map((committees) => - committees.map((committee) => ({ - label: committee.name, - value: committee.uid, - })) - ) - ), - { - initialValue: [], - } - ); - } + private initCommittees(): Signal<{ label: string; value: string }[]> { + return toSignal( + toObservable(this.project).pipe( + map((p) => p?.uid), + filter((uid): uid is string => !!uid), + distinctUntilChanged(), + switchMap((uid) => this.committeeService.getCommitteesByProject(uid)), + map((committees) => + committees.map((c) => ({ + label: c.name, + value: c.uid, + })) + ) + ), + { initialValue: [] } + ); + }Note: add these imports:
-import { toSignal } from '@angular/core/rxjs-interop'; +import { toObservable, toSignal } from '@angular/core/rxjs-interop';-import { map, take } from 'rxjs'; +import { distinctUntilChanged, filter, map, switchMap } from 'rxjs';
18-18: PrimeNG direct import — prefer LFX wrapper if availableIf an LFX Tooltip wrapper exists, use it for consistency.
Do we have an LFX Tooltip wrapper component/directive to replace primeng/tooltip here?
apps/lfx-one/src/server/routes/meetings.route.ts (4)
4-4: Avoid barrel import; use subpaths for constants/utilsSplit to match shared package structure.
-import { ALLOWED_FILE_TYPES, MAX_FILE_SIZE_BYTES, sanitizeFilename } from '@lfx-one/shared'; +import { ALLOWED_FILE_TYPES, MAX_FILE_SIZE_BYTES } from '@lfx-one/shared/constants'; +import { sanitizeFilename } from '@lfx-one/shared/utils';
189-196: Don’t trust client-provided file sizeAlways persist the server-computed size.
- file_size: fileSize || buffer.length, + file_size: buffer.length,
280-285: Incorrect param key in error log (uses id instead of uid)Fixes misleading telemetry.
- meeting_id: req.params['id'], + meeting_id: req.params['uid'],
406-410: Incorrect param key in error log (uses id instead of uid)Same logging fix in attachments fetch error path.
- meeting_id: req.params['id'], + meeting_id: req.params['uid'],apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts (2)
16-16: Avoid barrel import; use subpaths for utils and interfacesKeeps imports clear and prevents dragging extra surface.
-import { extractUrlsWithDomains, Meeting, MeetingAttachment, MeetingOccurrence, MeetingRegistrant } from '@lfx-one/shared'; +import { extractUrlsWithDomains } from '@lfx-one/shared/utils'; +import { Meeting, MeetingAttachment, MeetingOccurrence, MeetingRegistrant } from '@lfx-one/shared/interfaces';
20-25: PrimeNG direct modules — prefer LFX wrappers if availableStay consistent with wrapper usage across the app.
Are there LFX wrapper components/modules for Tooltip, ConfirmDialog, or AnimateOnScroll we should use here?
apps/lfx-one/src/app/modules/project/committees/components/upcoming-committee-meeting/upcoming-committee-meeting.component.ts (3)
31-39: Potential read-before-assign onupcomingMeeting.
committeescomputed is created in the constructor and readsthis.upcomingMeeting()which is only assigned inngOnInit. InitializeupcomingMeetingto a safe default.Apply:
- import { Component, computed, inject, Injector, input, OnInit, runInInjectionContext, Signal } from '@angular/core'; + import { Component, computed, inject, Injector, input, OnInit, runInInjectionContext, Signal, signal } from '@angular/core'; - public upcomingMeeting!: Signal<Meeting | null>; + public upcomingMeeting: Signal<Meeting | null> = signal<Meeting | null>(null);
69-82: Guard against empty results when filtering by committeeId.
committeeMeetings[0]can beundefined; returnnullinstead to match the declaredSignal<Meeting | null>.- return committeeMeetings.sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime())[0]; + const next = committeeMeetings + .sort((a, b) => new Date(a.start_time).getTime() - new Date(b.start_time).getTime())[0]; + return next ?? null; @@ - return meetings.filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime())[0]; + return meetings + .filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime())[0] ?? null;
60-82: Tiny readability/alloc nit: computenowonce.
Avoid repeatednew Date().getTime()calls.- const committeeMeetings = meetings.filter( - (meeting) => - new Date(meeting.start_time).getTime() > new Date().getTime() && + const now = Date.now(); + const committeeMeetings = meetings.filter( + (meeting) => + new Date(meeting.start_time).getTime() > now && meeting.committees && meeting.committees?.length > 0 && meeting.committees.some((c) => c.uid === committeeId) ); @@ - return meetings.filter((meeting) => new Date(meeting.start_time).getTime() > new Date().getTime())[0] ?? null; + const now = Date.now(); + return meetings.filter((meeting) => new Date(meeting.start_time).getTime() > now)[0] ?? null;apps/lfx-one/src/app/shared/services/committee.service.ts (1)
63-65: Return type vs behavior mismatch forgetCommitteeMember.
Signature allowsnull, but the method never emitsnull. Either change the return type toObservable<CommitteeMember>or map 404 tonull.- public getCommitteeMember(committeeId: string, memberId: string): Observable<CommitteeMember | null> { - return this.http.get<CommitteeMember>(`/api/committees/${committeeId}/members/${memberId}`); - } + public getCommitteeMember(committeeId: string, memberId: string): Observable<CommitteeMember | null> { + return this.http.get<CommitteeMember>(`/api/committees/${committeeId}/members/${memberId}`).pipe( + catchError((error) => { + if (error?.status === 404) { + return of(null); + } + return throwError(() => error); + }) + ); + }apps/lfx-one/src/server/controllers/meeting.controller.ts (1)
682-733: “Fail-fast on 403” only applies to the first item.
The helper stops early only if the first operation fails with 403; later 403s don’t short-circuit. Either update the comment to reflect behavior or implement true fail-fast for any 403.- private async processRegistrantOperations<T, R>( + private async processRegistrantOperations<T, R>( req: Request, next: NextFunction, startTime: number, operationName: string, meetingUid: string, inputData: T[], operation: (input: T) => Promise<R>, getIdentifier: (input: T, index?: number) => string ): Promise<{ results: PromiseSettledResult<R>[]; shouldReturn: boolean }> { - try { - // Process the first registrant - const firstResult = await operation(inputData[0]); - - // If the first registrant succeeds, process the remaining in parallel - let results: PromiseSettledResult<R>[]; - if (inputData.length > 1) { - const remainingResults = await Promise.allSettled(inputData.slice(1).map((input) => operation(input))); - results = [{ status: 'fulfilled', value: firstResult }, ...remainingResults]; - } else { - results = [{ status: 'fulfilled', value: firstResult }]; - } - - // Return the results and shouldReturn flag - return { results, shouldReturn: false }; - } catch (error: any) { - // Check if it's a 403 error - if so, fail fast - // This will stop the processing if a 403 error is encountered - if (error?.status === 403 || error?.statusCode === 403) { - Logger.error(req, operationName, startTime, error, { - meeting_uid: meetingUid, - identifier: getIdentifier(inputData[0], 0), - fail_fast: true, - }); - - // Send the error to the next middleware - next(error); - return { results: [], shouldReturn: true }; - } - - // For other errors, continue processing the remaining items - let results: PromiseSettledResult<R>[] = [{ status: 'rejected', reason: error }]; - - if (inputData.length > 1) { - const remainingResults = await Promise.allSettled(inputData.slice(1).map((input) => operation(input))); - results = [...results, ...remainingResults]; - } - - // Return the results and shouldReturn flag - return { results, shouldReturn: false }; - } + const results: PromiseSettledResult<R>[] = []; + for (let i = 0; i < inputData.length; i++) { + try { + const value = await operation(inputData[i]); + results.push({ status: 'fulfilled', value } as PromiseFulfilledResult<R>); + } catch (error: any) { + if (error?.status === 403 || error?.statusCode === 403) { + Logger.error(req, operationName, startTime, error, { + meeting_uid: meetingUid, + identifier: getIdentifier(inputData[i], i), + fail_fast: true, + }); + next(error); + return { results, shouldReturn: true }; + } + results.push({ status: 'rejected', reason: error } as PromiseRejectedResult); + } + } + return { results, shouldReturn: false }; }
...s/project/meetings/components/agenda-template-selector/agenda-template-selector.component.ts
Show resolved
Hide resolved
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/deployment.md (3)
40-46: Fix port mismatch across PM2, server default, and Docker.Docs currently mix 4200 (PM2/env note) and 4000 (server default + Docker). Recommend standardizing on 4000 for SSR everywhere to avoid broken deployments.
Apply this diff to align the docs:
module.exports = { apps: [ { name: 'lfx-one', script: 'apps/lfx-one/dist/lfx-one/server/server.mjs', env: { PM2: 'true', NODE_ENV: 'production', - PORT: 4200, + PORT: 4000, },-# Port: Exposes port 4200 +# Port: Exposes port 4000-# Build for specific environment -docker build --build-arg BUILD_ENV=prod -f Dockerfile -t lfx-one . +# Build for specific environment (example: prod) +docker build --build-arg BUILD_ENV=prod -f Dockerfile -t lfx-one .No change needed to:
- startServer default (4000) — already correct
- docker run -p 4000:4000 — already correct
Also applies to: 106-111, 123-126, 141-142, 148-155, 151-151 --- `199-206`: **Production vars: ENV is set to “development”.** This is misleading in a “Required Production Variables” block. Set to production. ```diff -ENV=development +ENV=production
169-176: Verify workflow rename and paths.
- docs/deployment.md references .github/workflows/deploy-dev.yml (docs/deployment.md:169) but .github/workflows contains no deploy-dev.yml — present workflows include docker-build-main.yml, docker-build-pr.yml, docker-build-tag.yml, e2e-tests.yml, etc. Restore/rename the workflow or update the docs to point to the actual file.
- Confirm the deployment workflow (and any ECR/image pushes) uses the lfx-one image/repo names and not lfx-pcc. Repo-wide search shows lfx-one in use; a potential leftover appears as a User‑Agent string "LFX-PCC-Server/1.0" in apps/lfx-one/src/server/services/api-client.service.ts — remove or document if intentional.
♻️ Duplicate comments (1)
docs/deployment.md (1)
129-129: Unify Dockerfile path examples; avoid absolute “/path/to/Dockerfile”.Prior review flagged “Dockerfile-changelog”; you replaced it, but examples are inconsistent (/, /path/to, bare). Use a single, repo‑relative example to reduce confusion.
Apply this diff:
-The application uses a multi-stage Docker build process defined in `/Dockerfile`: +The application uses a multi-stage Docker build process defined in `Dockerfile`:-# Build the image -docker build -f /path/to/Dockerfile -t lfx-one . +# Build the image +docker build -f Dockerfile -t lfx-one .(Keep the later example with
-f Dockerfileas-is.)Also applies to: 148-148, 154-154
🧹 Nitpick comments (1)
docs/deployment.md (1)
258-258: Fix repository directory cd step.After
git clone <your-repo>,cd lfx-oneis unlikely to be valid for this repo. Use a placeholder or the actual repo dir.-git clone <your-repo> -cd lfx-one +git clone <your-repo> +cd <repo-dir>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/lfx-one/e2e/project-dashboard-robust.spec.ts(2 hunks)apps/lfx-one/e2e/project-dashboard.spec.ts(2 hunks)docs/deployment.md(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/e2e/project-dashboard-robust.spec.ts
- apps/lfx-one/e2e/project-dashboard.spec.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.{ts,html} : Use LFX wrapper components instead of importing PrimeNG components directly
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (5)
docs/deployment.md (5)
12-12: LGTM on output path rename.The dist path correctly reflects the app rename to lfx-one.
75-76: PM2 commands updated consistently.Process name lfx-one is consistent across reload/logs/stop/delete.
Also applies to: 79-80, 82-86
97-97: Path reference looks correct.apps/lfx-one/src/server/server.ts matches the new app folder.
204-214: Legacy PCC_ env var names in One-branded docs — inventory & migration plan required*docs/deployment.md (lines 204–214) still documents PCC_* env vars (PCC_BASE_URL, PCC_AUTH0_*). Either document these as legacy, or rename to ONE_/LFX_ONE_ and migrate code/env usage with a deprecation window.
Run this inventory from the repo root and paste results here to confirm scope:
#!/bin/bash set -euo pipefail # search for direct process.env.PCC_* usage rg -n -uu -S 'process\.env\.(PCC_[A-Z0-9_]+)' -C2 || true # search for any PCC_ literal (docs, configs, env files) rg -n -uu -S '\bPCC_[A-Z0-9_]+\b' -C2 || true # search for ONE_/LFX_ONE_ variants rg -n -uu -S '\b(ONE_|LFX_ONE_)[A-Z0-9_]*\b' -C2 || true
179-184: ECS names updated — verify IaC & CI referencesLocation: docs/deployment.md (lines 179–184)
Confirm Cluster 'lfx-one', Service 'lfx-one-service', and log group '/ecs/lfx-one-logs' are updated in task definitions, Terraform/CloudFormation/Helm charts, and GitHub Actions env vars. Sandbox repo search did not run; manual verification required.
🧹 Deployment RemovedThe deployment for PR #87 has been removed. |
Summary
Comprehensive rename of the application from "LFX PCC" (Project Control Center) and "LFX Projects" to "LFX One" across the entire codebase for improved branding and consistency.
Changes Made
apps/lfx-pcc/toapps/lfx-one/@lfx-pcc/sharedto@lfx-one/sharedFiles Affected
Test plan
Technical Impact
package.jsonfilesJIRA: LFXV2-489
🤖 Generated with Claude Code