feat: add "edit" link to playground strategies #4027
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
This change adds an "edit" link to all playground strategies when they are returned from the API, allowing the user to jump directly to the evaluated strategy edit screen. This change applies to both old and new strategies, so it should even work in the old playground. ## Discussion points: Should "links" be an object or a singular string? I know the notifications service uses just "link", but using an object will make it easier to potentially add more actions in the future (such as "enable"/"disable", maybe?) Do we need to supply basePath? I noticed that the notifications links only ever use an empty string for base path, so it might not be necessary and can potentially be left out. ## Changes I've implemented the link building in the playground service, as that is where we have all the information we need.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
This causes breakage when parsing URLs because they are interpreted as being directory navigation. Interestingly enough, we also have this issue in Unleash itself.
kwasniew
approved these changes
Jun 21, 2023
thomasheartman
force-pushed
the
feat/playground-link-to-strategies
branch
from
0bac0a8
to
f281dd6
Compare
thomasheartman
force-pushed
the
feat/playground-link-to-strategies
branch
from
6f030c1
to
5bab66d
Compare
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.
This change adds an "edit" link to all playground strategies when they
are returned from the API, allowing the user to jump directly to the
evaluated strategy edit screen.
This change applies to both old and new strategies, so it should even
work in the old playground.
This does not use this information in the frontend yet.
Discussion points:
Should "links" be an object or a singular string? I know the
notifications service uses just "link", but using an object will make
it easier to potentially add more actions in the future (such as
"enable"/"disable", maybe?)
Do we need to supply basePath? I noticed that the notifications links
only ever use an empty string for base path, so it might not be
necessary and can potentially be left out.
Changes
I've implemented the link building in a new view model file. Inspecting the output after the result is fully ready requires some gnarly introspection and mapping, but it's tested.
Further, I've done a little bit of work to stop the playground service using the schema types directly as the schema types now contain extra information.
This PR also updates the
urlFriendlyStringarbitrary to not produce strings that contain only periods. This causes issues when parsing URLs (and is also something we struggle with in the UI).