Skip to content

Conversation

@jsandoval81
Copy link
Collaborator

@jsandoval81 jsandoval81 commented Sep 26, 2025

Requirements List

  • New .env variables:
    • VUE_APP_ENV=local
    • VUE_APP_STATSIG_KEY=<get from Statsig account: Settings -> Keys & Environments -> Client API Keys>
  • New dependencies
    • yarn install --ignore-engines

Description List

  • Added new env vars for app environment and Statsig key
  • Updated the EnvConfig plugin to handle new env vars
  • Updated README for new env vars + some general env var doc cleanup
  • Added Statsig package dependencies
  • Add a Statsig plugin to allow injecting the Statsig client into components
  • Initialize Statsig client in main.ts
  • Handle Statsig user change in login & logout flows
  • Don't allow Statsig init to block loading for more than 2 seconds
  • Add a polling process to periodically refetch the Statsig gate config from the server
  • Add full examples of feature gate usage to the Styelguide example components
  • Inject feature checking Statsig client into some models
  • Updated CSP Lambda with Statsig domains
  • Fix some existing transient errors in the tests related to the LicenseeList & Sorting components

Testing List

  • yarn test:unit:all should run without errors or warnings
  • yarn serve should run without errors or warnings
  • yarn build should run without errors or warnings
  • For API configuration changes: CDK tests added/updated in backend/compact-connect/tests/unit/test_api.py
  • Code review
  • Testing
    • On launching the app in the browser, explore the dev tools Network tab to ensure calls to Statsig domains are successful
      • Note that Statsig uses a variety of different root domains - seen in the CSP Lambda updates
    • Navigate the app to make sure things are generally stable and there are no indications of regressions or Statsig-related issues
    • Navigate to the /Styleguide page and scroll to the bottom to see the Example Feature Gates section
      • All of the different flag evaluations should be in the expected Enabled / Disabled state depending on the state of the flag(s) in Statsig
      • Go to the Statsig web console and update the toggle status of the flag(s)
      • Back in the Styleguide page, either:
        • Wait for the Statsig refresh timer in App.ts, then navigate away and back to the Styleguide page, or
        • Refresh the Styleguide page
      • Either case should result in the Styleguide flag evaluations being updated to the new state (Enabled / Disabled)

Closes #1094

Summary by CodeRabbit

  • New Features

    • Integrated feature flags and analytics; added Example Feature Gates to the Style Guide.
    • App initializes analytics and updates user periodically; clears analytics user on logout.
    • Added new translations for feature gate labels.
  • Bug Fixes

    • Improved sorting to avoid errors when no callback is provided.
  • Style

    • Minor UI refinements adding rr-block styling across forms, cards, and confirmation views.
  • Documentation

    • Updated environment variable guidance and setup instructions.
  • Tests

    • Added tests for feature flags plugin and Example Feature Gates; updated test helpers.
  • Chores

    • Updated CI/CD, deployment configs, CSP allowlist, and added analytics dependencies.

- @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
- @todo: Set statsig user (email) in authenticated context
- @todo: Unset statsig user during logout
- @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
- @todo: Remove sample gate check & display in PublicDashboard
- @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
- @todo: Look at test coverage improvements
- @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
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Introduces 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

