fix: handle blank dynamic parameter values consistently#26122
Open
code-asher wants to merge 4 commits into
Open
fix: handle blank dynamic parameter values consistently#26122code-asher wants to merge 4 commits into
code-asher wants to merge 4 commits into
Conversation
This made it so we could not store it in ws.current before the message handler fired, which in some tests would result in not being able to send the initial message since we relied on having the socket in that handler.
These mirror the template and workspace build parameters which makes it much easier to reason about how things should work in tests.
38f9a4e to
f5b738b
Compare
f5b738b to
6240369
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes https://linear.app/codercom/issue/DEVEX-243/unable-to-remove-value-from-parameter-with-default
There was inconsistency with what the form showed and what actually was sent to the backend. I opted to make it so that explicitly blank values are always sent rather than have blank values silently changing to the default value (which in addition to being surprising also prevents you from submitting the edit form due to
hasUnsyncedParameters).An alternative would be to make it so blank values get updated in the form to their defaults, but I feel like that behavior would be more surprising. An explicit reset button might be better for that if we need it?
Also fix a minor issue in the mock websocket where it would fire responses before we could get a handle set to
ws.currentwhich basically caused client responses to get missed.The actual changes are minor (basically just adding
undefinedchecks); most of this is tests for both user input and auto-fill code paths for both the create and edit workspace settings pages to not only ensure that the form shows the right values, but that the websocket and API requests also use the right values.