feat(config): implement runtime configuration injection for client IDs #223
Conversation
- Add reusable CommitteeSelectorComponent for linking committees to mailing lists - Implement Step 3 (People & Groups) with committee search and selection - Refactor settings card layout from 2-column to 3-column design - Remove attachments toggle per Groups.io API constraints - Add warning for permanent "Members only" visibility (Groups.io restriction) - Disable public visibility option when editing private lists - Enhance autocomplete component with dropdown and dataKey props - Change "Create" to "Add" button text on dashboard - Add query param navigation for edit mode stepper LFXV2-966 Signed-off-by: Asitha de Silva <[email protected]>
Implement runtime configuration injection for LaunchDarkly and DataDog client IDs, allowing a single Docker image to be deployed to any environment with different configurations passed at container startup. Changes: - Add RuntimeConfig interface in shared package - Create runtime-config.provider.ts for TransferState setup - Update feature-flag.provider.ts to use runtime config - Update server.ts to pass config via REQUEST_CONTEXT - Remove build-time LAUNCHDARKLY_CLIENT_ID from angular.json - Remove launchDarklyClientId from environment files - Update Dockerfile to remove build-time secret injection - Update GitHub workflows to remove AWS Secrets Manager steps - Add comprehensive documentation Environment variables: - LD_CLIENT_ID: LaunchDarkly client-side ID - DD_RUM_CLIENT_ID: DataDog RUM client token (future) - DD_RUM_APPLICATION_ID: DataDog RUM application ID (future) LFXV2-971 Signed-off-by: Asitha de Silva <[email protected]>
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
WalkthroughThe PR shifts LaunchDarkly client ID configuration from build-time injection via AWS Secrets Manager to runtime injection through environment variables. Configuration is now transferred from server to browser via TransferState and consumed during application initialization, eliminating build-time Docker secrets and OIDC authentication flows. Changes
Sequence DiagramsequenceDiagram
participant Server
participant SSR as Angular SSR
participant TransferState
participant Browser as Browser/Client
participant RuntimeConfig as Runtime Config Provider
participant FeatureFlags as Feature Flags Provider
Server->>Server: Read env vars (LD_CLIENT_ID, DD_*)
Server->>Server: Construct runtimeConfig object
Server->>SSR: Pass runtimeConfig in options
SSR->>TransferState: Store runtimeConfig
SSR->>Browser: Send HTML + TransferState
Browser->>RuntimeConfig: provideRuntimeConfig() initializes
RuntimeConfig->>TransferState: Hydrate runtimeConfig from SSR state
RuntimeConfig->>Browser: Make config available
Browser->>FeatureFlags: provideFeatureFlags() initializes
FeatureFlags->>RuntimeConfig: getRuntimeConfig(transferState)
RuntimeConfig-->>FeatureFlags: Return LaunchDarkly client ID
FeatureFlags->>FeatureFlags: initializeOpenFeature async
FeatureFlags->>Browser: Feature flags ready for use
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements runtime configuration injection for client-side IDs (LaunchDarkly and DataDog RUM), allowing a single Docker image to be deployed across all environments with different configurations passed as environment variables at container startup.
Key Changes:
- Implements runtime config injection using Angular's TransferState mechanism
- Removes build-time secret injection from Docker builds and GitHub workflows
- Adds comprehensive documentation for the new architecture
However, the PR includes significant unrelated changes to mailing list management features (committee selector, settings UI, stepper navigation) that should be in separate PRs.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/runtime-config.interface.ts | New interface defining runtime configuration structure |
| packages/shared/src/interfaces/index.ts | Export runtime config interface |
| packages/shared/src/interfaces/components.interface.ts | UNRELATED: Adds CommitteeSelectorOption interface |
| docs/runtime-configuration.md | Comprehensive documentation for runtime configuration |
| docs/deployment.md | Updated with new environment variables |
| docs/build-time-secrets.md | Removed obsolete documentation |
| docs/architecture/frontend/feature-flags.md | Updated to reflect runtime configuration approach |
| apps/lfx-one/src/server/server.ts | Builds runtime config from environment variables and passes via REQUEST_CONTEXT |
| apps/lfx-one/src/environments/environment*.ts | Removes build-time LAUNCHDARKLY_CLIENT_ID declarations |
| apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts | New provider that transfers config from server to browser |
| apps/lfx-one/src/app/shared/providers/feature-flag.provider.ts | Updated to use runtime config from TransferState |
| apps/lfx-one/src/app/app.config.ts | Adds provideRuntimeConfig before provideFeatureFlags |
| apps/lfx-one/angular.json | Removes LAUNCHDARKLY_CLIENT_ID from define blocks |
| apps/lfx-one/.env.example | Updates with runtime environment variables |
| Dockerfile | Removes build-time secret mount for LaunchDarkly |
| .github/workflows/*.yml | Removes AWS Secrets Manager steps and OIDC authentication |
| Mailing list components | UNRELATED: Extensive changes to committee selector, settings, navigation |
Comments suppressed due to low confidence (1)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts:226
- The extensive changes to mailing list management component including stepper state management, step navigation logic, committee imports, and form changes are unrelated to runtime configuration. These should be in a separate PR focused on mailing list management features.
import { Component, computed, inject, Signal, signal } from '@angular/core';
import { toObservable, toSignal } from '@angular/core/rxjs-interop';
import { FormControl, FormGroup, ReactiveFormsModule, Validators } from '@angular/forms';
import { ActivatedRoute, Router, RouterLink } from '@angular/router';
import { ButtonComponent } from '@components/button/button.component';
import { CommitteeSelectorComponent } from '@components/committee-selector/committee-selector.component';
import { COMMITTEE_LABEL, MAILING_LIST_TOTAL_STEPS } from '@lfx-one/shared/constants';
import { MailingListAudienceAccess, MailingListType } from '@lfx-one/shared/enums';
import { CreateGroupsIOServiceRequest, CreateMailingListRequest, GroupsIOMailingList, GroupsIOService, MailingListCommittee } from '@lfx-one/shared/interfaces';
import { markFormControlsAsTouched } from '@lfx-one/shared/utils';
import { announcementVisibilityValidator, htmlMaxLengthValidator, htmlMinLengthValidator, htmlRequiredValidator } from '@lfx-one/shared/validators';
import { MailingListService } from '@services/mailing-list.service';
import { ProjectContextService } from '@services/project-context.service';
import { ProjectService } from '@services/project.service';
import { MessageService } from 'primeng/api';
import { StepperModule } from 'primeng/stepper';
import { catchError, filter, Observable, of, switchMap, tap, throwError } from 'rxjs';
import { MailingListBasicInfoComponent } from '../components/mailing-list-basic-info/mailing-list-basic-info.component';
import { MailingListSettingsComponent } from '../components/mailing-list-settings/mailing-list-settings.component';
@Component({
selector: 'lfx-mailing-list-manage',
imports: [
ReactiveFormsModule,
RouterLink,
ButtonComponent,
StepperModule,
MailingListBasicInfoComponent,
MailingListSettingsComponent,
CommitteeSelectorComponent,
],
templateUrl: './mailing-list-manage.component.html',
styleUrl: './mailing-list-manage.component.scss',
})
export class MailingListManageComponent {
private readonly router = inject(Router);
private readonly route = inject(ActivatedRoute);
private readonly mailingListService = inject(MailingListService);
private readonly messageService = inject(MessageService);
private readonly projectContextService = inject(ProjectContextService);
private readonly projectService = inject(ProjectService);
public readonly totalSteps = MAILING_LIST_TOTAL_STEPS;
public readonly committeeLabel = COMMITTEE_LABEL;
public readonly mode = signal<'create' | 'edit'>('create');
public readonly mailingListId = signal<string | null>(null);
public readonly isEditMode = computed(() => this.mode() === 'edit');
public readonly submitting = signal<boolean>(false);
// Stepper state - internal step tracking for create mode
private readonly internalStep = signal<number>(1);
public currentStep!: Signal<number>;
public readonly form = signal<FormGroup>(this.createFormGroup());
public readonly mailingList = this.initializeMailingList();
public readonly project = computed(() => this.projectContextService.selectedProject() || this.projectContextService.selectedFoundation());
// Services state - reactively fetched when project changes
public readonly servicesLoaded = signal<boolean>(false);
public readonly availableServices = this.initializeServices();
public readonly selectedService = computed(() => this.availableServices()[0] ?? null);
// Parent service tracking for shared service creation
public readonly parentService = signal<GroupsIOService | null>(null);
public readonly needsSharedServiceCreation = computed(
() => this.parentService() !== null && this.availableServices().filter((service) => service.type === 'shared').length === 0
);
// Prefix calculation for shared services
public readonly servicePrefix = computed(() => {
if (this.needsSharedServiceCreation()) {
const project = this.project();
return project ? `${this.cleanSlug(project.slug)}` : '';
}
return this.selectedService()?.prefix || '';
});
// Max group name length accounting for prefix (total max is 34)
public readonly maxGroupNameLength = computed(() => {
const prefix = this.servicePrefix() + '-';
return 34 - prefix.length;
});
// Track form changes reactively using toSignal
public readonly formValue = toSignal(this.form().valueChanges, { initialValue: this.form().value });
// Validation computed signals
public readonly canGoPrevious = computed(() => this.currentStep() > 1);
public readonly canGoNext = computed(() => {
// Access formValue to trigger reactivity on form changes
this.formValue();
return this.currentStep() < this.totalSteps && this.canNavigateToStep(this.currentStep() + 1);
});
public readonly isFirstStep = computed(() => this.currentStep() === 1);
public readonly isLastStep = computed(() => this.currentStep() === this.totalSteps);
// Track initial public value for edit mode - Groups.io doesn't allow changing from private to public
public readonly initialPublicValue = computed(() => this.mailingList()?.public ?? null);
public constructor() {
// Initialize step based on mode - use query params in edit mode, internal signal in create mode
this.currentStep = toSignal(
this.route.queryParamMap.pipe(
switchMap((params) => {
// In edit mode, use query parameters
if (this.isEditMode()) {
const stepParam = params.get('step');
if (stepParam) {
const step = parseInt(stepParam, 10);
if (step >= 1 && step <= this.totalSteps) {
return of(step);
}
}
return of(1);
}
// In create mode, use internal step signal
return toObservable(this.internalStep);
})
),
{ initialValue: 1 }
);
}
public nextStep(): void {
const next = this.currentStep() + 1;
if (next <= this.totalSteps && this.canNavigateToStep(next)) {
if (this.isEditMode()) {
this.router.navigate([], { queryParams: { step: next } });
} else {
this.internalStep.set(next);
}
}
}
public previousStep(): void {
const previous = this.currentStep() - 1;
if (previous >= 1) {
if (this.isEditMode()) {
this.router.navigate([], { queryParams: { step: previous } });
} else {
this.internalStep.set(previous);
}
}
}
public goToStep(step: number | undefined): void {
if (step !== undefined && step >= 1 && step <= this.totalSteps) {
if (this.isEditMode()) {
// In edit mode, allow navigation to any step via query params
this.router.navigate([], { queryParams: { step } });
} else if (step <= this.currentStep()) {
// In create mode, only allow going back to previous steps
this.internalStep.set(step);
}
}
}
public onCancel(): void {
this.router.navigate(['/mailing-lists']);
}
public onSubmit(): void {
if (this.form().invalid) {
markFormControlsAsTouched(this.form());
return;
}
this.submitting.set(true);
// Determine if we need to create a shared service first
const serviceCreation$: Observable<GroupsIOService | null> =
this.needsSharedServiceCreation() && !this.isEditMode() ? this.createSharedService() : of(null);
serviceCreation$
.pipe(
switchMap((newService: GroupsIOService | null) => {
const service = newService ?? this.selectedService();
const data = this.prepareMailingListData(service);
return this.isEditMode() ? this.mailingListService.updateMailingList(this.mailingListId()!, data) : this.mailingListService.createMailingList(data);
})
)
.subscribe({
next: () => {
this.messageService.add({
severity: 'success',
summary: 'Success',
detail: `Mailing list ${this.isEditMode() ? 'updated' : 'created'} successfully`,
});
this.router.navigate(['/mailing-lists']);
},
error: (error: Error) => {
const isServiceError = error?.message?.includes('service');
this.messageService.add({
severity: 'error',
summary: 'Error',
detail: isServiceError
? 'Failed to create mailing list service for this project'
: `Failed to ${this.isEditMode() ? 'update' : 'create'} mailing list`,
});
this.submitting.set(false);
},
});
}
public isCurrentStepValid(): boolean {
return this.isStepValid(this.currentStep());
}
private createFormGroup(): FormGroup {
return new FormGroup(
{
// Step 1: Basic Information
group_name: new FormControl('', [Validators.required, Validators.minLength(3), Validators.maxLength(34), Validators.pattern(/^[a-zA-Z0-9_-]+$/)]),
description: new FormControl('', [htmlRequiredValidator(), htmlMinLengthValidator(11), htmlMaxLengthValidator(500)]),
// Step 2: Settings
audience_access: new FormControl<MailingListAudienceAccess>(MailingListAudienceAccess.PUBLIC, [Validators.required]),
type: new FormControl<MailingListType>(MailingListType.DISCUSSION_OPEN, [Validators.required]),
public: new FormControl<boolean>(true, [Validators.required]),
// Step 3: People & Groups
committees: new FormControl<MailingListCommittee[]>([]),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.html
Show resolved
Hide resolved
...lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/settings-card/settings-card.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.ts
Show resolved
Hide resolved
...e/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.html
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: 2
🤖 Fix all issues with AI agents
In
@apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.html:
- Around line 23-25: Remove the unsupported dataKey binding from the
AutoComplete template and the matching input in the component class: delete the
[dataKey]="dataKey()" attribute in autocomplete.component.html and remove the
public dataKey = input<string>() declaration (and any other uses of dataKey)
from the autocomplete.component.ts so the PrimeNG AutoComplete only uses
supported props like dropdown() and dropdownMode().
In @apps/lfx-one/src/server/server.ts:
- Around line 237-242: The runtime config falls back to an empty string for
launchDarklyClientId which can make feature flags fail silently; update the
server.ts code that builds RuntimeConfig to check process.env['LD_CLIENT_ID']
(or the launchDarklyClientId value) and if missing either emit a clear warning
via the application logger (e.g., processLogger.warn or console.warn) or fail
fast with an error, and ensure the RuntimeConfig still reflects the validated
value (RuntimeConfig and launchDarklyClientId).
🧹 Nitpick comments (3)
apps/lfx-one/.env.example (1)
79-80: Consider reordering environment variables for linter compliance.The static analysis tool suggests placing
DD_RUM_APPLICATION_IDbeforeDD_RUM_CLIENT_IDfor alphabetical ordering.📋 Suggested reordering
-DD_RUM_CLIENT_ID=your-datadog-rum-client-token DD_RUM_APPLICATION_ID=your-datadog-rum-application-id +DD_RUM_CLIENT_ID=your-datadog-rum-client-tokenapps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (1)
21-31: Consider removing async keyword for clarity.The
initializeRuntimeConfigfunction is declared asasyncbut doesn't perform any asynchronous operations. While this is harmless (and may provide future flexibility), it's clearer to omitasyncunless actually needed.♻️ Suggested simplification
-async function initializeRuntimeConfig(): Promise<void> { +function initializeRuntimeConfig(): void { const transferState = inject(TransferState); const reqContext = inject(REQUEST_CONTEXT, { optional: true }) as { runtimeConfig: RuntimeConfig; } | null; // Server-side: Store config to TransferState for browser hydration if (reqContext?.runtimeConfig) { transferState.set(RUNTIME_CONFIG_KEY, reqContext.runtimeConfig); } }apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.ts (1)
15-20: Missingdata-testidattribute for reliable test targeting.Per coding guidelines, new components should include
data-testidattributes. Consider adding adata-testidto the component's host element or ensuring the template includes appropriate test identifiers.♻️ Suggested addition
@Component({ selector: 'lfx-committee-selector', imports: [ReactiveFormsModule, AutocompleteComponent, ButtonModule], templateUrl: './committee-selector.component.html', styleUrl: './committee-selector.component.scss', + host: { '[attr.data-testid]': 'testIdPrefix()' }, })
📜 Review details
Configuration used: Organization 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 (33)
.github/workflows/docker-build-main.yml.github/workflows/docker-build-pr.yml.github/workflows/docker-build-tag.ymlDockerfileapps/lfx-one/.env.exampleapps/lfx-one/angular.jsonapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.htmlapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.scssapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/shared/components/settings-card/settings-card.component.htmlapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/environments/environment.dev.tsapps/lfx-one/src/environments/environment.prod.tsapps/lfx-one/src/environments/environment.staging.tsapps/lfx-one/src/environments/environment.tsapps/lfx-one/src/server/server.tsdocs/architecture/frontend/feature-flags.mddocs/build-time-secrets.mddocs/deployment.mddocs/runtime-configuration.mdpackages/shared/src/interfaces/components.interface.tspackages/shared/src/interfaces/index.tspackages/shared/src/interfaces/runtime-config.interface.ts
💤 Files with no reviewable changes (8)
- apps/lfx-one/src/environments/environment.prod.ts
- apps/lfx-one/src/environments/environment.staging.ts
- apps/lfx-one/src/environments/environment.dev.ts
- docs/build-time-secrets.md
- apps/lfx-one/src/environments/environment.ts
- .github/workflows/docker-build-tag.yml
- .github/workflows/docker-build-pr.yml
- .github/workflows/docker-build-main.yml
🧰 Additional context used
📓 Path-based instructions (9)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
packages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tspackages/shared/src/interfaces/runtime-config.interface.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants must be centralized in @lfx-one/shared package
Files:
packages/shared/src/interfaces/index.tspackages/shared/src/interfaces/components.interface.tspackages/shared/src/interfaces/runtime-config.interface.ts
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
packages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlpackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.scssapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.htmlpackages/shared/src/interfaces/runtime-config.interface.tsapps/lfx-one/src/app/shared/components/settings-card/settings-card.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.htmlapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
packages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tspackages/shared/src/interfaces/runtime-config.interface.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Use data-testid naming convention: [section]-[component]-[element] for hierarchical structure
Use flex + flex-col + gap-* instead of space-y-* for spacing
Files:
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.htmlapps/lfx-one/src/app/shared/components/settings-card/settings-card.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.html
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.scssapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.htmlapps/lfx-one/src/app/shared/components/settings-card/settings-card.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.htmlapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
apps/lfx-one/src/**/*.{html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX colors for styling
Files:
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.htmlapps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.scssapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.htmlapps/lfx-one/src/app/shared/components/settings-card/settings-card.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.html
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: All PrimeNG components must be wrapped in LFX components for UI library independence
Add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-10T23:20:29.281Z
Learnt from: asithade
Repo: linuxfoundation/lfx-v2-ui PR: 206
File: apps/lfx-one/src/app/modules/meetings/components/agenda-template-selector/agenda-template-selector.component.ts:11-15
Timestamp: 2025-12-10T23:20:29.281Z
Learning: For Angular 19+ projects, standalone defaults to true for components, directives, and pipes when not explicitly declared. Therefore, components with an imports array in the Component decorator do not require an explicit standalone: true declaration to be standalone. In reviews, verify that a component without an explicit standalone flag and with an imports array will behave as a standalone component according to Angular 19+ defaults. If you need to enforce explicit standalone behavior for clarity or consistency, you can still add standalone: true, but it is not required by default in Angular 19+. Ensure this guidance is considered when reviewing any component.ts files across the codebase.
Applied to files:
apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.tsapps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts
🧬 Code graph analysis (3)
apps/lfx-one/src/app/app.config.ts (1)
apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (1)
provideRuntimeConfig(37-37)
apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (1)
packages/shared/src/interfaces/runtime-config.interface.ts (1)
RuntimeConfig(9-28)
apps/lfx-one/src/server/server.ts (1)
packages/shared/src/interfaces/runtime-config.interface.ts (1)
RuntimeConfig(9-28)
🪛 dotenv-linter (4.0.0)
apps/lfx-one/.env.example
[warning] 80-80: [UnorderedKey] The DD_RUM_APPLICATION_ID key should go before the DD_RUM_CLIENT_ID key
(UnorderedKey)
⏰ 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: Agent
🔇 Additional comments (39)
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.scss (1)
1-2: Copyright header is properly formatted.The license header follows the required LFX format with proper SPDX identifier.
apps/lfx-one/src/app/shared/components/settings-card/settings-card.component.html (1)
4-4: LGTM! Minor presentational improvement.The addition of
h-fullensures the settings card fills its parent container's height, which likely supports the layout consistency for the refactored mailing list UI. The code follows all coding guidelines (Tailwind CSS usage, flex + gap-* spacing pattern, license header present).apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.html (1)
43-48: LGTM! Consistent terminology update.The update from "Create" to "Add" is applied consistently in both the comment and the button label, improving UX terminology alignment.
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-view/mailing-list-view.component.html (1)
116-116: LGTM! Query parameter correctly adds step navigation.The addition of the
step: '3'query parameter properly enables direct navigation to step 3 in the edit flow, which aligns with the stepper changes mentioned in the commit. The syntax is correct and the data-testid attribute follows the required naming convention.apps/lfx-one/.env.example (1)
69-81: Runtime configuration section well-documented.The new runtime client IDs section clearly documents the shift from build-time to runtime injection, aligning with the PR objectives.
apps/lfx-one/angular.json (1)
65-65: LGTM! Build-time secret injection successfully removed.The removal of
LAUNCHDARKLY_CLIENT_IDdefine blocks from all build configurations correctly implements the shift to runtime configuration injection, preventing secrets from being embedded in built bundles.packages/shared/src/interfaces/components.interface.ts (1)
570-581: LGTM! Well-defined interface for committee selector.The
CommitteeSelectorOptioninterface is well-documented and follows established patterns in the codebase, providing a clean type definition for committee selection functionality.apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.ts (1)
37-39: LGTM! New autocomplete inputs properly implemented.The new input properties (dropdown, dropdownMode, dataKey) are fully supported by PrimeNG Autocomplete 20.4.0 and correctly extend the component's capabilities with dropdown functionality and data key mapping, using appropriate signal-based inputs for Angular 19.
packages/shared/src/interfaces/index.ts (1)
70-72: LGTM!The new runtime config interface export follows the established pattern and is properly documented with a descriptive comment.
apps/lfx-one/src/app/app.config.ts (2)
20-20: LGTM!The import statement follows the established convention for provider imports.
53-54: LGTM!The provider ordering is correctly documented and implemented. The runtime config provider is registered before the feature flags provider, ensuring that runtime configuration is available when feature flags are initialized.
docs/deployment.md (1)
241-245: LGTM!The documentation clearly describes the runtime environment variables and correctly matches the implementation in server.ts. The distinction between runtime injection and build-time constants is well-documented, and the future placeholders for DataDog are appropriately marked.
apps/lfx-one/src/server/server.ts (2)
7-7: LGTM!The RuntimeConfig import is correctly added and maintains alphabetical ordering.
244-252: LGTM!The runtime config is correctly passed to the Angular SSR engine, enabling it to be serialized via TransferState and made available to the client-side application.
Dockerfile (2)
27-33: LGTM!The build process is correctly simplified to remove build-time secret injection. The comments clearly document that client IDs are now provided at runtime via environment variables, which aligns with the PR objectives and improves deployment flexibility.
39-39: LGTM!The CMD statement is correct and will start the SSR server with the runtime configuration provided through environment variables.
packages/shared/src/interfaces/runtime-config.interface.ts (1)
1-28: LGTM! Well-structured runtime configuration interface.The interface is clean, well-documented, and correctly located in the shared package. The JSDoc comments clearly indicate that these are publicly-publishable IDs (safe to expose in browser), which is important security context for developers using this interface.
apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.html (1)
1-91: LGTM! Clean component template with proper conventions.The template follows all coding guidelines:
- Uses
flex+flex-col+gap-*for spacing (nospace-y-*) ✓- Implements hierarchical
data-testidnaming convention ✓- Uses Tailwind CSS with PrimeUI integration ✓
- License header present ✓
apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (1)
39-45: LGTM! Clean helper function for runtime config retrieval.The
getRuntimeConfighelper provides a simple API for accessing runtime configuration from TransferState with a sensible fallback to default values.apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html (2)
95-95: LGTM! Proper input binding for edit mode support.The additions of
isEditModeandinitialPublicValueinputs correctly enable the settings component to handle edit mode behavior, particularly for disabling the public option when editing a private list.
101-116: LGTM! Well-structured People & Groups section.The new section follows all guidelines:
- Uses
flex flex-col gap-6for proper spacing ✓- Dynamic labeling with
committeeLabel.pluralfor flexibility ✓- Proper
data-testidattribute for testing ✓- All required inputs provided to
lfx-committee-selector✓apps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.ts (2)
5-5: LGTM! Clean import updates for computed property support.The addition of
computedto the imports and removal of unusedToggleComponentproperly support the new edit mode functionality.Also applies to: 14-14
19-20: LGTM! Correct implementation of edit mode restrictions.The new inputs and computed property correctly implement the business logic to prevent changing a private mailing list to public during editing (Groups.io restriction). The use of Angular 19 signals with
input()andcomputed()follows best practices.Based on learnings, Angular 19+ defaults standalone to true, so no explicit declaration is needed.
Also applies to: 24-26
docs/runtime-configuration.md (1)
1-377: Well-structured runtime configuration documentation.The documentation thoroughly covers the runtime configuration architecture, including the data flow, provider setup, environment variables, and migration guidance. The code examples align with the implementation patterns in the provider files.
Minor formatting note: Line 54 has a slight spacing inconsistency in the ASCII diagram (
app.config.tshas an extra space at the end), but this doesn't affect readability.apps/lfx-one/src/app/shared/components/committee-selector/committee-selector.component.ts (3)
127-151: Good reactive data loading pattern.The
initCommitteeOptionsmethod properly:
- Manages loading state with
tap- Filters out invalid UIDs before API calls
- Handles errors gracefully with console logging and empty array fallback
- Pre-populates filtered suggestions for immediate display
157-176: Reactive form and signal interop is well-implemented.The
initSelectedItemsmethod correctly combines form value changes with loaded options usingcombineLatestandswitchMap. ThestartWithoperator ensures the initial value is emitted immediately.
219-229: No actionable issues found. The asymmetry between multi-select and single-select modes is intentional and appropriate: theallowed_voting_statusesfield is optional in theMailingListCommitteeinterface. Multi-select mode initializes all voting statuses as a default for bulk operations, while single-select mode omits the optional field for a simpler object structure. This design pattern correctly aligns with different use cases.apps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-settings/mailing-list-settings.component.html (3)
11-12: Good responsive grid layout implementation.The three-column layout with
grid-cols-1 md:grid-cols-3provides proper responsive behavior. Usingitems-stretchensures consistent card heights.
117-120: Good implementation of edit-mode restrictions for visibility.The disabled state handling for the "Public" option correctly:
- Removes cursor interaction with
[class.cursor-pointer]="!isPublicDisabled()"- Applies visual feedback with
[class.opacity-50]="isPublicDisabled()"- Disables the actual radio button via
[disabled]="isPublicDisabled()"This properly prevents users from changing a private mailing list back to public, which aligns with Groups.io constraints.
129-144: Clear conditional messaging for visibility constraints.The amber warning for "Members only" selection and the red error for invalid announcement+private combinations provide good user feedback with appropriate color semantics.
apps/lfx-one/src/app/shared/providers/feature-flag.provider.ts (2)
16-30: Correct runtime config integration for LaunchDarkly initialization.The implementation properly:
- Guards against SSR execution with
typeof window === 'undefined'- Retrieves config from
TransferStateviagetRuntimeConfig- Gracefully handles missing
clientIdwith a warningThis aligns with the documented architecture in
docs/runtime-configuration.md.
46-50: Provider documentation note is helpful.The JSDoc comment clearly indicates that
provideRuntimeConfig()must be included before this provider. This helps prevent misconfiguration.docs/architecture/frontend/feature-flags.md (3)
184-220: Documentation accurately reflects the two-provider architecture.The updated provider setup section correctly documents:
runtime-config.provider.tsfor TransferState setupfeature-flag.provider.tsfor LaunchDarkly initialization- The correct initialization flow and dependencies
The code examples match the actual implementation in the provider files.
628-694: Comprehensive runtime configuration guidance.The documentation provides clear instructions for:
- Local development with
.envfiles- Docker deployment with
-eflags- Kubernetes deployment with
secretKeyRefThis aligns with the patterns documented in
docs/runtime-configuration.md.
1384-1415: Updated troubleshooting covers runtime config scenarios.The troubleshooting section now includes:
- Missing
.envfile for local development- Missing Docker runtime variable
- TransferState hydration issues
- Provider ordering issues
The reference to
runtime-configuration.md#troubleshootingprovides a single source of truth for detailed debugging.apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts (4)
127-159: Well-structured dual-mode navigation.The navigation methods correctly handle:
- Edit mode: URL-based navigation via query params, allowing direct access to any step
- Create mode: Signal-based navigation with linear progression enforcement
The asymmetry in
goToStep(edit allows any step, create only allows backward) is appropriate for the different use cases.
213-230: Form structure supports the three-step wizard.The form group properly defines:
- Step 1:
group_name,descriptionwith validation- Step 2:
audience_access,type,publicwith form-level validator- Step 3:
committees(optional)The
announcementVisibilityValidator()ensures business rules are enforced across fields.
100-101:initialPublicValuecorrectly tracks edit mode constraints.This computed signal provides the initial
publicvalue from the loaded mailing list, enabling the settings component to enforce the Groups.io constraint that private lists cannot be changed to public.
103-125: No timing issue: mode is correctly established before currentStep processes route changes.The dual-mode initialization is sound. While
initializeMailingList()usestoSignal(making it a lazy subscription), the critical detail is thatroute.paramMapandroute.queryParamMapboth emit synchronously during the same navigation event. ThemailingListproperty initializer (line 58) establishes its subscription before the constructor runs, ensuringmode.set('edit')is called beforecurrentStep'sswitchMapevaluatesisEditMode()during route navigation. In create mode (no:idparam),isEditMode()correctly remainsfalseand usestoObservable(this.internalStep). The implementation properly handles both modes.
apps/lfx-one/src/app/shared/components/autocomplete/autocomplete.component.html
Show resolved
Hide resolved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/lfx-one/.env.example (1)
79-80: Consider reordering for consistency.The static analysis tool suggests placing
DD_RUM_APPLICATION_IDbeforeDD_RUM_CLIENT_IDfor alphabetical ordering. While this is purely cosmetic, consistent ordering can improve readability.♻️ Proposed reordering
# DataDog RUM Browser Monitoring (future integration) # Client token and application ID for browser monitoring +DD_RUM_APPLICATION_ID=your-datadog-rum-application-id DD_RUM_CLIENT_ID=your-datadog-rum-client-token -DD_RUM_APPLICATION_ID=your-datadog-rum-application-idpackages/shared/src/interfaces/runtime-config.interface.ts (1)
9-28: Consider making @future properties optional for better API clarity.The
dataDogRumClientIdanddataDogRumApplicationIdproperties are marked as@future(not yet integrated) but are required in the interface. While the server gracefully handles missing environment variables using default empty strings (process.env['DD_RUM_CLIENT_ID'] || ''in server.ts), making these properties optional would better communicate that they're optional placeholder features:export interface RuntimeConfig { /** * LaunchDarkly client-side ID for feature flag evaluation. * This is a publicly-publishable ID (safe to expose in browser). */ launchDarklyClientId: string; /** * DataDog RUM client token for browser monitoring. * This is a publicly-publishable token (safe to expose in browser). * @future Not yet integrated - placeholder for future use */ - dataDogRumClientId: string; + dataDogRumClientId?: string; /** * DataDog RUM application ID. * @future Not yet integrated - placeholder for future use */ - dataDogRumApplicationId: string; + dataDogRumApplicationId?: string; }This aligns the type signature with the actual runtime behavior and clarifies the intent for future maintainers.
docs/runtime-configuration.md (1)
151-160: Minor clarification needed for required vs. future variables.The table marks
DD_RUM_CLIENT_IDandDD_RUM_APPLICATION_IDas "future" in the description, but the table header implies all are "Required Variables". Consider updating the header to "Environment Variables" or adding a "Status" column to clarify which are actively used vs. placeholders.📝 Suggested table update
-### Required Variables +### Environment Variables -| Variable | Description | Example | -| ----------------------- | ----------------------------------- | -------------------------- | -| `LD_CLIENT_ID` | LaunchDarkly client-side ID | `691b727361cbf309e9d74468` | -| `DD_RUM_CLIENT_ID` | DataDog RUM client token (future) | `pub123456789` | -| `DD_RUM_APPLICATION_ID` | DataDog RUM application ID (future) | `app-uuid-here` | +| Variable | Description | Required | Example | +| ----------------------- | ----------------------------------- | -------- | -------------------------- | +| `LD_CLIENT_ID` | LaunchDarkly client-side ID | Yes | `691b727361cbf309e9d74468` | +| `DD_RUM_CLIENT_ID` | DataDog RUM client token | Future | `pub123456789` | +| `DD_RUM_APPLICATION_ID` | DataDog RUM application ID | Future | `app-uuid-here` |
📜 Review details
Configuration used: Organization 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 (20)
.github/workflows/docker-build-main.yml.github/workflows/docker-build-pr.yml.github/workflows/docker-build-tag.ymlDockerfileapps/lfx-one/.env.exampleapps/lfx-one/angular.jsonapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/environments/environment.dev.tsapps/lfx-one/src/environments/environment.prod.tsapps/lfx-one/src/environments/environment.staging.tsapps/lfx-one/src/environments/environment.tsapps/lfx-one/src/server/server.tsdocs/architecture/frontend/feature-flags.mddocs/build-time-secrets.mddocs/deployment.mddocs/runtime-configuration.mdpackages/shared/src/interfaces/index.tspackages/shared/src/interfaces/runtime-config.interface.ts
💤 Files with no reviewable changes (8)
- .github/workflows/docker-build-tag.yml
- apps/lfx-one/src/environments/environment.prod.ts
- .github/workflows/docker-build-main.yml
- apps/lfx-one/src/environments/environment.ts
- docs/build-time-secrets.md
- apps/lfx-one/src/environments/environment.dev.ts
- .github/workflows/docker-build-pr.yml
- apps/lfx-one/src/environments/environment.staging.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-one/src/server/server.tspackages/shared/src/interfaces/runtime-config.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
apps/lfx-one/src/server/server.tspackages/shared/src/interfaces/runtime-config.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
apps/lfx-one/src/server/server.tspackages/shared/src/interfaces/runtime-config.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/server/server.tsapps/lfx-one/src/app/shared/providers/runtime-config.provider.tsapps/lfx-one/src/app/app.config.tsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants must be centralized in @lfx-one/shared package
Files:
packages/shared/src/interfaces/runtime-config.interface.tspackages/shared/src/interfaces/index.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/server/server.ts (1)
packages/shared/src/interfaces/runtime-config.interface.ts (1)
RuntimeConfig(9-28)
apps/lfx-one/src/app/shared/providers/feature-flag.provider.ts (1)
apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (1)
getRuntimeConfig(43-45)
🪛 dotenv-linter (4.0.0)
apps/lfx-one/.env.example
[warning] 80-80: [UnorderedKey] The DD_RUM_APPLICATION_ID key should go before the DD_RUM_CLIENT_ID key
(UnorderedKey)
⏰ 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: Agent
🔇 Additional comments (24)
apps/lfx-one/angular.json (1)
82-87: Build-time secret injection successfully removed from all configurations.The removal of
LAUNCHDARKLY_CLIENT_IDdefine entries from production, staging, development, and local build configurations is complete and correct. The remaining fileReplacements and externalDependencies are appropriate.Runtime configuration is properly implemented via
runtime-config.provider.ts, reading environment variables (LD_CLIENT_ID,DD_RUM_CLIENT_ID,DD_RUM_APPLICATION_ID) and injecting them through server-side REQUEST_CONTEXT to the browser via TransferState, eliminating build-time secret exposure.Dockerfile (1)
30-33: LGTM! Clear documentation of runtime injection.The comments clearly document the architectural shift from build-time to runtime injection of client IDs. This approach correctly enables a single Docker image to be deployed across multiple environments.
apps/lfx-one/.env.example (1)
69-81: LGTM! Clear documentation of runtime configuration.The comments effectively explain the runtime injection approach and clarify that these values are publicly-publishable client IDs, which is correct for LaunchDarkly client-side IDs and DataDog RUM tokens.
packages/shared/src/interfaces/index.ts (1)
71-72: LGTM! Follows established patterns.The export follows the existing pattern and correctly exposes the RuntimeConfig interface from the shared package, aligning with the coding guideline to centralize shared types.
docs/deployment.md (1)
241-245: LGTM! Clear and comprehensive documentation.The documentation clearly explains the runtime injection approach and provides helpful inline comments for each environment variable. The cross-reference to
docs/runtime-configuration.mdfor architecture details is a good practice.apps/lfx-one/src/server/server.ts (2)
7-7: LGTM - Import updated correctly.The import now includes
RuntimeConfigfrom the shared interfaces, properly typed for the new runtime configuration pattern.
237-247: Well-structured runtime configuration injection.The implementation correctly:
- Builds a typed
RuntimeConfigobject from environment variables- Uses empty string fallbacks allowing the feature-flag provider to handle missing configuration gracefully
- Passes the config alongside
authto the Angular SSR handler for TransferState hydrationdocs/runtime-configuration.md (2)
1-46: Excellent documentation for the runtime configuration architecture.The documentation clearly explains:
- The data flow from container startup through SSR to browser hydration
- The TransferState-based transfer mechanism
- Benefits of the single Docker image approach
The ASCII diagrams effectively illustrate the architecture.
244-309: Comprehensive guide for extending runtime configuration.The step-by-step instructions cover all necessary changes across the codebase when adding new runtime configuration values. This will be valuable for future maintainers.
apps/lfx-one/src/app/app.config.ts (2)
20-21: Import added correctly.The import follows the project's direct import pattern for standalone providers.
53-54: Correct provider ordering with clear documentation.The
provideRuntimeConfig()is correctly placed beforeprovideFeatureFlags()to ensure TransferState is populated before feature flags attempt to read the configuration. The inline comment clearly documents this dependency.apps/lfx-one/src/app/shared/providers/runtime-config.provider.ts (4)
1-5: Clean file setup with proper license header and imports.The file correctly imports Angular core utilities and the shared
RuntimeConfiginterface.
7-15: Well-defined TransferState key and default configuration.The
makeStateKeywith a descriptive name ensures consistent key usage across server and browser. The default configuration with empty strings allows consumers to handle missing values gracefully.
21-31: Server-side initialization logic is correct.The function properly:
- Uses optional injection for
REQUEST_CONTEXT(only available during SSR)- Stores the runtime config to TransferState only when present
- Enables browser hydration without requiring additional browser-side logic
One consideration: the function is marked
asyncbut doesn't useawait. This is acceptable sinceprovideAppInitializerexpects a function returningPromise<void>, but you could simplify to a sync function returningPromise.resolve()if preferred.
37-45: Clean public API for runtime configuration.The
provideRuntimeConfigandgetRuntimeConfigexports provide a well-encapsulated interface:
provideRuntimeConfig()for app configurationgetRuntimeConfig(transferState)for consuming the configuration with built-in fallback to defaultsapps/lfx-one/src/app/shared/providers/feature-flag.provider.ts (4)
4-10: Updated imports align with runtime configuration pattern.The imports correctly add
TransferStateandinjectfor accessing runtime config, while keepingenvironmentfor the production-aware logger configuration.
16-30: Robust browser-only initialization with graceful degradation.The implementation correctly:
- Skips execution on the server where LaunchDarkly isn't supported
- Retrieves the client ID from TransferState via
getRuntimeConfig- Logs a warning and exits gracefully when no client ID is configured
This ensures the application continues to function with default flag values when configuration is missing.
32-44: Proper error handling for LaunchDarkly initialization.The try-catch block ensures that initialization failures don't crash the application. The error is logged for debugging while allowing the app to continue with feature flags disabled.
46-50: Updated provider registration.The
provideAppInitializer(initializeOpenFeature)pattern correctly registers the async initialization function with Angular's app initializer system.docs/architecture/frontend/feature-flags.md (5)
184-220: Documentation updated for two-provider architecture.The new section clearly explains the separation between runtime config and feature flag providers, including the complete code examples that match the actual implementation.
266-291: App config example and provider notes updated.The documentation correctly emphasizes:
- Provider ordering requirements
- TransferState mechanism for client ID transfer
- SSR guard behavior
- Graceful degradation when configuration is missing
630-678: Configuration section updated for runtime environment variables.The documentation now clearly explains:
- Environment variable names and purposes
- Local development setup with
.envfile- Docker and Kubernetes deployment patterns
This aligns with the migration from build-time to runtime configuration.
1392-1415: Troubleshooting section updated for runtime configuration issues.The updated troubleshooting now covers common runtime config problems including:
- Missing environment variables
- TransferState hydration issues
- Provider ordering problems
The reference to the runtime configuration troubleshooting docs provides additional guidance.
1545-1562: Version and references updated appropriately.The version bump to 2.0.0 reflects the significant architectural change from build-time to runtime configuration. Related documentation links are updated to include the new runtime configuration docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Changes
Environment Variables
LD_CLIENT_IDDD_RUM_CLIENT_IDDD_RUM_APPLICATION_IDDocumentation
docs/runtime-configuration.mdwith full architecture detailsdocs/architecture/frontend/feature-flags.mddocs/deployment.mdwith new environment variablesLFXV2-971
🤖 Generated with Claude Code