Skip to content

feat(Segment membership inspection PoC): Surface identity counts in segments UI#7467

Open
khvn26 wants to merge 6 commits into
feat/segment-membership-countsfrom
feat/segment-membership-counts-ui
Open

feat(Segment membership inspection PoC): Surface identity counts in segments UI#7467
khvn26 wants to merge 6 commits into
feat/segment-membership-countsfrom
feat/segment-membership-counts-ui

Conversation

@khvn26
Copy link
Copy Markdown
Member

@khvn26 khvn26 commented May 9, 2026

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

Contributes to #7464 (stacked on top — review after backend PR).

Surfaces the new Segment.memberships field on three places in the existing UI:

  • Project segments list — total identities across envs + relative sync time on each row.
  • Identities tab on the segment edit page — same total, in compact form, next to the tab label.
  • Inside the Identities tab — per-env count beside each option in the environment select; selecting an env reveals the full last_synced_at timestamp underneath.

Two small enabling changes outside the segments folder:

  • Shared Option in project-components.js now honours selectProps.formatOptionLabel when callers pass one (falls back to the existing label/description layout).
  • Drops !important on the global .chip align-self so the badge can opt into vertical centering inline.

Screenshots

Identity count chips in the Segments list:

Identity count chips in the Segments list

Identity count chip on the Identities tab in the Segment page:

Identity count chip on the Identities tab in the Segment page

Per-environment chips:

Per-environment chips

Last sync date for selected environment:

Last sync date for selected environment

How did you test this code?

Manually drove the segments list, segment edit page, and the Identities tab in a local dev server pointed at staging, with a fetch interceptor injecting synthetic memberships arrays into the segments responses. Verified the total badge on each row, the compact badge next to the Identities tab label, per-env chips inside the select options, and the full timestamp display on selection — including the Last synced: — placeholder before any selection.

Added one Playwright spec (e2e/tests/segment-membership-test.pw.ts) using page.route to stub memberships end-to-end against a real seeded segment, asserting the total badge in the list, the badge on the tab, per-env badges in the select, and the full-timestamp line on selection.

khvn26 added 6 commits May 8, 2026 23:34
Backfills identities from Dynamo to Snowflake daily, then refreshes
per-(segment, environment) match counts in the new `SegmentMembership`
cache. The translator from `flagsmith-sql-flag-engine` turns each
canonical segment into a SQL `WHERE` predicate; counts are
materialised as `COUNT(*) ... GROUP BY environment_id` per segment.
The serializer surfaces them as a list of `{environment, count,
last_synced_at}`, ready to back per-env count badges in the
Identities-tab environment dropdown.

Pipeline shape:

- `backfill_identities_to_snowflake` is the daily recurring task
  (`timeout=4h` to fit large environments). After backfilling each
  project's environments it dispatches one
  `refresh_project_segment_counts(project_id)` per project so the
  count refresh always sees the freshly backfilled snapshot rather
  than racing a separate schedule.
- `refresh_project_segment_counts` opens its own Snowpark session,
  re-checks the FoF flag at execution time so a stale fan-out skips
  orgs that have since been disabled, and bulk-upserts via Postgres
  `ON CONFLICT` (single statement per project).
- `compute_segment_counts_for_project` returns a list of unsaved
  `SegmentMembership` instances; the task stamps `last_synced_at`
  consistently across the batch. Untranslatable segments emit a
  structlog `compute.segment.skipped` error event so we hear about
  predicate gaps rather than silently dropping rows.

Both tasks short-circuit when SNOWFLAKE_* env vars are unset and
skip per-organisation when the `segment_membership_inspection`
Flagsmith-on-Flagsmith flag is False, so SaaS rolls out gradually
and self-hosted is unaffected.

DELETE-then-INSERT runs without an explicit transaction. Snowflake
holds micropartition locks for the lifetime of an open transaction,
and at 10M+ identities a BEGIN/COMMIT around the whole env partition
would keep that lock open for minutes. Per-statement implicit
commits leave a brief mid-refresh window where readers see an empty
partition; acceptable under the FoF flag's gradual rollout.

Backfill writes via Snowpark DataFrames against the canonical
IDENTITIES schema, with `DynamoIdentity` documents projected through
`segment_membership.mappers.map_identity_document_to_snowflake_row`.
Refresh issues a single batched UNION ALL using parameterised SQL —
env keys are bound, predicates from the engine are already escape-
safe. Schema setup is a `RunPython` migration gated on
`is_snowflake_configured()`, so it no-ops on self-hosted and in the
test suite.

The segment serializer surfaces cached counts via a new `memberships`
list field; absence of an entry is the read-side signal, no flag
check on the read path. `SegmentMembershipSerializer` gives
drf-spectacular a typed schema. Adds a generic `batched` helper to
`api/util/util.py` for the per-INSERT batching.

beep boop
…ps prefetch

The new `prefetch_related("memberships")` adds one IN-clause query per
list response, even when no rows exist. Update the regression
expectations so the existing test suite reflects the new baseline.

beep boop
… pre-release

Switches the api dep from a private-repo git URL — which the Docker
build can't clone in CI — to a versioned pin against Flagsmith's
staging CodeArtifact PyPI (`flagsmith-pypi-staging`,
account 302456015006, eu-west-2). Initial published release: 0.1.0a1.

The reusable docker-build workflow now unconditionally assumes the
OIDC role
`arn:aws:iam::302456015006:role/codeartifact-github-actions-staging`
(trust policy allows any `repo:Flagsmith/*`), fetches an
authorisation token, and exposes it to every build as the
`codeartifact_token` BuildKit secret. Builds that don't mount the
secret simply ignore it; the OIDC + token cost is a couple of
seconds per build.

