-
Notifications
You must be signed in to change notification settings - Fork 7
Frontend/feature flags #1114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Frontend/feature flags #1114
Conversation
- @todo: Figure out mock implementation for tests & mock API - @todo: Update README for new env vars - @todo: Set statsig user (email) in authenticated context - @todo: Unset statsig user during logout - @todo: Update CSP Lambda tests - @todo: Work with backend to set new env vars in CDK build - @todo: Work with backend to make sure CSP Lambda hash builds as expected
WalkthroughIntroduces frontend feature-flag/analytics integration via Statsig: new plugin, env keys, app bootstrap updates, model wiring, store/action, example UI, tests, and translations. Deployment and CSP updated to include Statsig configuration/domains. Adds periodic analytics user updates and logout cleanup. Minor guards, workflow/env additions, and several presentational rr-block class tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant B as Browser
participant V as Vue App
participant S as Statsig Plugin
participant R as Router
participant N as Network (dataApi)
B->>V: Load main.ts
activate V
V->>S: initStatsig()
alt Live client ready (within timeout)
S-->>V: StatsigClient
else Fallback
S-->>V: StatsigClientMock
end
V->>V: app.createApp()
V->>V: app.use(envConfig, statsig, router, store, i18n, api, ...)
V->>V: app.config.globalProperties.$features/$analytics set
V->>B: mount #app
deactivate V
sequenceDiagram
autonumber
participant C as Component: ExampleFeatureGates
participant ST as Store
participant M as Model (User)
participant N as Network (exampleDataApi)
participant F as $features
C->>ST: dispatch(styleguide/getFeatureGateExample)
ST->>F: checkGate(EXAMPLE_FEATURE_1)
F-->>ST: boolean
ST-->>C: boolean
C->>M: user.features.checkGate(EXAMPLE_FEATURE_1)
M-->>C: boolean
C->>N: getExampleFeatureGate()
N->>F: checkGate(EXAMPLE_FEATURE_1)
F-->>N: boolean
N-->>C: boolean
sequenceDiagram
autonumber
participant A as App.vue
participant F as $analytics
participant T as Timer
A->>A: created()
A->>A: setFeatureGateRefetchInterval(60s)
T-->>A: interval tick
A->>F: updateUserAsync(userPayload)
F-->>A: (resolve or error swallowed)
A->>A: beforeUnmount() → clear interval
sequenceDiagram
autonumber
participant L as Logout Page
participant F as $analytics
participant S as Store/Auth
L->>F: updateUserAsync({}) (fire-and-forget)
L->>L: stashWorkingUri()
L->>S: logoutRequest(authType)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (25)
webroot/src/components/Licensee/LicenseeList/LicenseeList.spec.ts (1)
25-26: Remove async from describe callback.Mocha ignores promises returned by describe; keep async only on tests.
-describe('LicenseeList component', async () => { +describe('LicenseeList component', () => {webroot/package.json (1)
16-18: Add privacy guards for analytics/session replay and consider lazy‑loading.
- @statsig/web-analytics and @statsig/session-replay can capture sensitive data if misconfigured. Ensure explicit opt‑in, masking, and consent checks before activation; disable auto‑capture by default. Based on learnings.
- To keep bundle size lean, dynamically import these modules only when enabled.
Example (outside this file) to lazy‑load with explicit enablement:
// e.g., in statsig.plugin.ts or analytics bootstrap async function enableAnalyticsIfAllowed(env, consent) { if (!consent || !env.statsigKey) return; const [{ StatsigClient }, webAnalytics, sessionReplay] = await Promise.all([ import('@statsig/js-client'), import('@statsig/web-analytics'), import('@statsig/session-replay'), ]); // initialize with strict sanitization/masking here }webroot/README.md (1)
127-134: Fill in VUE_APP_STATSIG_KEY guidance and values before release.Replace TODOs with where to obtain client keys, note they’re client‑side (non‑secret), and how to set via CI/CD secrets. Also document that the app safely no‑ops when the key is absent in local/dev. Based on learnings.
I can add a short “Statsig setup” subsection with copy‑paste steps for each environment. Want me to draft it?
webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.spec.ts (1)
13-18: Optional: strengthen the assertion.Consider asserting a known text or data-testid to ensure render content, not just existence.
webroot/src/app.config.ts (1)
291-293: Feature gate enum looks good; keep IDs stable.These string IDs are external contract with Statsig; avoid renames.
If you only need compile-time types, a const object + as const can avoid emitting enum JS in bundles. Can follow up later.
webroot/src/models/Licensee/Licensee.model.ts (2)
22-22: Import Statsig type-only to avoid bundling it.Use a type-only import so the model doesn’t pull @statsig/js-client into runtime bundles.
Apply:
-import { StatsigClient } from '@statsig/js-client'; +import type { StatsigClient } from '@statsig/js-client';
66-66: Extract common globalProperties into a BaseModel
- $features?: StatsigClient | null (alongside $t/$tm) is duplicated in User.model.ts, Licensee.model.ts, License.model.ts and CompactFeeConfig.model.ts; move these into a shared BaseModel and have each model extend it to ensure a single, consistent declaration.
webroot/src/network/data.api.ts (1)
565-571: Add explicit return type for new gate method.Improve clarity and TS inference.
Apply this diff:
- public getExampleFeatureGate() { + public getExampleFeatureGate(): Promise<boolean> { return exampleDataApi.getExampleFeatureGate(); }webroot/tests/helpers/setup.ts (1)
166-189: Also set $features/$analytics on window.Vue for model/network access.Some code (models/network mocks) reads feature flags from window.Vue.config.globalProperties; set them alongside VTU mocks to avoid test flakiness.
Apply this diff in both mount helpers right after retrieving statsigClientMock:
const statsigClientMock = await getStatsigClientMock(); + // Ensure globals used by models/network are populated for tests + (window as any).Vue = (window as any).Vue || { config: { globalProperties: {} } }; + (window as any).Vue.config.globalProperties.$features = statsigClientMock; + (window as any).Vue.config.globalProperties.$analytics = statsigClientMock;Also applies to: 219-242
webroot/src/pages/Logout/Logout.ts (1)
94-98: Harden fire‑and‑forget analytics clear.Use optional chaining to avoid TypeErrors and mark call as intentionally non‑awaited.
Apply this diff:
- this.unsetAnalyticsUser(); // Not awaiting analytics so it doesn't block other critical steps + void this.unsetAnalyticsUser(); // Not awaiting analytics so it doesn't block other critical steps- async unsetAnalyticsUser(): Promise<void> { - try { - await this.$analytics.updateUserAsync({}); - } catch (err) { - // Continue - } - } + async unsetAnalyticsUser(): Promise<void> { + try { + const maybeUpdate = this.$analytics?.updateUserAsync?.({}); + await maybeUpdate; + } catch { + // continue + } + }Also applies to: 100-106
webroot/src/store/styleguide/styleguide.actions.ts (1)
8-8: Type the store action result and note global dependency.Explicit return type aids callers; ensure window.Vue globals are set in tests/runtime (addressed in setup.ts suggestion).
Apply this diff:
- getFeatureGateExample: () => { + getFeatureGateExample: (): boolean => { const { $features } = (window as any).Vue?.config?.globalProperties || {}; // Obviously store actions aren't needed to *just* check a feature gate; // This is just an example of how a feature gate can be evaluated in a store action if needed. return $features?.checkGate(FeatureGates.EXAMPLE_FEATURE_1) || false; },Also applies to: 47-54
webroot/src/plugins/EnvConfig/envConfig.plugin.ts (2)
65-65: Provide a safe default for appEnv.Default to
localwhenVUE_APP_ENVisn’t provided to avoid falling into “production” defaults elsewhere.- appEnv: context.VUE_APP_ENV, + appEnv: context.VUE_APP_ENV || appEnvironments.APP_LOCAL,
20-26: Narrow env string types for safer comparisons.Mark the env map as literal to gain type narrowing across the app.
-export const appEnvironments = { +export const appEnvironments = { APP_PRODUCTION: 'production', APP_BETA: 'beta', APP_TEST_IA: 'ia-test', APP_TEST_CSG: 'csg-test', APP_LOCAL: 'local', -}; +} as const;webroot/src/plugins/Statsig/statsig.d.ts (1)
2-2: Minor: file header name mismatch.Header says
statsigapi.d.tsbut filename isstatsig.d.ts. Consider syncing to avoid confusion.webroot/src/network/exampleApi/data.api.ts (1)
187-193: Annotate return type and keep API stable.Make the method signature explicit for callers and tests.
- public getExampleFeatureGate() { + public getExampleFeatureGate(): Promise<boolean> { const { $features } = (window as any).Vue?.config?.globalProperties || {}; // Obviously network call functions aren't needed to *just* check a feature gate; // This is just an example of how a feature gate can be evaluated in a network call if needed. return this.wait(0).then(() => $features?.checkGate(FeatureGates.EXAMPLE_FEATURE_1) || false); }webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.ts (2)
31-35: Run async checks in parallel to reduce time-to-render.No ordering dependency; execute concurrently.
- async created(): Promise<void> { - await this.setFeatureGateEnabledInStoreLayer(); - await this.setFeatureGateEnabledInModelLayer(); - await this.setFeatureGateEnabledInNetworkLayer(); - } + async created(): Promise<void> { + await Promise.all([ + this.setFeatureGateEnabledInStoreLayer(), + this.setFeatureGateEnabledInModelLayer(), + this.setFeatureGateEnabledInNetworkLayer(), + ]); + }
24-27: Consider reacting to Statsig updates.These flags are set once in created(). If gates refresh (polling), values won’t update unless something re-renders. Subscribing to the client’s update event to recompute would keep the demo live.
Based on learnings
webroot/src/plugins/Statsig/statsig.plugin.ts (1)
22-40: Default tier should be development, not production.Safer fallback avoids unintentionally enabling prod analytics/session replay.
- default: - statsigEnvironment = STATSIG_PRODUCTION; + default: + statsigEnvironment = STATSIG_DEVELOPMENT; break;webroot/src/main.ts (3)
52-58: Avoid duplicating $features/$analytics; rely on the plugin to own these globals.Manually setting
$features/$analyticsbeforeapp.use(statsig, ...)can conflict with what the plugin intends to register and makes behavior diverge over time.Apply either of these:
- Preferred: remove the manual
$features/$analyticswrites and let the plugin set them.- if (!globalProperties.$features) { - (globalProperties as any).$features = statsigClient; - } - - if (!globalProperties.$analytics) { - (globalProperties as any).$analytics = statsigClient; - } + // Rely on the statsig plugin to attach $features / $analytics
- Or, if you must keep manual assignment, install the plugin first and only set if the plugin doesn’t:
- app.use(envConfig); - app.use(statsig, { statsigClient }); + app.use(envConfig); + app.use(statsig, { statsigClient }); app.use(router); ... + if (!globalProperties.$features) (globalProperties as any).$features = statsigClient; + if (!globalProperties.$analytics) (globalProperties as any).$analytics = statsigClient;Also applies to: 66-68
31-33: Gate devtool/perf flags and global leaks to non‑prod.
app.config.performance = trueand exposingwindow.Vue/appshould be enabled only in dev/test to avoid perf overhead and leaking internals in production pages.- app.config.performance = true; + if (import.meta.env.DEV) { + app.config.performance = true; + (window as any).Vue = app; + } ... - if ((window as any).Cypress) { + if (import.meta.env.DEV && (window as any).Cypress) { (window as any).app = app; }Alternatively, use your existing envConfig flags (e.g., isLocal/isDev).
Also applies to: 60-62, 120-122
44-50: Optional: avoid shadowing i18n globals; the i18n plugin already injects $t/$tm.Unless there’s a specific reason, this duplication isn’t needed. It can also complicate typing and mocks in tests.
- if (!globalProperties.$t) { - (globalProperties as any).$t = $t; - } - if (!globalProperties.$tm) { - (globalProperties as any).$tm = $tm; - } + // Rely on i18n plugin's global injection for $t/$tmwebroot/src/components/App/App.ts (4)
24-24: Use a type‑only import for StatsigUser to avoid bundling the type at runtime.-import { StatsigUser } from '@statsig/js-client'; +import type { StatsigUser } from '@statsig/js-client';
132-148: Prefer private attributes for PII and guard missing analytics client.You’re sending
privateAttributes(evaluated locally, not logged server‑side) and bail early if$analyticsis absent.- const analyticsUser: StatsigUser = {}; + const analyticsUser: StatsigUser = {}; if (appUser?.id) { analyticsUser.userID = appUser.id; } - if (appUser?.compactConnectEmail) { - analyticsUser.email = appUser.compactConnectEmail; - } + if (appUser?.compactConnectEmail) { + analyticsUser.privateAttributes = { + ...(analyticsUser.privateAttributes || {}), + email: appUser.compactConnectEmail, + }; + } try { - await this.$analytics.updateUserAsync(analyticsUser); + if (this.$analytics?.updateUserAsync) { + await this.$analytics.updateUserAsync(analyticsUser); + } } catch (err) { // Continue }Please confirm this aligns with your data handling policies for PII. Based on learnings.
150-160: Interval lifecycle hygiene and type.
- Current clear is fine, but resetting the id after clearing avoids accidental reuse.
- Optionally type the id as
ReturnType<typeof setInterval>if you ever move away fromwindow.setInterval.- featureGateFetchIntervalId: number | undefined = undefined; + featureGateFetchIntervalId: number | undefined = undefined; // or: ReturnType<typeof setInterval> | undefined ... clearFeatureGateRefetchInterval(): void { - (window as Window).clearInterval(this.featureGateFetchIntervalId); + (window as Window).clearInterval(this.featureGateFetchIntervalId); + this.featureGateFetchIntervalId = undefined; }
129-131: Ensure Statsig user is cleared on logout to prevent stale gate evaluations.You update the user on login flows, but there’s no explicit update on logout. After pushing to Logout, set an anonymous user to flush cached gates for the prior identity.
@Watch('userStore.isLoggedInAsLicensee') async handleLicenseeLogin() { if (!this.userStore.isLoggedIn) { - this.$router.push({ name: 'Logout' }); + this.$router.push({ name: 'Logout' }); + // clear analytics user + this.updateAnalyticsUser(); // with empty user -> anonymous } else if (this.userStore.isLoggedInAsLicensee) { await this.handleAuth(); } } @Watch('userStore.isLoggedInAsStaff') async handleStaffLogin() { if (!this.userStore.isLoggedIn) { - this.$router.push({ name: 'Logout' }); + this.$router.push({ name: 'Logout' }); + // clear analytics user + this.updateAnalyticsUser(); } else if (this.userStore.isLoggedInAsStaff) { await this.handleAuth(); } }Confirm if your auth/logout flow elsewhere already calls
updateUserAsync({}). If so, this addition may be redundant.Also applies to: 213-227
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
webroot/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (37)
.github/workflows/check-webroot.yml(2 hunks)backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js(1 hunks)backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js(1 hunks)webroot/.env.example(2 hunks)webroot/README.md(1 hunks)webroot/nyc.config.js(1 hunks)webroot/package.json(1 hunks)webroot/src/app.config.ts(1 hunks)webroot/src/components/App/App.ts(4 hunks)webroot/src/components/Licensee/LicenseeList/LicenseeList.spec.ts(1 hunks)webroot/src/components/Lists/Sorting/Sorting.ts(2 hunks)webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.less(1 hunks)webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.spec.ts(1 hunks)webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.ts(1 hunks)webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.vue(1 hunks)webroot/src/locales/en.json(1 hunks)webroot/src/locales/es.json(1 hunks)webroot/src/main.ts(2 hunks)webroot/src/models/CompactFeeConfig/CompactFeeConfig.model.ts(3 hunks)webroot/src/models/License/License.model.ts(3 hunks)webroot/src/models/Licensee/Licensee.model.ts(3 hunks)webroot/src/models/LicenseeUser/LicenseeUser.model.ts(1 hunks)webroot/src/models/StaffUser/StaffUser.model.ts(1 hunks)webroot/src/models/User/User.model.ts(3 hunks)webroot/src/network/data.api.ts(3 hunks)webroot/src/network/exampleApi/data.api.ts(2 hunks)webroot/src/network/mocks/mock.data.api.ts(2 hunks)webroot/src/pages/Logout/Logout.ts(1 hunks)webroot/src/pages/StyleGuide/StyleGuide.less(1 hunks)webroot/src/pages/StyleGuide/StyleGuide.ts(2 hunks)webroot/src/pages/StyleGuide/StyleGuide.vue(1 hunks)webroot/src/plugins/API/api.d.ts(1 hunks)webroot/src/plugins/EnvConfig/envConfig.plugin.ts(4 hunks)webroot/src/plugins/Statsig/statsig.d.ts(1 hunks)webroot/src/plugins/Statsig/statsig.plugin.ts(1 hunks)webroot/src/store/styleguide/styleguide.actions.ts(2 hunks)webroot/tests/helpers/setup.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-29T17:54:57.683Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#1042
File: webroot/src/locales/es.json:958-958
Timestamp: 2025-08-29T17:54:57.683Z
Learning: Spanish translations in webroot/src/locales/es.json are currently generated using Google Translate and are considered nice-to-have. The team plans to use professional translation services if formal Spanish support is needed in the future. Don't spend time on nuanced translation quality issues for Spanish locale files.
Applied to files:
webroot/src/locales/es.json
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
Applied to files:
webroot/src/models/StaffUser/StaffUser.model.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
PR: csg-org/CompactConnect#905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/App/App.ts
🧬 Code graph analysis (8)
webroot/src/plugins/EnvConfig/envConfig.plugin.ts (1)
webroot/vue.config.js (2)
ENV_PRODUCTION(18-18)ENV_TEST(19-19)
webroot/tests/helpers/setup.ts (1)
webroot/src/plugins/Statsig/statsig.plugin.ts (1)
getStatsigClientMock(72-79)
webroot/src/plugins/Statsig/statsig.plugin.ts (2)
webroot/src/plugins/EnvConfig/envConfig.plugin.ts (1)
appEnvironments(20-26)webroot/src/pages/PublicDashboard/PublicDashboard.ts (1)
isUsingMockApi(69-71)
webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.ts (3)
webroot/src/pages/StyleGuide/StyleGuide.ts (1)
Component(16-28)webroot/src/models/User/User.model.ts (1)
User(29-86)webroot/src/network/data.api.ts (1)
dataApi(574-574)
webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.spec.ts (1)
webroot/tests/helpers/setup.ts (1)
mountShallow(265-265)
webroot/src/network/data.api.ts (1)
webroot/src/network/exampleApi/data.api.ts (1)
exampleDataApi(196-196)
webroot/src/components/Licensee/LicenseeList/LicenseeList.spec.ts (1)
webroot/tests/helpers/setup.ts (1)
mountShallow(265-265)
webroot/src/main.ts (1)
webroot/src/plugins/Statsig/statsig.plugin.ts (1)
initStatsig(81-86)
⏰ 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). (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestPython
🔇 Additional comments (23)
webroot/src/components/Lists/Sorting/Sorting.ts (1)
106-108: Defensive guard avoids runtime blowsThanks for adding the
typeof === 'function'guard here (and in the direction handler). It keeps the component resilient when tests or demos stubsortChangewithnull/undefined, which would previously throw before we ever dispatch. Looks good.webroot/src/components/Licensee/LicenseeList/LicenseeList.spec.ts (1)
27-31: Switch to mountShallow is appropriate here.This reduces bootstrapping surface (Statsig/plugin wiring) while still validating render. Good use of the updated async helpers.
webroot/src/plugins/API/api.d.ts (1)
2-2: Comment header rename is fine.No functional changes; typings remain intact.
webroot/nyc.config.js (1)
20-21: Excluding statsig.plugin.ts from coverage is reasonable.Third‑party integration glue is often noisy for coverage; tests should focus on app logic.
webroot/README.md (1)
39-47: Good addition documenting VUE_APP_ENV.Clear mapping across environments; aligns with EnvConfig updates.
webroot/src/pages/StyleGuide/StyleGuide.vue (1)
16-17: StyleGuide now exercises feature gates — nice.Visibility here helps QA validate polling/refresh behavior.
webroot/.env.example (2)
3-3: Including VUE_APP_ENV in example env is helpful.Defaults to local as expected.
15-15: Provide a realistic placeholder for VUE_APP_STATSIG_KEY.Use a sample client key format (e.g., client-xxxxxxxxxxxxxxxxxxxx) or comment it out with a note. Avoid leaving bare TODOs to reduce setup friction. Based on learnings.
.github/workflows/check-webroot.yml (1)
78-93: Disable mock API in CI and inject Statsig key
SetVUE_APP_MOCK_API: falseand add a public Statsig environment key viaVUE_APP_STATSIG_KEYfrom GitHub Secrets.initStatsigwill only use the mock client whenisTestorisUsingMockApiis true, so with mock disabled and the key provided, the real StatsigClient initializes properly.webroot/src/pages/StyleGuide/StyleGuide.less (1)
12-12: LGTM: bottom padding addition is appropriate.Extra bottom space helps avoid overlap with the new StyleGuide content.
webroot/src/models/LicenseeUser/LicenseeUser.model.ts (2)
33-33: Same $features typing concern as StaffUser.Please ensure the property is declared on the base User or this class for consistency.
40-42: LGTM on wiring $features from Vue globals.Matches the pattern used for $tm/$t.
webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.less (1)
8-20: LGTM: styles are scoped and minimal.Uses existing variables; no conflicts expected.
webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.spec.ts (1)
12-12: Do not make describe async.Mocha’s describe shouldn’t be async; wrap awaits inside it/ beforeEach.
Apply:
-describe('ExampleFeatureGates component', async () => { +describe('ExampleFeatureGates component', () => {⛔ Skipped due to learnings
Learnt from: jsandoval81 PR: csg-org/CompactConnect#1042 File: webroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.spec.ts:12-12 Timestamp: 2025-08-29T18:22:23.275Z Learning: In the CompactConnect frontend codebase, async describe blocks are widely used and supported in their version of Mocha. The pattern `describe('Test Suite', async () => {` is an accepted and working approach throughout their test files, so suggestions to remove the async keyword from describe blocks should not be made for this codebase.webroot/src/models/StaffUser/StaffUser.model.ts (1)
49-58: Ignore missing $features declaration
StaffUser inheritspublic $features?: StatsigClient | nullfromUser.model.ts, sothis.$featuresis already typed and no additional declaration is needed.Likely an incorrect or invalid review comment.
webroot/src/locales/es.json (1)
1037-1045: Verified en.json mirrors es.json for styleGuide.exampleFeature keys.*webroot/src/models/User/User.model.ts (1)
11-11: Good integration of Statsig client on the model.Property typing and guarded global access look correct and non‑disruptive.
Also applies to: 32-32, 44-53
webroot/src/locales/en.json (1)
1039-1046: LGTM: i18n keys for feature gate demo.Keys are consistent and scoped under styleGuide.
webroot/src/pages/StyleGuide/StyleGuide.ts (1)
14-25: LGTM: registers ExampleFeatureGates on the page.Import and component registration look correct.
webroot/src/models/CompactFeeConfig/CompactFeeConfig.model.ts (1)
34-49: LGTM: model-level feature client injection is consistent.Reads
$featuresfrom Vue globals and stores on the model; aligns with the pattern used elsewhere.webroot/src/plugins/Statsig/statsig.d.ts (1)
10-15: LGTM: Vue globals are properly augmented.
$featuresand$analyticstyped asStatsigClientenable safe access in components.webroot/src/models/License/License.model.ts (1)
74-107: LGTM: feature client wired onto model.Non-invasive addition; preserves existing behavior and adds
$featuresaccess.webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.vue (1)
8-76: LGTM: clear, self-documenting demo UI.Good showcase across component/store/model/network layers.
backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webroot/src/network/data.api.ts (1)
565-571: Align the signature and JSDoc with the boolean return.Let’s annotate the method explicitly and lowercase the primitive in the JSDoc so tooling reads it correctly.
- * @return {Promise<Boolean>} + * @return {Promise<boolean>} */ - public getExampleFeatureGate() { + public getExampleFeatureGate(): Promise<boolean> { return exampleDataApi.getExampleFeatureGate(); }webroot/src/plugins/EnvConfig/envConfig.plugin.ts (1)
20-26: Preserve literal types onappEnvironments.Without
as const, consumers only seestringfor each value, so you lose the helpful'production' | 'beta' | …literal union. Locking the object keeps downstream type checks and autocomplete tight.-export const appEnvironments = { +export const appEnvironments = { APP_PRODUCTION: 'production', APP_BETA: 'beta', APP_TEST_IA: 'ia-test', APP_TEST_CSG: 'csg-test', APP_LOCAL: 'local', -}; +} as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js(1 hunks)backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js(1 hunks)webroot/src/components/LicenseCard/LicenseCard.vue(2 hunks)webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue(1 hunks)webroot/src/components/PaymentProcessorConfig/PaymentProcessorConfig.vue(1 hunks)webroot/src/components/PrivilegeCard/PrivilegeCard.vue(1 hunks)webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts(2 hunks)webroot/src/components/PrivilegePurchaseInformationConfirmation/PrivilegePurchaseInformationConfirmation.vue(1 hunks)webroot/src/components/UserAccount/UserAccount.vue(1 hunks)webroot/src/network/data.api.ts(3 hunks)webroot/src/network/mocks/mock.data.api.ts(2 hunks)webroot/src/pages/LicensingDetail/LicensingDetail.vue(1 hunks)webroot/src/plugins/EnvConfig/envConfig.plugin.ts(4 hunks)webroot/src/plugins/Statsig/statsig.plugin.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- webroot/src/components/PaymentProcessorConfig/PaymentProcessorConfig.vue
- webroot/src/pages/LicensingDetail/LicensingDetail.vue
- webroot/src/components/PrivilegePurchaseInformationConfirmation/PrivilegePurchaseInformationConfirmation.vue
- webroot/src/components/UserAccount/UserAccount.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js
- backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-12T22:51:48.937Z
Learnt from: rmolinares
PR: csg-org/CompactConnect#1009
File: webroot/src/pages/LicenseeProof/LicenseeProof.less:158-165
Timestamp: 2025-08-12T22:51:48.937Z
Learning: In webroot/src/pages/LicenseeProof/LicenseeProof.vue, the .max-gap elements inside .licenses-container are intentionally hard-coded as empty elements that serve as space placeholders for tablet+ screen widths. On mobile, they are hidden completely using the :empty pseudo-class. This is an intentional design pattern where the developers have full control over keeping these elements truly empty.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.vue
📚 Learning: 2025-06-11T18:47:17.329Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#847
File: webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts:91-109
Timestamp: 2025-06-11T18:47:17.329Z
Learning: In PrivilegePurchaseAcceptUI, the team requires that the AcceptUI script be appended only to the element with id `finalize-privilege-purchase-container`; falling back to `document.body` is not acceptable.
Applied to files:
webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts
📚 Learning: 2025-06-11T18:49:14.626Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#847
File: webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts:141-155
Timestamp: 2025-06-11T18:49:14.626Z
Learning: `PrivilegePurchaseAcceptUI` publishes payment results via `"success"` / `"error"` events; the visual feedback is rendered by the parent component (`PrivilegePurchaseFinalize.ts`), so the child intentionally does not set its own `errorMessage` / `successMessage`.
Applied to files:
webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts
🧬 Code graph analysis (3)
webroot/src/plugins/Statsig/statsig.plugin.ts (1)
webroot/src/plugins/EnvConfig/envConfig.plugin.ts (1)
appEnvironments(20-26)
webroot/src/network/data.api.ts (1)
webroot/src/network/exampleApi/data.api.ts (1)
exampleDataApi(196-196)
webroot/src/plugins/EnvConfig/envConfig.plugin.ts (1)
webroot/vue.config.js (2)
ENV_PRODUCTION(18-18)ENV_TEST(19-19)
⏰ 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). (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestPython
🔇 Additional comments (6)
webroot/src/network/mocks/mock.data.api.ts (1)
631-637: Add the explicitPromise<boolean>annotation.Please add the explicit return type so this stays in sync with the real DataApi signature and the prior feedback on this helper.
- public getExampleFeatureGate() { + public getExampleFeatureGate(): Promise<boolean> { const { $features } = (window as any).Vue?.config?.globalProperties || {}; // Obviously network call functions aren't needed to *just* check a feature gate; // This is just an example of how a feature gate can be evaluated in a network call if needed. return wait(0).then(() => $features?.checkGate(FeatureGates.EXAMPLE_FEATURE_1) || false); }webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
9-9: Consistent rr-block styling appliedAdding
rr-blockto the host container keeps this block aligned with the updated design system usage across the app. Looks good.webroot/src/components/LicenseCard/LicenseCard.vue (1)
72-72: License number styling sync is on pointApplying
rr-blockto the license number in both the card grid and modal keeps the presentation consistent with the new visual language. Nice catch on covering both surfaces.Also applies to: 124-124
webroot/src/components/PrivilegeCard/PrivilegeCard.vue (1)
82-82: Privilege ID now matches refreshed stylingExtending
rr-blockto the privilege ID display aligns it with the broader styling updates and maintains visual consistency. All good here.webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts (1)
160-161: AcceptUI overlay picks up rr-block cleanlyThe deferred class injection on the container plus tagging the iframe keeps the third-party UI aligned with the rest of the rr-block rollout without disturbing existing interaction hooks. Looks solid.
Also applies to: 188-189
webroot/src/plugins/Statsig/statsig.plugin.ts (1)
63-71: Gate analytics/session replay behind explicit opt-in flagsAuto-capture and session replay still light up automatically in prod/beta/test. Without an explicit consent toggle (plus redaction config), this re-introduces the privacy/compliance risk we previously called out. Please add dedicated env-driven flags (wired through
envConfig) that default to false, and only push these plugins when both the environment and the corresponding opt-in flag are true; otherwise skip installing them.Here’s one way to guard the plugins once the new config fields exist:
- if (isAppProduction || isAppBeta || isAppTest) { + const allowAutoCapture = envConfig.statsigEnableAnalytics === true; + if ((isAppProduction || isAppBeta || isAppTest) && allowAutoCapture) { plugins.push(new StatsigAutoCapturePlugin()); } - if (isAppProduction || isAppBeta || isAppTest) { + const allowSessionReplay = envConfig.statsigEnableSessionReplay === true; + if ((isAppProduction || isAppBeta || isAppTest) && allowSessionReplay) { plugins.push(new StatsigSessionReplayPlugin()); }Remember to surface
statsigEnableAnalytics/statsigEnableSessionReplayinEnvConfigand default them tofalse.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webroot/src/plugins/Statsig/statsig.plugin.ts (2)
14-14: Optional: Consider migrating from Moment to a modern alternative.Moment.js is in maintenance mode. Since this is a simple duration calculation, consider using a modern library (Luxon, date-fns) or native JavaScript for future maintenance. However, this is not blocking as Moment is already used extensively in the codebase (e.g., License.model.ts line 16).
Based on learnings.
101-113: LGTM! Await bug is fixed and timeout strategy is sound.The missing
awaitongetStatsigClientMock(line 105) has been addressed. The Promise.race pattern ensures that Statsig initialization doesn't block app loading beyond the configured timeout (2 seconds), gracefully falling back to a mock client if the live client takes too long.Minor note: When
!isLiveEnvironment, the race is between an already-resolved promise and a timeout, causing an unnecessary setTimeout to fire. This is a negligible inefficiency and does not affect correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
webroot/src/models/License/License.model.ts(3 hunks)webroot/src/models/Licensee/Licensee.model.ts(3 hunks)webroot/src/models/LicenseeUser/LicenseeUser.model.ts(1 hunks)webroot/src/models/StaffUser/StaffUser.model.ts(1 hunks)webroot/src/models/User/User.model.ts(3 hunks)webroot/src/plugins/Statsig/statsig.plugin.ts(1 hunks)webroot/src/plugins/Statsig/statsig.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- webroot/src/models/StaffUser/StaffUser.model.ts
- webroot/src/models/User/User.model.ts
- webroot/src/models/Licensee/Licensee.model.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T19:05:36.255Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
Applied to files:
webroot/src/plugins/Statsig/statsig.plugin.ts
⏰ 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: TestPython
🔇 Additional comments (13)
webroot/src/models/License/License.model.ts (1)
17-17: LGTM! Statsig client properly wired into License model.The import, property declaration, and constructor wiring follow the same pattern used across other models (Licensee, User, CompactFeeConfig, etc.) and safely handle the optional global property.
Also applies to: 74-74, 98-101
webroot/src/models/LicenseeUser/LicenseeUser.model.ts (1)
33-37: LGTM! Statsig client properly wired into LicenseeUser model.The destructuring and assignment follow the existing pattern and safely handle optional global properties with appropriate fallbacks.
webroot/src/plugins/Statsig/statsig.spec.ts (7)
8-26: LGTM! Test setup is clean and appropriate.Imports are correct, chai-match-pattern is properly registered, and the original environment is captured for proper test cleanup.
28-40: LGTM! Thorough test cleanup ensures isolation.The afterEach hook comprehensively resets all environment flags, preventing test pollution and ensuring each test runs in a clean state.
41-76: LGTM! Environment mapping tests provide comprehensive coverage.All environment tiers are tested (production, beta, test environments, local), and the unknown/empty environment default to production is validated, matching the plugin's implementation.
77-100: LGTM! Mock client tests validate both fallback modes.Tests confirm the mock client's behavior in both live fallback (all gates disabled) and non-live (enabled except specific disabled gates) modes, covering all mock client methods.
101-126: LGTM! Plugin configuration tests validate environment-specific behavior.Tests confirm that the correct plugins (auto-capture for production, session replay for beta/test) are loaded based on environment flags, and that test runners run with no plugins.
127-145: LGTM! Initialization and installer tests validate plugin contract.Tests confirm that initStatsig returns a working client and that the Vue plugin installer correctly exposes $features and $analytics on globalProperties.
146-158: LGTM! Meta-test validates cleanup effectiveness.This test confirms that the afterEach hook successfully restores the environment state, ensuring test isolation is maintained throughout the suite.
webroot/src/plugins/Statsig/statsig.plugin.ts (4)
20-41: LGTM! Environment mapping is clear and safe.The function correctly maps application environments to Statsig tiers, and the default to production for unknown environments is a conservative, safe choice.
43-57: LGTM! Mock client implementation is appropriate.The mock provides a safe fallback with two modes: live fallback (all gates disabled) and test mode (gates enabled except specific disabled gates), supporting both error scenarios and testing needs.
59-99: LGTM! Client initialization with graceful degradation.The try/catch around
initializeAsync(lines 90-96) effectively handles missing keys and other initialization failures, falling back to a mock client with a clear warning. This addresses the earlier concern about empty keys causing noisy errors.The plugin setup (lines 64-72) is environment-specific and clear. As noted in the previous review discussion, redaction and privacy configurations are subject to ongoing stakeholder discussions.
115-120: LGTM! Vue plugin installer is correctly implemented.The plugin correctly exposes the Statsig client as both
$features(for feature gates) and$analytics(for analytics) on Vue's globalProperties, following Vue 3's plugin pattern and matching the type augmentation in statsig.d.ts.
- Set new frontend env vars in CDK build - Update CSP Lambda build snapshots - Update frontend README with guidance for CDK build vars & CSP Lambda snapshots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/compact-connect-ui-app/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json (1)
162-182: Remove non-Statsig domains from CSP connect-src
cloudflare-dns.comandbeyondwickedmapping.orgare not part of Statsig’s documented service endpoints. Keeping them whitelisted unnecessarily broadens the CSP and weakens our security posture. Please drop those hosts so only the required Statsig domains stay allowed.
♻️ Duplicate comments (1)
backend/compact-connect-ui-app/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json (1)
162-182: Refer to earlier CSP domain commentThe same unnecessary
connect-srchost entries noted in the Test snapshot are present here as well; once fixed there, regenerate this snapshot.
🧹 Nitpick comments (1)
backend/compact-connect-ui-app/lambdas/nodejs/cloudfront-csp/test/index.test.js (1)
159-170: LGTM! Consider extracting domain list to reduce duplication.The test correctly mirrors the CSP domains added to the main Lambda function, ensuring test coverage remains accurate.
To reduce maintenance burden, consider extracting the Statsig domain list to a shared constant that both the Lambda and test can reference:
// In a shared constants file or at the top of index.js const STATSIG_DOMAINS = [ 'https://api.statsig.com/', 'https://featuregates.org/', 'https://statsigapi.net/', 'https://events.statsigapi.net/', 'https://api.statsigcdn.com/', 'https://featureassets.org/', 'https://assetsconfigcdn.org/', 'https://prodregistryv2.org/', 'https://cloudflare-dns.com/', 'https://beyondwickedmapping.org/', ];This would ensure consistency and make future updates simpler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/workflows/check-webroot.yml(3 hunks)backend/compact-connect-ui-app/cdk.context.beta-example.json(1 hunks)backend/compact-connect-ui-app/cdk.context.prod-example.json(1 hunks)backend/compact-connect-ui-app/cdk.context.sandbox-example.json(1 hunks)backend/compact-connect-ui-app/cdk.context.test-example.json(1 hunks)backend/compact-connect-ui-app/lambdas/nodejs/README.md(2 hunks)backend/compact-connect-ui-app/lambdas/nodejs/cloudfront-csp/index.js(1 hunks)backend/compact-connect-ui-app/lambdas/nodejs/cloudfront-csp/test/index.test.js(1 hunks)backend/compact-connect-ui-app/stacks/frontend_deployment_stack/deployment.py(3 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/SandboxUI-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/SandboxUI-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect-ui-app/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)webroot/README.md(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/compact-connect-ui-app/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/check-webroot.yml
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
backend/compact-connect-ui-app/lambdas/nodejs/README.md
25-25: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
webroot/README.md
151-151: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
152-152: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
153-153: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
155-155: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
156-156: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
157-157: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
159-159: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
160-160: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
177-177: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
178-178: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
180-180: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
184-184: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (6)
backend/compact-connect-ui-app/cdk.context.sandbox-example.json (1)
13-15: LGTM! Configuration structure is correct.The addition of
statsig_keyandapp_envfields aligns with the deployment script changes and uses appropriate example values for a sandbox environment.backend/compact-connect-ui-app/cdk.context.test-example.json (1)
16-18: LGTM! Test environment configuration is correct.The configuration additions are consistent with other environment examples and use appropriate values for a test environment.
backend/compact-connect-ui-app/cdk.context.prod-example.json (1)
16-18: LGTM! Production configuration structure is correct.The configuration additions are appropriate. Note that this is an example file—ensure that actual production deployments use real, securely-managed Statsig client keys rather than placeholder values.
backend/compact-connect-ui-app/cdk.context.beta-example.json (1)
16-18: LGTM! Beta environment configuration is correct.The configuration additions are consistent with other environment examples and use appropriate values for a beta environment.
backend/compact-connect-ui-app/tests/resources/snapshots/SandboxUI-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json (1)
5-5: LGTM! Snapshot correctly reflects Lambda code changes.This snapshot update properly captures the modified Lambda function code with the new Statsig domains in the CSP policy.
backend/compact-connect-ui-app/lambdas/nodejs/cloudfront-csp/index.js (1)
246-257: Approve Statsig domains in connect-src.All 10 domains are documented, legitimate, and required (cloudflare-dns.com is a third-party DNS domain used by Statsig).
backend/compact-connect-ui-app/stacks/frontend_deployment_stack/deployment.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend changes look good (assuming the URLS are all accurate for the front end's needs to communicate with StatSig) Thanks for updating the example files as well 👍
|
@jlkravitz This is ready for your review. |
jlkravitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great! a couple of small questions.
jlkravitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isabeleliassen good to merge!
Requirements List
VUE_APP_ENV=localVUE_APP_STATSIG_KEY=<get from Statsig account: Settings -> Keys & Environments -> Client API Keys>yarn install --ignore-enginesDescription List
main.tsTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsbackend/compact-connect/tests/unit/test_api.py/Styleguidepage and scroll to the bottom to see the Example Feature Gates sectionCloses #1094
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Tests
Chores