Cohort / File(s) Summary
CI Workflow
.github/workflows/check-webroot.yml
Adds permissions; sets VUE_APP_ENV=production and VUE_APP_MOCK_API=true in build step.
Env Config + Deployment
webroot/.env.example, webroot/README.md, webroot/src/plugins/EnvConfig/envConfig.plugin.ts, backend/compact-connect-ui-app/stacks/frontend_deployment_stack/deployment.py, backend/compact-connect-ui-app/cdk.context.*-example.json
Introduces VUE_APP_ENV and VUE_APP_STATSIG_KEY plumbing; adds app environment flags and statsigKey; propagates through CDK contexts and build env; README mappings updated.
Statsig Plugin & Bootstrap
webroot/src/plugins/Statsig/statsig.plugin.ts, webroot/src/plugins/Statsig/statsig.d.ts, webroot/src/main.ts, webroot/package.json
Adds Statsig client/plugin, Vue typings, async app bootstrap with initStatsig, registers $features/$analytics; adds @statsig dependencies.
Feature Gates Config
webroot/src/app.config.ts
Adds FeatureGates enum with EXAMPLE_FEATURE_1.
Example Feature Gates UI
webroot/src/components/StyleGuide/ExampleFeatureGates/*, webroot/src/pages/StyleGuide/StyleGuide.*, webroot/src/pages/StyleGuide/StyleGuide.less, webroot/src/locales/en.json, webroot/src/locales/es.json
Adds ExampleFeatureGates component, styles, test, and mounts in StyleGuide; adds i18n strings; adjusts StyleGuide padding.
Models wired to $features
webroot/src/models/.../*.model.ts (User, Licensee, LicenseeUser, StaffUser, License, CompactFeeConfig)
Imports StatsigClient; adds public $features field; reads from global Vue config in constructors.
Network API for gate check
webroot/src/network/data.api.ts, webroot/src/network/exampleApi/data.api.ts, webroot/src/network/mocks/mock.data.api.ts
Adds getExampleFeatureGate method to public API, example API, and mock; minor JSDoc cleanup.
App-level analytics updates
webroot/src/components/App/App.ts, webroot/src/pages/Logout/Logout.ts
Adds periodic updateAnalyticsUser interval and cleanup; triggers update after account retrieval; logout unsets analytics user asynchronously.
Tests plumbing
webroot/tests/helpers/setup.ts, webroot/src/plugins/Statsig/statsig.spec.ts, webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.spec.ts, webroot/src/components/Licensee/LicenseeList/LicenseeList.spec.ts
Injects Statsig client mock into test mounts; adds Statsig plugin tests; adds ExampleFeatureGates test; switches one test to mountShallow.
CSP and snapshots
backend/.../cloudfront-csp/index.js, backend/.../cloudfront-csp/test/index.test.js, backend/.../tests/resources/snapshots/*UI_DISTRIBUTION*.json
Extends CSP connect-src with Statsig domains; updates tests and snapshots, including Lambda version refs and ZipFile embedding.
Minor UI/class tweaks
webroot/src/components/*/*.vue (LicenseCard, MilitaryAffiliationInfoBlock, PaymentProcessorConfig, PrivilegeCard, PrivilegePurchaseInformationConfirmation, UserAccount, .../LicensingDetail.vue), webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts
Adds rr-block CSS class to various fields/elements and AcceptUI DOM nodes; presentation-only changes.
Guarded callback
webroot/src/components/Lists/Sorting/Sorting.ts
Calls sortChange only if it is a function.
Typings/Header change
webroot/src/plugins/API/api.d.ts
Updates file header comment.
Backend README
backend/compact-connect-ui-app/lambdas/nodejs/README.md
Updates lint/test commands.

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
Loading
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
Loading
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
Loading
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)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • jlkravitz
  • isabeleliassen
  • landonshumway-ia

Poem

Bunny flips the feature switch with glee,
Gates go “on?” “off?” — we’ll see!
Stats hop in, with tracks to trace,
CSP widens, a safer space.
Models nibble flags so neat,
Logs out softly—no thumping feet.
Ship it quick—carrot-treat! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning Numerous CSS class additions of “rr-block” across unrelated UI components (LicenseCard, PaymentProcessorConfig, PrivilegeCard, PrivilegePurchaseAcceptUI, PrivilegePurchaseInformationConfirmation, UserAccount, LicensingDetail, etc.) and styling tweaks in StyleGuide are not mentioned in the feature flag objectives and appear outside the scope of the linked issue. Please remove or isolate the unrelated CSS class updates and presentational changes into a separate pull request, keeping this one focused solely on the Statsig feature flag integration.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Frontend/feature flags” accurately references the introduction of feature flags into the frontend codebase but is somewhat generic and could better highlight the specific Statsig integration and environment configuration changes.
Linked Issues Check ✅ Passed The changes comprehensively implement the objectives from issue #1094 by integrating the Statsig client and environment variables, updating the EnvConfig and CSP, initializing and polling feature gates in the app lifecycle, handling login/logout updates, and adding Styleguide examples with matching tests.
Description Check ✅ Passed The pull request description follows the repository template by including clear Requirements, Description, and Testing sections, matching the expected headings and providing the necessary details such as environment variables, feature list, and verification steps; non-critical template items like API endpoint spec updates are omitted appropriately since no endpoint changes were made.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 local when VUE_APP_ENV isn’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.ts but filename is statsig.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/$analytics before app.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/$analytics writes 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 = true and exposing window.Vue/app should 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/$tm
webroot/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 email to the analytics provider. Consider moving it to privateAttributes (evaluated locally, not logged server‑side) and bail early if $analytics is 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 from window.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

