Updates the strategy change component used in change requests to have a
"tab"-like functionality, where you can switch between "Change" (the
rendered strategy) and "View diff" (the JSON diff). This is a tab
switcher, so you navigate it with arrow keys if on a keyboard, and I've
set selection to follow focus for now. This is my preference as it saves
you extra space/enter taps, but we can remove it later if we want.
Most of the changes in this PR is wrapping the existing strategy change
components/sections in tab panels and tab context providers.
Later changes in this project will remove the existing "view diff" hover
link in favor of this view.
There is some work to do on the design front (in terms of spacing,
borders, etc), but this covers the core functionality.
The pre-existing strategy change component has been moved into the
"LegacyStrategyChange" file with no changes beyond a deprecation
comment. The existing tests now test the new component instead with no
breakage. (I anticipate breaking when we remove the view diff link,
though)
Change with strategy variants:
<img width="991" alt="image"
src="https://github.com/user-attachments/assets/ac28912f-5b08-4a9c-96da-81bfd0b2e68e"
/>
<img width="1005" alt="image"
src="https://github.com/user-attachments/assets/4addaacc-101c-46cb-888f-95dc3b1cac25"
/>
## Edge case: deleted strat with no reference strategy
There is a case in the code where "reference strategy" can be undefined
for a deleted strategy. It is defined as follows:
```ts
const referenceStrategy =
changeRequestState === 'Applied'
? change.payload.snapshot
: currentStrategy;
```
I've decided to still show the "(no changes)" json diff in that setting,
so that the tabs actually affect something. I don't know how often this
happens, though: probably not anymore unless your CR is _ancient_, as we
introduced the snapshot quite a while ago, and `currentStrategy`
shouldn't really be undefined here, I should think.
Deleted strategy with no reference strategy:
Change:
<img width="1013" alt="image"
src="https://github.com/user-attachments/assets/352eaec9-c0ef-4d5a-b765-11304daf4474"
/>
Diff:
<img width="1029" alt="image"
src="https://github.com/user-attachments/assets/e69c81a6-1ef7-47ff-853a-9fb900b26303"
/>
Adds a number of small corrections and fixes to (mainly) CR-related
component. Discovered as part of adding the new JSON diff view. Refer to
the various inline comments for explanations.
---------
Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
This PR continues the cleanup after removing the addEditStrategy flag
(part 2 of ???). The primary purpose of this PR is to delete and remove
all references to the LegacyConstraintAccordion.
I've gone and updated all references to the legacy files in external
components and verified manually that they still work.
Most of the files in this PR are changing references. I've extracted two
bits into more general constants/utils:
1. Constraint IDs are a symbol. it was exported as a const from the
previous createEmptyConstraint file. I've moved it into constants.
2. formatOperatorDescription was similarly used all over the place, so
I've placed it in the shared utils directory.
In reviewing this, you can ignore any changes in the legacy constraint
accordion folder, because that's all been deleted. Instead, focus on the
changes in the other files. It's primarily just import updates, but
would be good to get a second set of eyes, anyway.
Removes all usages of flag addEditStrategy and refactors code where
necessary.
This is only the first step of the cleanup. After this, there's still
lots of code to be removed. I've got a different PR that removes ~5k
lines of code (https://github.com/Unleash/unleash/pull/10105) that I
want to reach in pieces to make sure that everythnig works on the way
there.
We're migrating to ESM, which will allow us to import the latest
versions of our dependencies.
Co-Authored-By: Christopher Kolstad <chriswk@getunleash.io>
Code for constraint accordion was copy-pasted before previous
improvement. Old version is still in use for Segments. When we get to
improving constraint editing we should rebuild segments editing, without
use of this code.
- new way of showing strategy variants
- fixed wrapping issue in strategy editing, for a lot of variants
defined (`SplitPreviewSlider.tsx` change)
- aligned difference between API and manually added types
Use new design for release plans in flag environments.
- Move old ReleasePlanMilestone into Legacy file and update imports
- In the new version, use the same strategy list and item as in the
general strategy list and milestone template creation (components to be
extracted in the future)
- Fix an issue with the border being obscured by overflow by hiding
overflow
As of PR #8935, we no longer support both text and title, and confetti
has been removed.
This PR:
- removes `confetti` from the toast interface
- merges `text` and `title` into `text` and updates its uses across the
codebase.
- readjusts the text where necessary.
**Upgrade to React v18 for Unleash v6. Here's why I think it's a good
time to do it:**
- Command Bar project: We've begun work on the command bar project, and
there's a fantastic library we want to use. However, it requires React
v18 support.
- Straightforward Upgrade: I took a look at the upgrade guide
https://react.dev/blog/2022/03/08/react-18-upgrade-guide and it seems
fairly straightforward. In fact, I was able to get React v18 running
with minimal changes in just 10 minutes!
- Dropping IE Support: React v18 no longer supports Internet Explorer
(IE), which is no longer supported by Microsoft as of June 15, 2022.
Upgrading to v18 in v6 would be a good way to align with this change.
TS updates:
* FC children has to be explicit:
https://stackoverflow.com/questions/71788254/react-18-typescript-children-fc
* forcing version 18 types in resolutions:
https://sentry.io/answers/type-is-not-assignable-to-type-reactnode/
Test updates:
* fixing SWR issue that we have always had but it manifests more in new
React (https://github.com/vercel/swr/issues/2373)
---------
Co-authored-by: kwasniew <kwasniewski.mateusz@gmail.com>
This change fixes a bug where we would show the list of segments as
changed (causing a conflict) if their order wasn't the same in the
change as in the original.
By sorting the segments before comparing them, we can avoid this issue.
To avoid modifying the objects that are passed in (in case that has
knock-on effects anywhere), we copy the objects. And because `toSorted`
"only" has about 89% coverage now, I chose to use `sort` and spread the
arrays instead.
The current approach uses adds an extra parameter to the components and
passes it through from the parent components. It's never a lot of
levels, so it feels alright, but it's feels like a bit of a code smell.
I wonder if it would make sense to use a context for each change
request? 🤔
Supersedes: https://github.com/Unleash/unleash/pull/6181
This PR updates the way we show deleted strategies in the CR UI. Instead
of showing just the strategy name and a diff on hover, we show the same
strategy config as we do for new and updated strategies.
This makes it easier to see what you have deleted.
In doing so, it also fixes two issues:
1. inconsistent border radius for segment changes listed. Due to an
override in `frontend/src/themes/theme.ts`, these would get a border
radius of `theme.shape.borderRadiusLarge` instead of
`theme.shape.borderRadiusMedium`. It does this by adding a class and
making the selector more specific.
2. The background was unset for the strategy rollout box and constraint
item boxes.
It looks like this:
<img width="728" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/7cba28ac-0454-444d-8cfa-f46543ccf2dc">
<img width="728" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/832be653-3def-4afc-b72f-36fcd76ad83d">
Or with more kinds of strategies:
<img width="454" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/f18e5482-7d2e-4cbd-8177-9de6dfb10307">
Note: I'm happy to isolate the color changes to a separate PR if that's
preferable.
This PR adds showing of env variant conflicts in change requests.
This is a simple solution that only compares the total state of
variants. We *could* potentially do a modified version where we show
each and every variant as its own property. Because variants have to be
unique by name and because their names can't be changed after their
creation, we could create a map of variant name to their data.
<img width="1105" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/0c67f958-6c4e-453a-9791-0e132fb1f23e">
Use React's context to track how many CRs are moved into their next
state with conflicts present.
This PR wraps environment change requests and change request overviews
in a change request plausible context that contains a
`willOverwriteStrategyChanges` property. This property is updated by the
diff calculation if there are any conflicts and then read by the
`changeState` function in the `useChangeRequestApi` hook.
As long as at least one of the strategies in the CR contain conflicts,
it will be marked as overwriting changes.
We had to make some updates to let the compiler know about the types and
fix an issue with nested objects not being compared as objects (instead
as strings), but this saves us a few lines and is hopefully more
readable.
This PR fixes a bug in the displayed value of the conflict list so that
it shows the value it would update to instead of the snapshot value.
In doing so, it updates the logic of the algorithm to:
1. if the snapshot value and the current value are the same, it's not a
conflict (it's an intended change)
2. If the snapshot value differs from the current value, it is a
conflict if and only if the value in the change differs from the current
value. Otherwise, it's not a conflict.
The new test cases are:
- it shows a diff for a property if the snapshot and live version differ
for that property and the changed value is different from the live
version
- it does not show a diff for a property if the live version and the
change have the same value, even if the snapshot differs from the live
version
- it does not show a diff for a property if the snapshot and the live
version are the same
