-
Notifications
You must be signed in to change notification settings - Fork 9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix reset button in Try It Out form #9346
Fix reset button in Try It Out form #9346
Conversation
a165027
to
0d142c7
Compare
LGTM thanks for your efforts and for advancing further the reset functionality. Maybe you could add a test for regression |
Sure, I can add that. Could you point me in the right direction of what test I can write here. I see a test for this |
0d142c7
to
81140e0
Compare
ea44396
to
e9f1560
Compare
I have added a cypress test for regression. Kindly review the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this change.
I checked it out and it looks fine but I found more issues when using the specification from the issue itself.
It seems like it’s because the UserSource
schema there has maxProperties
set to 1 / lower count than each property of the schema. Despite this, each of the example values is filled and being sent. The reset button also stays visible even without making any changes. When resetting, only the first value is reset. It seems that not resetting other values is because we check if we already added the max amount of properties in sampleFromSchemaGeneric
.
Thanks for the review @glowcloud . I will take a look at this issue. |
I created a separate issue for The issue with only resetting some parameters seems related to this PR/issue, so leaving it here for now. |
Have used the respectXML prop to bypass maxProperties check. The reset button now only appears if the body is actually edited and all fields are reset. Please review the changes. @glowcloud |
15bace7
to
bbc4f4b
Compare
Thanks for trying to fix the issue with The issue is actually due to the fact that at first we’re getting the default values in a different way, by iterating through the properties of the schema here:
so we don’t see that the maxProperties limit exists in the parent schema.
As for the fix for resetting, it was superseded by #9717 with some fixes. Your contribution will be properly credited. |
Description
The reset button needed the following two corrections:
oas3Actions.setRequestBodyValue
expects a orderedMap/Map and was currently being passed a string. So a conversion from string to orderedMap was added.UPDATE_REQUEST_BODY_VALUE
action currently did not copy over the values if the value was a map assuming the user entered values will be string but that is not the case in reset, as the values will already have been converted to map. Therefore even the map values have been copied to correct key to reflect changes.Motivation and Context
Fixes #9158
How Has This Been Tested?
I manually tested this by opening the try it out form and using the yaml file provided in the issue description.
Screenshots (if appropriate):
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests