Allow the application to run without analytics #1802

maor-rozenfeld · 2026-01-06T11:43:40Z

linear · 2026-01-06T11:43:43Z

Copilot

Pull request overview

This PR introduces the ability to disable analytics functionality in the application by adding a new OPS_ANALYTICS_ENABLED environment variable that defaults to true. When set to false, analytics-related features are conditionally skipped.

Adds ANALYTICS_ENABLED system property with a default value of true
Conditionally registers analytics routes, seeds, and UI components based on the flag
Updates configuration files to include the new environment variable

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/server/shared/src/lib/system/system-prop.ts	Adds `ANALYTICS_ENABLED` enum value to `AppSystemProp`
packages/server/shared/src/lib/system/system.ts	Adds default value for `ANALYTICS_ENABLED` and implements `isAnalyticsEnabled()` helper
packages/server/shared/test/system/system.test.ts	Adds test coverage for the new `isAnalyticsEnabled()` function
packages/server/api/src/main.ts	Conditionally executes analytics seeding based on the enabled flag
packages/server/api/src/app/flags/flag.service.ts	Returns `undefined` for analytics public URL when analytics are disabled
packages/server/api/src/app/database/seeds/openops-analytics-seed.ts	Adds early return guard for disabled analytics
packages/server/api/src/app/authentication/authentication.controller.ts	Conditionally registers analytics routes only when enabled
packages/server/api/src/app/ai/mcp/tools-initializer.ts	Checks analytics enabled status before loading Superset tools
packages/server/api/src/app/ai/mcp/superset-tools.ts	Returns empty toolset when analytics are disabled
packages/server/api/test/unit/authentication/authentication.controller.test.ts	Adds tests verifying analytics routes are conditionally registered
packages/server/api/tsconfig.spec.json	Adds `@fastify/cookie` type declaration
packages/server/api/.env.tests	Sets `OPS_ANALYTICS_ENABLED=true` for test environment
packages/react-ui/src/app/router.tsx	Conditionally adds analytics route based on public URL flag
packages/react-ui/src/app/features/navigation/lib/menu-links-hook.ts	Conditionally includes analytics menu link when public URL is available
deploy/helm/openops/values.yaml	Adds `OPS_ANALYTICS_ENABLED: "true"` to Helm values
deploy/helm/openops/values.overrides-example.yaml	Includes example override for analytics enabled flag
deploy/docker-compose/.env.defaults	Sets default value for `OPS_ANALYTICS_ENABLED`
.env.template	Documents the new `OPS_ANALYTICS_ENABLED` environment variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/server/shared/test/system/system.test.ts

packages/server/api/test/unit/authentication/authentication.controller.test.ts

Copilot · 2026-01-06T11:44:59Z

@maor-rozenfeld I've opened a new pull request, #1803, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-06T11:45:08Z

@maor-rozenfeld I've opened a new pull request, #1804, to work on those changes. Once the pull request is ready, I'll request review from you.

The analytics test in `system.test.ts` modifies `process.env['OPS_ANALYTICS_ENABLED']` but doesn't restore it, causing potential test pollution. ## Additional Notes Wrapped test assertions in try/finally block to guarantee cleanup even on assertion failure. Maintains consistency with existing `Reflect.deleteProperty` usage in the file's `beforeEach` hook. ```typescript it('enables analytics by default and respects disabling flag', () => { const originalAnalyticsEnabled = process.env['OPS_ANALYTICS_ENABLED']; try { expect(system.isAnalyticsEnabled()).toBe(true); process.env['OPS_ANALYTICS_ENABLED'] = 'false'; expect(system.isAnalyticsEnabled()).toBe(false); } finally { if (originalAnalyticsEnabled === undefined) { Reflect.deleteProperty(process.env, 'OPS_ANALYTICS_ENABLED'); } else { process.env['OPS_ANALYTICS_ENABLED'] = originalAnalyticsEnabled; } } }); ``` ## Testing Checklist Check all that apply: - [x] I tested the feature thoroughly, including edge cases - [x] I verified all affected areas still work as expected - [x] Automated tests were added/updated if necessary - [x] Changes are backwards compatible with any existing data, otherwise a migration script is provided ## Visual Changes (if applicable) N/A  --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: maor-rozenfeld <49363375+maor-rozenfeld@users.noreply.github.com>

…#1803) The test for analytics being disabled only verified that analytics GET routes were skipped, but didn't confirm that essential POST routes (`/sign-up`, `/sign-in`, `/sign-out`) remained functional. ## Changes Enhanced `skips analytics routes when analytics are disabled` test to verify: - Analytics GET routes are not registered (existing) - All three POST authentication routes are registered with correct paths, options, and handlers (new) ```typescript // Now verifies both what's disabled and what remains active expect(app.get).not.toHaveBeenCalled(); expect(app.post).toHaveBeenCalledTimes(3); const signUpRoute = app.post.mock.calls.find(([path]) => path === '/sign-up'); expect(signUpRoute).toBeDefined(); expect(signUpRoute?.[1]).toEqual(expect.any(Object)); expect(signUpRoute?.[2]).toEqual(expect.any(Function)); // Similar checks for /sign-in and /sign-out ``` ## Additional Notes Addresses review feedback from #1802 requesting verification that core authentication functionality remains intact when analytics is disabled. ## Testing Checklist - [x] I tested the feature thoroughly, including edge cases - [x] I verified all affected areas still work as expected - [x] Automated tests were added/updated if necessary - [x] Changes are backwards compatible with any existing data, otherwise a migration script is provided ## Visual Changes (if applicable) N/A - test-only changes  --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: maor-rozenfeld <49363375+maor-rozenfeld@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/server/api/test/unit/authentication/authentication.controller.test.ts:1

