Skip to content

fix(experimentation): hide the first-event button for customer warehouses on self-hosted - #8874

Merged
Zaimwa9 merged 1 commit into
mainfrom
fix/warehouse-test-event-self-hosted
Nov 5, 2026
Merged

Zaimwa9 merged 1 commit into
mainfrom
fix/warehouse-test-event-self-hosted

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

On self-hosted, "Send your first event" for ClickHouse, Databricks and bring-your-own PostgreSQL posts to Flagsmith Cloud's ingestion server, which rejects the self-hosted key, so the event never lands and the button never goes away.

  • Show the button for customer warehouses only on SaaS; the Flagsmith database connection keeps it on self-hosted, since it posts to the installation's own API.

How did you test this code?

Unit tests for the extracted shouldShowSendFirstEvent predicate.

@vercel

vercel Bot commented Nov 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
flagsmith-frontend-preview Ready Ready Preview Nov 4, 2026 4:23pm UTC
flagsmith-frontend-staging Ready Ready Preview Nov 4, 2026 4:23pm UTC
1 Skipped Deployment
Project Deployment Actions Updated
docs Ignored Ignored Nov 4, 2026 4:23pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

Walkthrough

The change adds and tests shouldShowSendFirstEvent to determine whether to show the first-event prompt. WarehouseConnectionCard now calls this helper with the connection and SaaS status.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Minimal · up to ec57f

The change restricts customer-warehouse prompts on self-hosted installations while retaining the Flagsmith database prompt. The remaining suggestions improve test coverage and do not block merging after normal checks.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-actions github-actions Bot added front-end Issue related to the React Front End Dashboard fix labels Nov 4, 2026
@Zaimwa9
Zaimwa9 marked this pull request as ready for review November 5, 2026 09:54
@Zaimwa9
Zaimwa9 requested a review from a team as a code owner November 5, 2026 09:54
@Zaimwa9
Zaimwa9 requested review from talissoncosta and removed request for a team November 5, 2026 09:54
@github-actions github-actions Bot removed the fix label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-8874 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-8874 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api:pr-8874 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith:pr-8874 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8874 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-frontend:pr-8874 Finished ✅ Results ✅

@github-actions github-actions Bot added the fix label Nov 5, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick comments: 2

frontend/web/components/pages/environment-settings/tabs/warehouse-tab/__tests__/warehouseTypes.test.ts: 2

124-135: Trivial · Maintainability & Code Quality · Quick win

Add a managed Flagsmith case for the connected status.

The flagsmith branch of shouldShowSendFirstEvent hides the button for connected connections. The new cases cover only created and pending_connection. Removing the connected check would leave the suite passing. Add a row with status: 'connected' and the expected result false.

Suggested fix

     [
       { config: {}, warehouse_type: 'flagsmith' },
       { status: 'pending_connection', total_events_received: null },
       true,
       false,
     ],
+    [
+      { config: {}, warehouse_type: 'flagsmith' },
+      { status: 'connected', total_events_received: null },
+      true,
+      false,
+    ],

115-117: Trivial · Functional Correctness · Quick win

Add Databricks and PostgreSQL customer-warehouse cases for SaaS.

The customer-warehouse cases use only ClickHouse. If CUSTOMER_WAREHOUSE_TYPES dropped databricks or postgres, every test in this block would still pass. The Flagsmith database case returns before the list check, so no case tests an ordinary PostgreSQL connection against the list.

Suggested test cases

     [byo, { status: 'connected', total_events_received: 3 }, true, false],
+    [
+      { config: {}, warehouse_type: 'databricks' },
+      { status: 'connected', total_events_received: 0 },
+      true,
+      true,
+    ],
+    [
+      { config: {}, warehouse_type: 'postgres' },
+      { status: 'connected', total_events_received: 0 },
+      true,
+      true,
+    ],
Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 1e3a8e67-88a7-4a2d-ba99-701cb44456ad
Commits

Reviewing files that changed from the base of the PR and between af5c88f and ec57f57.

Files selected for processing: 3

  • frontend/web/components/pages/environment-settings/tabs/warehouse-tab/WarehouseConnectionCard.tsx
  • frontend/web/components/pages/environment-settings/tabs/warehouse-tab/__tests__/warehouseTypes.test.ts
  • frontend/web/components/pages/environment-settings/tabs/warehouse-tab/warehouseTypes.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #22879 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  12 seconds
commit  ec57f57
info  🔄 Run: #22879 (attempt 1)

🗂️ Previous results
✅ private-cloud · depot-ubuntu-latest-16 — run #22879 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  4 passed

Details

stats  4 tests across 4 suites
duration  3.6 seconds
commit  ec57f57
info  🔄 Run: #22879 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #22879 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  35.7 seconds
commit  ec57f57
info  🔄 Run: #22879 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #22879 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  31.7 seconds
commit  ec57f57
info  🔄 Run: #22879 (attempt 1)

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Visual Regression

24 screenshots compared. See report for details.
View full report

@Zaimwa9
Zaimwa9 merged commit e755012 into main Nov 5, 2026
46 checks passed
@Zaimwa9
Zaimwa9 deleted the fix/warehouse-test-event-self-hosted branch November 5, 2026 12:23

This branch was successfully deployed

2 active deployments
Preview – flagsmith-frontend-staging — ec57f57e Deployed Nov 4, 2026 by vercel[bot]
Preview – flagsmith-frontend-preview — ec57f57e Deployed Nov 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix front-end Issue related to the React Front End Dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants