fixed segments not being copied #2105
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
# Conflicts: # src/lib/__snapshots__/create-config.test.ts.snap
FredrikOseberg
requested changes
Sep 30, 2022
Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>
FredrikOseberg
requested changes
Oct 5, 2022
| @@ -70,6 +71,8 @@ export class PublicInviteController extends Controller { | |||
| openApiService.validPath({ | |||
| tags: ['Public signup tokens'], | |||
| operationId: 'addPublicSignupTokenUser', | |||
| summary: | |||
There was a problem hiding this comment.
Unsure if the schema updates for public-invite belongs in this PR. Fine for now, but we should aim to separate PR concerns in the future
Comment on lines
+730
to
+735
| if (segments && segments.length > 0) { | ||
| result = { | ||
| ...result, | ||
| segments: segments.map((segment) => segment.id), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Is there a test that validates that this gives the correct result?
There was a problem hiding this comment.
Also, if there are no segments defined, should we return an empty array?
There was a problem hiding this comment.
I added one here https://github.com/ivarconr/unleash-enterprise/pull/138
There was a problem hiding this comment.
@andreas-unleash It seems this is not reflected in the specification, at least I can't see a spec update for this data structure.
sighphyre
pushed a commit
that referenced
this pull request
Oct 13, 2022
* fixed segments not being copied * fix fmt * bug fix * return segmentId[] when getting a feature strategy * do not return segments if they are not there * Update src/lib/services/feature-toggle-service.ts Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com> * fix lint * fix: more explicit column sorting and bug fix * update snapshot * rollback * add segment ids to feature strategies * bug fix Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently when you tried to copy a strategy (or all strategies) from one env to another, the segments where not being copied as expected.
This PR fixes the bug by adding an optional
copyOfparameter in the CreateFeatureStrategy schema, so that the handling of copying the segments over is moved to the backendThe need for handling this this way was created by the desired functionality of copying all strategies from one env to another
About the changes
Closes # .
Important files
Discussion points