Skip to content

Conversation

@kyle-ssg
Copy link
Member

@kyle-ssg kyle-ssg commented Dec 3, 2025

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

  • 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?
  • I have used a Conventional Commit title for this Pull Request

Changes

Allows editing a versioned change request

  • Opens up the edit feature modal with the changesets applied
  • Corresponding changes are then converted back to changesets and updated with the change request

How did you test

Edit.Change.Requests.mov

this code?

Edit a change request in a versioned environment

@vercel
Copy link

vercel bot commented Dec 3, 2025

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

Project Deployment Review Updated (UTC)
flagsmith-frontend-preview Ready Ready Preview, Comment Dec 16, 2025 1:34pm
flagsmith-frontend-staging Ready Ready Preview, Comment Dec 16, 2025 1:34pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Ignored Ignored Preview Dec 16, 2025 1:34pm

@kyle-ssg kyle-ssg changed the base branch from main to chore/create-flag-migration December 3, 2025 19:22
@github-actions github-actions bot added front-end Issue related to the React Front End Dashboard feature New feature or request labels Dec 3, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Docker builds report

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

@Zaimwa9 Zaimwa9 requested review from talissoncosta and removed request for Zaimwa9 December 4, 2025 10:17
@kyle-ssg kyle-ssg mentioned this pull request Dec 9, 2025
4 tasks
@matthewelwell
Copy link
Contributor

My understanding of the requirements, written in test scenarios. @kyle-ssg is this your understanding of the behaviour? Anything else that we should be testing?

Non-scheduled CRs

  1. User can edit unpublished CRs they created
  2. User cannot edit unpublished CRs created by another user
  3. User cannot edit published CRs

Scheduled CRs

  1. User can edit unpublished scheduled CRs that they created
  2. User cannot edit unpublished scheduled CRs created by another user
  3. User cannot edit published scheduled CRs

@kyle-ssg
Copy link
Member Author

kyle-ssg commented Dec 16, 2025

My understanding of the requirements, written in test scenarios. @kyle-ssg is this your understanding of the behaviour? Anything else that we should be testing?

@matthewelwell The exception to that at the moment is anyone can edit a change request if they have the ability to create one. I can see a use case for this tbh, if someone is away we should let someone with appropriate permissions to do this.

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.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

...v,
originalPriority: i,
priority: i,
}))
Copy link

Choose a reason for hiding this comment

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

Bug: Segment override priorities from changeset are discarded

When editing a versioned change request, segment override priorities from the changeset are set at lines 119 and 166 (priority: changedFeatureState.feature_segment?.priority || 0), but then immediately overwritten with array indices at lines 185-189. Since the save function at create-feature/index.js:604 uses the top-level priority property for feature_segment.priority, any priority reordering specified in the original change request will be lost when the edit is saved. The segments also remain in the current live order rather than being sorted by the changeset's intended priorities.

Additional Locations (1)

Fix in Cursor Fix in Web

changeRequest.feature_states[0].feature_state_value,
),
}
}
Copy link

Choose a reason for hiding this comment

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

Bug: Legacy change requests ignored in versioned environments

The condition logic creates a gap for legacy change requests. The first branch requires isVersioned && changeRequest.change_sets, and the second requires !isVersioned && changeRequest.feature_states?.[0]. If an environment has versioning enabled but a change request was created before versioning (using feature_states instead of change_sets), neither branch executes. The changedEnvironmentFlag would retain the current live values rather than the change request's intended values, causing the edit modal to display incorrect data.

Fix in Cursor Fix in Web

@matthewelwell
Copy link
Contributor

@matthewelwell The exception to that at the moment is anyone can edit a change request if they have the ability to create one. I can see a use case for this tbh, if someone is away we should let someone with appropriate permissions to do this.

My concern with that is that we don't have any indication when a change request has been edited. If we're not going to add that (which is probably the correct approach for now), I would rather restrict it to the author, or environment admins for now.

At the moment, it's only the author that can delete a change request for example - I think we should add environment admins to this list, but I don't think anyone should be able to edit or delete a CR.

@matthewelwell
Copy link
Contributor

I've aligned the BE with my expectations in a PR here.

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Dec 16, 2025
@kyle-ssg
Copy link
Member Author

kyle-ssg commented Dec 16, 2025

I've aligned the BE with my expectations in a PR here.

Ok yep, I've updated the FE to do this too.

Testing scenarios look valid

? changeRequest.feature_states?.[0]
?.multivariate_feature_state_values
: undefined
}
Copy link

Choose a reason for hiding this comment

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

Bug: Versioned CR edit loses multivariate allocation values

When editing a versioned change request, multivariate_options is explicitly passed as undefined. During save, this causes a fallback to projectFlag.multivariate_options (project defaults) instead of preserving the changeset's multivariate allocation values. If the original change request modified multivariate percentage allocations, those modifications would be lost and replaced with default values when the edited change request is saved.

Additional Locations (1)

Fix in Cursor Fix in Web

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Dec 16, 2025
})
const mergedStates = mergeChangeSets(
changeRequest.change_sets,
currentFeatureStatesResponse.data.results,
Copy link

Choose a reason for hiding this comment

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

Bug: Missing null check causes crash when API call fails

The editChangeRequest function calls getFeatureStates and immediately accesses currentFeatureStatesResponse.data.results without checking if data exists. When an RTK Query endpoint fails (network error, API error, etc.), the response object has data: undefined and an error property instead. This will cause a TypeError crash when attempting to access .results on undefined. The codebase uses optional chaining (data?.results) elsewhere for similar patterns, suggesting this check was inadvertently omitted here.

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

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.

4 participants