Skip to content

FCT2-21225: Fix "Move selected" button visible in prod due to missing feature flag gate in v1 screen - #533

Merged
HasanCPS merged 3 commits into
mainfrom
fix/FCT2-21225-private-beta-feature-user-group2-env-var
Aug 6, 2026
Merged

FCT2-21225: Fix "Move selected" button visible in prod due to missing feature flag gate in v1 screen#533
HasanCPS merged 3 commits into
mainfrom
fix/FCT2-21225-private-beta-feature-user-group2-env-var

Conversation

@HasanCPS

@HasanCPS HasanCPS commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

PR checklist

Correctness

  • Does what the ticket asks for

Keeping it simple

  • Doesn't rebuild something that already exists in the codebase
  • No more complex or slower than it needs to be
  • No leftover debug logging or commented-out code

Easy to miss (a green pipeline won't flag these)

  • One logical change, not a pile of unrelated stuff

What

Two fixes for FCT2-21225 where the "Move selected" button was incorrectly showing in production on the Transfer from Egress page.

Fix 1 - Missing feature flag gate in v1 screen (root cause)

TransferControls in the v1 screen was rendering the Move button whenever transferSource === "egress", without checking featureFlags.transferMove. The old screen (NetAppFolderContainer) had this check but it never got carried over when the v1 screen was built.

  • onMove is now only passed to TransferControls when featureFlags.transferMove is true
  • TransferControls only renders the button when onMove is defined
  • Unit tests added covering the render conditions

Fix 2 - Env var name mismatch in pipeline (secondary bug)

PR #524 added VITE_PRIVATE_BETA_USER_GROUP2 to the pipeline env: block with the FEATURE segment missing. config.ts references VITE_PRIVATE_BETA_FEATURE_USER_GROUP2, so Vite couldn't resolve the value at build time. This means group-gated features (transferMove, caseDetails, globalNav) can't correctly resolve group membership.

  • Renamed to VITE_PRIVATE_BETA_FEATURE_USER_GROUP2 in the pipeline YAML
  • Added to .env.playwright for test coverage

Why

User could see the Move button in prod because:

  1. VITE_FEATURE_FLAG_TRANSFER_MATERIALS_V1 = "true" routes all users to the v1 screen
  2. The v1 screen was passing onMove unconditionally with no check against featureFlags.transferMove
  3. TransferControls rendered the button whenever transferSource === "egress"

The env var typo is a separate real bug (group membership checks are broken for any feature using PRIVATE_BETA_FEATURE_USER_GROUP2 as a groupKey) but it wasn't the direct cause of what user saw.

Resolves FCT2-21225

…me in pipeline

The env var was added in #524 as VITE_PRIVATE_BETA_USER_GROUP2 (missing
FEATURE segment), so Vite could not resolve
VITE_PRIVATE_BETA_FEATURE_USER_GROUP2 at build time. This caused the
transfer-move feature gate to malfunction in production.

Also adds the variable to .env.playwright for test coverage.
@HasanCPS
HasanCPS requested a review from lilachdavis August 5, 2026 15:53

@lilachdavis lilachdavis left a comment

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.

These changes are useful, but I don't think they'll fix the issue described in the ticket.

  1. The correct value for VITE_PRIVATE_BETA_FEATURE_USER_GROUP2 is already consumed by the pipeline, even when not specified in the env block. This can be seen in the screenshot captured by Mark.

  2. We use the user group to enable additional features to its members, not to limit the features they have.

  3. The expected behaviour is that the feature is enabled based on both conditions being true: (1) The feature flag is set to True, and (2) the user is a member of the correct group.
    In the screenshot above we can see that VITE_FEATURE_FLAG_TRANSFER_MOVE is set to "false". This alone should disable the Move functionality for Mark, regardless of his group membership.

  4. For additional context - Mark is already a member of group2, so if the feature was enabled for the group, he'd be able to see the Move button.

brad-cps
brad-cps previously approved these changes Aug 5, 2026

@brad-cps brad-cps left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like Lilach's comments need further investigation...

@HasanCPS

HasanCPS commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

These changes are useful, but I don't think they'll fix the issue described in the ticket.

  1. The correct value for VITE_PRIVATE_BETA_FEATURE_USER_GROUP2 is already consumed by the pipeline, even when not specified in the env block. This can be seen in the screenshot captured by Mark:
  1. We use the user group to enable additional features to its members, not to limit the features they have.
  2. The expected behaviour is that the feature is enabled based on both conditions being true: (1) The feature flag is set to True, and (2) the user is a member of the correct group.
    In the screenshot above we can see that VITE_FEATURE_FLAG_TRANSFER_MOVE is set to "false". This alone should disable the Move functionality for Mark, regardless of his group membership.
  3. For additional context - Mark is already a member of group2, so if the feature was enabled for the group, he'd be able to see the Move button.

Good shout, and you're right to question it. I dug into the codebase properly and I think the env var typo is a real bug but it's not what caused what Mark saw.

Here's what I found:

transferMaterialsV1 in useUserGroupsFeatureFlag.ts doesn't pass any group claims to shouldShowFeature -- it just reads FEATURE_FLAG_TRANSFER_MATERIALS_V1 directly. So group2 membership has nothing to do with which screen a user lands on. Everyone with that flag set to "true" gets the v1 screen.

transferMove does pass group claims, but FEATURE_FLAG_TRANSFER_MOVE is "false" in prod so the short-circuit kicks in before group membership is even evaluated. So the unresolved group2 var doesn't factor in here either.

The actual problem is that TransferControls.tsx in the v1 screen renders the Move button whenever transferSource === "egress" with no feature flag check at all:

{transferSource === "egress" && (
  <Button onClick={onMove} disabled={disableControls}>
    Move selected
  </Button>
)}

The old screen (NetAppFolderContainer.tsx) does this correctly:

if (featureFlags?.transferMove) {
  items = [...items, { id: `${path}:move`, label: "Move", ... }];
}

That gate just never got carried over when the v1 screen was built.

So the fix should be in transfer-materials-v1/index.tsx, only passing onMove if the flag is on:

onMove={featureFlags?.transferMove ? () => handleTransferAction("move") : undefined}

And a matching guard in TransferControls.tsx:

{transferSource === "egress" && onMove && (
  <Button onClick={onMove} disabled={disableControls}>
    Move selected
  </Button>
)}

I'll update the PR with this fix. The env var rename is still worth doing as a cleanup since the name genuinely doesn't match config.ts, but I'll keep it as a separate commit so the intent is clear.

Thanks for pushing back on this one.

…re flag

TransferControls rendered the Move button whenever transferSource was
egress, without checking featureFlags.transferMove. The old screen
(NetAppFolderContainer) had this gate but it was never carried over
to the v1 screen.

Now onMove is only passed when featureFlags.transferMove is true, and
TransferControls only renders the button when onMove is defined.

Adds unit tests for TransferControls covering all render conditions.
@HasanCPS HasanCPS changed the title FCT2-21225: Fix correct VITE_PRIVATE_BETA_FEATURE_USER_GROUP2 env var name in pipeline FCT2-21225: Fix "Move selected" button visible in prod due to missing feature flag gate in v1 screen Aug 6, 2026
@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

@lilachdavis lilachdavis left a comment

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.

LGTM!

@HasanCPS
HasanCPS merged commit f8deb4b into main Aug 6, 2026
11 checks passed
@HasanCPS
HasanCPS deleted the fix/FCT2-21225-private-beta-feature-user-group2-env-var branch August 6, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants