-
Notifications
You must be signed in to change notification settings - Fork 465
feat: edit versioned change request #6368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: chore/create-flag-migration
Are you sure you want to change the base?
feat: edit versioned change request #6368
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Docker builds report
|
|
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
Scheduled CRs
|
@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. |
There was a problem hiding this 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, | ||
| })) |
There was a problem hiding this comment.
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)
| changeRequest.feature_states[0].feature_state_value, | ||
| ), | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
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. |
|
I've aligned the BE with my expectations in a PR here. |
ae6114e to
42704e8
Compare
7ac5ea9 to
6f090f5
Compare
Fixed |
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Allows editing a versioned change request
How did you test
Edit.Change.Requests.mov
this code?
Edit a change request in a versioned environment