The assertion checks index [2] for the handler function, but based on the registration pattern in line 111 where embedRoute?.[1] is checked for the /analytics-embed-id route (which has no options object), the handler should be at index [1], not [2]. This index should match the pattern used for the embed route.

import { FastifyInstance } from 'fastify';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T12:17:33Z

packages/server/api/src/app/flags/flag.service.ts

+        value: system.isAnalyticsEnabled()
+          ? system.get(AppSystemProp.ANALYTICS_PUBLIC_URL)
+          : undefined,


The conditional logic returns undefined when analytics is disabled, but system.get() already returns undefined when the value is not set. Consider simplifying to return system.get(AppSystemProp.ANALYTICS_PUBLIC_URL) ?? undefined when analytics is enabled, or verify if there's a scenario where the URL could be set but analytics disabled.

alexandrudanpop · 2026-01-09T09:25:21Z

packages/react-ui/src/app/features/navigation/lib/menu-links-hook.ts

+  const { data: analyticsPublicUrl } = flagsHooks.useFlag<string | undefined>(
+    FlagId.ANALYTICS_PUBLIC_URL,
+  );


this leads to an issue if ANALYTICS_ENABLED=false but someone still leaves ANALYTICS_PUBLIC_URL in .env. Since you added ANALYTICS_ENABLED I would propose to also use it here and in the router file.

https://github.com/openops-cloud/openops/blob/main/packages/server/api/src/app/flags/flag.service.ts#L230-L237

I think it's covered

what i mean it's not consistent as the check in the BE is different using the new var

cezudas · 2026-01-09T10:57:35Z

packages/react-ui/src/app/router.tsx

    FlagId.FEDERATED_LOGIN_ENABLED,
  );

+  const { data: analyticsPublicUrl } = flagsHooks.useFlag<string | undefined>(


It slightly violates rule of hooks, it still works because createRoutes() is called directly inside the component, at the top level. It is consistent with the other flag checks above.

React Hook "useHasAnalyticsAccess" is called in function "createRoutes" that is neither a React function component nor a custom React Hook function.

…alytics route and analytics menu link (#1822) Part of OPS-3284 --------- Co-authored-by: Marcelo Gonçalves <marcelo@openops.com>

Copilot

Pull request overview

Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T15:43:50Z

packages/server/api/src/app/authentication/analytics/access-service.ts

+/* eslint-disable @typescript-eslint/no-empty-function */
+
+export const analyticsAccessService = {
+  verifyUserAnalyticsAccess(_: string): void {},


The parameter name '_' is not descriptive. Use a meaningful name like 'userId' or 'openopsUserId' to match the type definition in access-service-factory.ts.

Suggested change

verifyUserAnalyticsAccess(_: string): void {},

verifyUserAnalyticsAccess(userId: string): void {},

# Conflicts: # packages/react-ui/src/app/features/navigation/lib/menu-links-hook.ts # packages/react-ui/src/app/router.tsx # packages/server/api/src/app/authentication/authentication.controller.ts

sonarqubecloud · 2026-01-09T15:47:53Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Allow the application to run without analytics

e324dc5

Copilot AI review requested due to automatic review settings January 6, 2026 11:43

Copilot AI reviewed Jan 6, 2026

View reviewed changes

packages/server/shared/test/system/system.test.ts Show resolved Hide resolved

packages/server/api/test/unit/authentication/authentication.controller.test.ts Show resolved Hide resolved

maor-rozenfeld requested review from MarceloRGonc, alexandrudanpop and cezudas January 6, 2026 11:44

Copilot AI mentioned this pull request Jan 6, 2026

Verify non-analytics routes remain registered when analytics disabled #1803

Merged

4 tasks

Copilot AI mentioned this pull request Jan 6, 2026

Add cleanup for process.env modifications in analytics test #1804

Merged

4 tasks

maor-rozenfeld and others added 5 commits January 6, 2026 12:53

Fix lint

d1eaa5f

Merge remote-tracking branch 'origin/main' into make-analytics-optional

f56b7b3

Fix lint

e19c855

maor-rozenfeld requested a review from Copilot January 6, 2026 12:16

Copilot AI reviewed Jan 6, 2026

View reviewed changes

MarceloRGonc approved these changes Jan 7, 2026

View reviewed changes

Merge branch 'main' into make-analytics-optional

0e4eeff

alexandrudanpop reviewed Jan 9, 2026

View reviewed changes

cezudas reviewed Jan 9, 2026

View reviewed changes

Fix merge conflicts and adjust FE conditional checks for including an…

5ff58e5

…alytics route and analytics menu link (#1822) Part of OPS-3284 --------- Co-authored-by: Marcelo Gonçalves <marcelo@openops.com>

maor-rozenfeld requested a review from Copilot January 9, 2026 15:42

Copilot AI reviewed Jan 9, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into make-analytics-optional

ad3f8e9

# Conflicts: # packages/react-ui/src/app/features/navigation/lib/menu-links-hook.ts # packages/react-ui/src/app/router.tsx # packages/server/api/src/app/authentication/authentication.controller.ts

	verifyUserAnalyticsAccess(_: string): void {},
	verifyUserAnalyticsAccess(userId: string): void {},

Allow the application to run without analytics #1802

Are you sure you want to change the base?

Allow the application to run without analytics #1802

Uh oh!

Conversation

maor-rozenfeld commented Jan 6, 2026

Uh oh!

linear bot commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jan 6, 2026

Uh oh!

Copilot AI commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

alexandrudanpop Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cezudas Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

alexandrudanpop Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

cezudas Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 9, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alexandrudanpop Jan 9, 2026 •

edited

Loading

cezudas Jan 9, 2026 •

edited

Loading