Prevent versioned change requests from overwriting each other

[matthewelwell]

The way in which change requests work when versioning is enabled is such that a new version is created as a snapshot of the current state of a feature in a given environment when the change request is opened. This is illustrated in the following process:

Given Feature A, Segment X and Segment Y.

1. Create CR1 which adds a segment override for Segment X
2. Create CR2 which adds a segment override for Segment Y
3. Publish CR1
4. Publish CR2

The expected behaviour here is that both segment overrides should exist after following this process. Currently, the behaviour however, is that the end result is that only the override for Segment Y applied in CR2 will exist after following these steps because the override for Segment X did not exist when CR2 was created.

[matthewelwell]

Some thoughts on possible (perhaps temporary) improvements:

1. We should show an alert when viewing / publishing a change request if another change request has been published between the time that this one was created and the current time.
2. We could perhaps impose a limit on the number of change requests for a given feature (so that only 1 change request can exist for a given feature)
3. We should allow edits to be made to an existing change request (by it's author)

Some thoughts on a more permanent solution:

1. Change requests should consist of a 'diff' only, e.g. segment_overrides_to_add: [...], segment_overrides_to_update: [...], segment_overrides_to_delete: [...], environment_feature_state_updates: {...}. When a change request is published, these changes can be applied to a newly created version at that point (which will therefore include any changes made by other change requests up to that point).
2. We could implement a workflow to handle 'merges' so that when you open a change request, it shows that other changes have been made to a given feature since the change request was created and it won't let you publish it until a merge is completed. This merge should step through any differences between the current version and the version in the CR and verify if they should be added to the CR's version.

My preference here would be option (1) since I believe that will handle scheduled changes as well, but we will likely still need to consider how we deal with merge conflicts between multiple change requests. For example, if CR1 enables a feature for a segment, and CR2 adds a value for it, how would we handle this?

[matthewelwell]

As discussed with @kyle-ssg the permanent solution will behave as follows.

When creating a change request, a 'diff' will be provided to the API. This diff will consist of:

1. The updates to make to existing components of the feature in the given environment (the environment default state and value, and any modifications to existing segment overrides)
2. Any new segment overrides to create
3. Any segment overrides to remove

This diff will include only the list of changes to be made to the flag, not the entire feature as it exists in the given environment. When a change request is published, this diff will be applied to the feature at that point to create a new version of the feature by merging the changes into the other existing components of the feature.

If there are conflicts at the point of publishing, or when the scheduled date is reached, an error will be raised rather than overwriting what is there.

It is still TBD how / if users will be able to reconcile any conflicts by updating the change request to include any changes that have been made to the same components (e.g. segment overrides) of that feature between the time that the change request was created and the time at which it is published, or scheduled to go live.

Here are some scenarios to illustrate the behaviour:

Scenario 1

Initial state
Feature A - enabled: false, value: ""
Segment override A - enabled: true, value: "foo"

User A creates CR A with the following changes:

• Segment override B - enabled: true, value: "bar"

User B creates CR B with the following changes:
~ Feature A - enabled: true, value: "foobar"
~ Segment override A - enabled: true, value: "baz"

User B publishes CR B
User A publishes CR A

Final state
Feature A - enabled: true, value: "foobar"
Segment override A - enabled: true, value: "baz"
Segment override B - enabled: true, value: "bar"

Scenario 2

Initial state
Feature A - enabled: false, value: ""
Segment override A - enabled: true, value: "foo"

User A creates CR A with the following changes:

• Segment override B - enabled: true, value: "bar"

User B creates CR B with the following changes:

• Segment override B - enabled: true, value: "baz"

User B publishes CR B
User A attempts to publish CR A but receives an error stating that a segment override already exists for segment B

[matthewelwell]

@kyle-ssg I’ve made a start on the API for this one. I’ve got a PR up here but, because it involves private code the Uffizzi preview won’t work.

To spin up a local version to develop the FE against, you should be able to just do:

export CR_PAT=<GH PAT with packages:read scope>
echo $CR_PAT | docker login ghcr.io -u <GH username> --password-stdin

Then just update the docker-compose.yml file in the root of the repository to use the image at ghcr.io/flagsmith/flagsmith-private-cloud:pr-4245 instead of flagsmith.docker.scarf.sh/flagsmith/flagsmith:latest here and run docker compose up -d from the root of the repository.

Currently, the API expects a very similar data structure to the one on the updated endpoint to create a version. I've added a new attribute to the create-change-request endpoint "change_sets" which expects an object that looks very similar to the data in the create version endpoint:

{
  "feature_states_to_update": "",
  "feature_states_to_create": "",
  "segment_ids_to_delete_overrides": ""
}

The only difference is that (for the moment at least, I'm trying to improve this) the data is expected to be stringified json for each of these attributes. Here's an example payload (obtained from a working test I have in the API).

{
  "title": "Test CR",
  "description": "",
  "feature_states": [],
  "environment_feature_versions": [],
  "change_sets": [
    {
      "feature": 1,
      "feature_states_to_update": "[{\"feature_segment\": null, \"enabled\": true, \"feature_state_value\": {\"type\": \"unicode\", \"string_value\": \"some_updated_value\"}}, {\"feature_segment\": {\"segment\": 1, \"priority\": 2}, \"enabled\": true, \"feature_state_value\": {\"type\": \"unicode\", \"string_value\": \"segment_override_updated_value\"}}]",
      "feature_states_to_create": "[{\"feature_segment\": {\"segment\": 3, \"priority\": 1}, \"enabled\": true, \"feature_state_value\": {\"type\": \"int\", \"integer_value\": 42}}]",
      "segment_ids_to_delete_overrides": "[2]"
    }
  ]
}