`Dockerfile`'s four `make install*` lines mount the
`codeartifact_token` secret and export
`POETRY_HTTP_BASIC_FLAGSMITH_PYPI_STAGING_*` so poetry resolves the
dep from CodeArtifact. The header documents the
`--secret="id=codeartifact_token,env=..."` incantation for local
builds.

beep boop
…fact

The unit-test, MCP-schema-push, makefile-target, and update-flagsmith
workflows all run `make install-packages`, which now needs CodeArtifact
credentials to resolve the `flagsmith-sql-flag-engine` pre-release.

Encapsulate the OIDC role assumption + token fetch in a composite action,
reuse it from the Docker build workflow, and wire it into every workflow
that runs poetry install.

beep boop
CodeQL flagged the MD5 truncation as a sensitive-data hashing risk.
UUIDv4 already gives us the random bits we need for a dedup key, so
take the high 64 bits directly via int.from_bytes and drop the hash.

beep boop
Adds two reusable badges sourced from the Segment.memberships field:
- SegmentMembershipTotalBadge: aggregates counts across environments and
  shows the most recent sync time as a relative interval. Rendered on
  each segment row in the project segments list (with the "identities"
  noun) and next to the Identities tab label on the segment edit page
  (compact, count + sync only).
- SegmentMembershipEnvBadge: per-env count, rendered as the option label
  inside the Identities tab's environment select.

Selecting an environment in the Identities tab displays the full
last_synced_at timestamp underneath the select; before any selection
the row stays in place as a placeholder.

The shared Option component in project-components now honours
selectProps.formatOptionLabel when callers provide one, falling back to
the existing label/description layout. Drops the !important on the
chip's align-self so badges can opt into vertical centering inline.

beep boop
@khvn26 khvn26 requested a review from a team as a code owner May 9, 2026 02:07
@khvn26 khvn26 requested review from kyle-ssg and removed request for a team May 9, 2026 02:07
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 9, 2026

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

Project Deployment Actions Updated (UTC)
flagsmith-frontend-preview Ready Ready Preview, Comment May 9, 2026 2:07am
flagsmith-frontend-staging Ready Ready Preview, Comment May 9, 2026 2:07am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored May 9, 2026 2:07am

Request Review

@github-actions github-actions Bot added front-end Issue related to the React Front End Dashboard feature New feature or request labels May 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Docker builds report

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

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

failed  3 failed

Details

stats  3 tests across 2 suites
duration  41 seconds
commit  a757fd1
info  📦 Artifacts: View test results and HTML report
🔄 Run: #16624 (attempt 1)

Failed tests

firefox › tests/segment-membership-test.pw.ts › Segment membership badges render in list, tab, and env select @oss
firefox › tests/segment-test.pw.ts › Segment test 1 - Create, update, and manage segments with multivariate flags @oss
firefox › tests/segment-test.pw.ts › Segment test 4 - Create ANY rule type segment and verify match changes when rule is updated @oss

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

failed  3 failed
passed  1 passed

Details

stats  4 tests across 3 suites
duration  51.3 seconds
commit  a757fd1
info  📦 Artifacts: View test results and HTML report
🔄 Run: #16624 (attempt 1)

Failed tests

firefox › tests/segment-membership-test.pw.ts › Segment membership badges render in list, tab, and env select @oss
firefox › tests/segment-test.pw.ts › Segment test 1 - Create, update, and manage segments with multivariate flags @oss
firefox › tests/segment-test.pw.ts › Segment test 4 - Create ANY rule type segment and verify match changes when rule is updated @oss

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Visual Regression

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

@khvn26 khvn26 changed the title feat(segment_membership): Surface identity counts in segments UI feat(Segment membership inspection PoC): Surface identity counts in segments UI May 9, 2026
@kyle-ssg
Copy link
Copy Markdown
Member

I feel like we could neaten up the badges by putting the word identities and ~x days ago into a descriptive tooltip like we do for override badges. Other than that it looks great!

@khvn26
Copy link
Copy Markdown
Member Author

khvn26 commented May 11, 2026

@kyle-ssg The tooltips worked really badly with the tab and the environment select; that's how I came to dumping everything in the chip label. Besides that, I feel like last sync time is important info to surface before we switch to CDC / pure Snowflake lookups for counts.

Copy link
Copy Markdown
Contributor

@Zaimwa9 Zaimwa9 left a comment

Choose a reason for hiding this comment

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

2 comments on my side:

  • A subjective one related to the verbosity of the chip
  • As environments are central to those components, it looks to me that we have some room for typing improvements, if you agree on it we could have another pass there sooner than later

<UsersIcon className='chip-svg-icon' />
<span>
{count}
{compact ? '' : ` ${noun}`}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would either just get rid of mentioning "identities" as this badge has been already used or use compact everywhere (but it becomes dead code).
Regarding the last sync, this could be a tooltip on hover to not overload the UI

Copy link
Copy Markdown
Contributor

@Zaimwa9 Zaimwa9 May 11, 2026

Choose a reason for hiding this comment

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

I'm also thinking that we'll need to display the breakdown per environment for the identities. Could be through the tooltip but we can discuss the obstacles you had implementing it

return map
}, [memberships])

const renderEnvOption = (data: unknown) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should be able to use data: EnvOption directly no ?


type EnvProps = {
membership: SegmentMembership
environment?: Environment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT typing: I feel like the environment should always be present in those components ? So it should be required here and cascaded in all these new components (e.g here)

@khvn26 khvn26 force-pushed the feat/segment-membership-counts branch from 89e88f9 to 6e1584b Compare May 14, 2026 10:15
@khvn26 khvn26 requested review from a team as code owners May 14, 2026 10:15
@khvn26 khvn26 requested review from Zaimwa9 and removed request for a team May 14, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request front-end Issue related to the React Front End Dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants