Skip to content

fix: validate-metadata-required-for-on-all-entities#6658

Open
Zaimwa9 wants to merge 14 commits intomainfrom
fix/required-metadata-validation
Open

fix: validate-metadata-required-for-on-all-entities#6658
Zaimwa9 wants to merge 14 commits intomainfrom
fix/required-metadata-validation

Conversation

@Zaimwa9
Copy link
Contributor

@Zaimwa9 Zaimwa9 commented Feb 4, 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

Closes #6520

Includes a refactor of the logic in order to prepare #6620

  • Simplified AddMetadataToEntity component
  • Created mergeMetadataFields and metadataValidation in a utils + unit tests
  • Added validation for both update and create for environments and features
  • ⚠️ NB: disabled state for segments has not been implemented as it is on a different page (could be confusing) and error from api is well handled

How did you test this code?

  • New unit tests
  • Create metadata fields in organisation (at least 2 required for each of them)
  • Go to create feature / update feature / create environment / update environment / segments and play around

@Zaimwa9 Zaimwa9 requested a review from a team as a code owner February 4, 2026 17:24
@Zaimwa9 Zaimwa9 requested review from talissoncosta and removed request for a team February 4, 2026 17:24
@vercel
Copy link

vercel bot commented Feb 4, 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 Feb 19, 2026 4:52pm
flagsmith-frontend-staging Ready Ready Preview, Comment Feb 19, 2026 4:52pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Feb 19, 2026 4:52pm

Request Review

@Zaimwa9 Zaimwa9 marked this pull request as draft February 4, 2026 17:24
@github-actions github-actions bot added front-end Issue related to the React Front End Dashboard fix labels Feb 4, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

Docker builds report

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

talissoncosta
talissoncosta previously approved these changes Feb 6, 2026
Copy link
Contributor

@talissoncosta talissoncosta left a comment

Choose a reason for hiding this comment

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

Looks good, just some non blocking comments

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

toast(getMetadataErrors(result.error as MetadataErrorResponse), 'danger')
} else {
toast('Environment Field Updated')
setHasChanges(false)
Copy link

Choose a reason for hiding this comment

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

Environment save reverts UI to stale cached data

High Severity

After a successful environment metadata save, setHasChanges(false) triggers the useEffect at line 77 which checks !hasChanges. Since updateEnvironment only invalidates Environment tags (not Metadata tags), the initialFields from useGetEntityMetadataFieldsQuery remains stale with pre-edit values. The effect then overwrites metadataFields with those stale values, visually reverting the user's just-saved changes.

Additional Locations (1)

Fix in Cursor Fix in Web

}

return updatedMetadataFields
})
Copy link

Choose a reason for hiding this comment

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

Side effect inside React state updater function

Low Severity

The onChange callback is invoked inside the setMetadataFields updater function, which React requires to be pure. In React 18 strict mode (development), updater functions are called twice to detect impurities, which would cause onChange to fire twice per user change. Moving the onChange call outside the updater (e.g., after setMetadataFields) would keep the updater pure and avoid the double-fire in development.

Fix in Cursor Fix in Web

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.

Custom Fields mandatory field UI/UX bug

2 participants

Comments