swagger-ui
swagger-ui copied to clipboard
Fix reset button in Try It Out form
Description
The reset button needed the following two corrections:
- The
oas3Actions.setRequestBodyValueexpects a orderedMap/Map and was currently being passed a string. So a conversion from string to orderedMap was added. - The
UPDATE_REQUEST_BODY_VALUEaction 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. - Furthermore, the loop to copy over the values overwrote the previous iteration changes due to it using the same unchanged variable every loop. The variable is now updated and retained every iteration to preserve changes from previous iterations.
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...
- [ ] No code changes (
src/is unmodified: changes to documentation, CI, metadata, etc.) - [ ] Dependency changes (any modification to dependencies in
package.json) - [x] Bug fixes (non-breaking change which fixes an issue)
- [ ] Improvements (misc. changes to existing features)
- [ ] Features (non-breaking change which adds functionality)
My changes...
- [ ] are breaking changes to a public API (config options, System API, major UI change, etc).
- [ ] are breaking changes to a private API (Redux, component props, utility functions, etc.).
- [ ] are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
- [x] are not breaking changes.
Documentation
- [x] My changes do not require a change to the project documentation.
- [ ] My changes require a change to the project documentation.
- [ ] If yes to above: I have updated the documentation accordingly.
Automated tests
- [ ] My changes can not or do not need to be tested.
- [ ] My changes can and should be tested by unit and/or integration tests.
- [ ] If yes to above: I have added tests to cover my changes.
- [ ] If yes to above: I have taken care to cover edge cases in my tests.
- [x] All new and existing tests passed.
LGTM thanks for your efforts and for advancing further the reset functionality. Didn't noticed that when implementing #6837
Maybe you could add a test for regression
LGTM thanks for your efforts and for advancing further the reset functionality. Didn't noticed that when implementing #6837
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 oas3-user-edit-request-body-flows.cy.js. Should that be edited or a new one be added.
mathis-m
I have added a cypress test for regression. Kindly review the changes.
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
UserSourceschema there hasmaxPropertiesset 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 insampleFromSchemaGeneric.
Thanks for the review @glowcloud . I will take a look at this issue.
I created a separate issue for maxProperties not being validated by Swagger UI https://github.com/swagger-api/swagger-ui/issues/9675
The issue with only resetting some parameters seems related to this PR/issue, so leaving it here for now.
I created a separate issue for
maxPropertiesnot being validated by Swagger UI #9675The 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
Thanks for trying to fix the issue with maxProperties, but the sampleFromSchemaGeneric function works correctly - we should generate an example with limited amount of properties. You can see that it does so if you change the content type to application/json instead of application/x-www-form-urlencoded.
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: https://github.com/swagger-api/swagger-ui/blob/94f2d8298a60a6cf29c1be84e622f476c9c1df91/src/core/plugins/oas3/components/request-body.jsx#L153
so we don’t see that the maxProperties limit exists in the parent schema.
As for the fix for resetting, it was superseded by https://github.com/swagger-api/swagger-ui/pull/9717 with some fixes. Your contribution will be properly credited.