📥 Commits

Reviewing files that changed from the base of the PR and between ab45efa and 0b2184d.

⛔ Files ignored due to path filters (1)
  • webroot/yarn.lock is 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 blows

Thanks for adding the typeof === 'function' guard here (and in the direction handler). It keeps the component resilient when tests or demos stub sortChange with null/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
Set VUE_APP_MOCK_API: false and add a public Statsig environment key via VUE_APP_STATSIG_KEY from GitHub Secrets. initStatsig will only use the mock client when isTest or isUsingMockApi is 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 inherits public $features?: StatsigClient | null from User.model.ts, so this.$features is 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 $features from 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.

$features and $analytics typed as StatsigClient enable 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 $features access.

webroot/src/components/StyleGuide/ExampleFeatureGates/ExampleFeatureGates.vue (1)

8-76: LGTM: clear, self-documenting demo UI.

Good showcase across component/store/model/network layers.

- @todo: Look at test coverage improvements
- @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
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 on appEnvironments.

Without as const, consumers only see string for 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2184d and 104e795.

📒 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 explicit Promise<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 applied

Adding rr-block to 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 point

Applying rr-block to 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 styling

Extending rr-block to 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 cleanly

The 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 flags

Auto-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 / statsigEnableSessionReplay in EnvConfig and default them to false.

@jsandoval81 jsandoval81 marked this pull request as draft September 26, 2025 17:28
- Improve test coverage
- @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
- Minor test update
- @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
@jsandoval81
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@jsandoval81 jsandoval81 marked this pull request as ready for review September 29, 2025 23:15
@jsandoval81 jsandoval81 marked this pull request as draft September 29, 2025 23:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 await on getStatsigClientMock (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

📥 Commits

Reviewing files that changed from the base of the PR and between 104e795 and 5de9fba.

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

- Remove nodemailer dependency from CSP Lambda
- @todo: Work with backend to set new frontend env vars in CDK build
- @todo: Work with backend to make sure CSP Lambda hash builds as expected
- Add github permission settings to check-webroot workflow config
- @todo: Work with backend to set new frontend env vars in CDK build
- @todo: Work with backend to make sure CSP Lambda hash builds as expected
- 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
@jsandoval81 jsandoval81 marked this pull request as ready for review October 10, 2025 18:24
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.com and beyondwickedmapping.org are 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 comment

The same unnecessary connect-src host 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5de9fba and ba39543.

📒 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_key and app_env fields 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).

Copy link
Collaborator

@landonshumway-ia landonshumway-ia left a 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 👍

@jsandoval81 jsandoval81 requested a review from jlkravitz October 10, 2025 18:46
@jsandoval81
Copy link
Collaborator Author

@jlkravitz This is ready for your review.

Copy link
Collaborator

@jlkravitz jlkravitz left a 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.

@jsandoval81 jsandoval81 requested a review from jlkravitz October 13, 2025 16:01
Copy link
Collaborator

@jlkravitz jlkravitz left a 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!

@isabeleliassen isabeleliassen merged commit a6e6bfa into csg-org:development Oct 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature flag FE implementation

5 participants