feat(ux): enhance media-type switching experience in RequestBodyEditor (#6837)

* feat(ux): enhance media-type switching experience in RequestBodyEditor

1. When canceling the try-out mode the request body will be reset to its initial state.
2. When the user switches the media-type in the try-out mode, the experience is as follows:
   - If the user did edit the request body the body wont be touched and only media type is updated. This is to ensure that user content is NEVER accidentally overwritten with a default value.
   - If the user did not edit the request body it is safe to be replaced by the default value of the target media-type.

Multiple example needed some care in order to allow the retain example value to function properly

* fix(test): workaround cypress issue that can't be reproduced manually

* test: added new feature to ensure enhanced user editing flow

Signed-off-by: mathis-m <mathis.michel@outlook.de>
This commit is contained in:
Mahtis Michel
2021-01-25 20:16:07 +01:00
committed by GitHub
parent a5eb3dc0c3
commit e877580d54
12 changed files with 235 additions and 32 deletions

View File

@@ -36,16 +36,22 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
examples: ImPropTypes.map,
onSelect: PropTypes.func,
updateValue: PropTypes.func, // mechanism to update upstream value
userHasEditedBody: PropTypes.bool,
getComponent: PropTypes.func.isRequired,
currentUserInputValue: PropTypes.any,
currentKey: PropTypes.string,
currentNamespace: PropTypes.string,
setRetainRequestBodyValueFlag: PropTypes.func.isRequired,
// (also proxies props for Examples)
}
static defaultProps = {
userHasEditedBody: false,
examples: Map({}),
currentNamespace: "__DEFAULT__NAMESPACE__",
setRetainRequestBodyValueFlag: () => {
// NOOP
},
onSelect: (...args) =>
console.log( // eslint-disable-line no-console
"ExamplesSelectValueRetainer: no `onSelect` function was provided",
@@ -72,11 +78,16 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
lastDownstreamValue: valueFromExample,
isModifiedValueSelected:
// valueFromExample !== undefined &&
this.props.userHasEditedBody ||
this.props.currentUserInputValue !== valueFromExample,
}),
}
}
componentWillUnmount() {
this.props.setRetainRequestBodyValueFlag(false)
}
_getStateForCurrentNamespace = () => {
const { currentNamespace } = this.props
@@ -122,7 +133,12 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
}
_onExamplesSelect = (key, { isSyntheticChange } = {}, ...otherArgs) => {
const { onSelect, updateValue, currentUserInputValue } = this.props
const {
onSelect,
updateValue,
currentUserInputValue,
userHasEditedBody,
} = this.props
const { lastUserEditedValue } = this._getStateForCurrentNamespace()
const valueFromExample = this._getValueForExample(key)
@@ -141,9 +157,8 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
this._setStateForCurrentNamespace({
lastDownstreamValue: valueFromExample,
isModifiedValueSelected:
isSyntheticChange &&
!!currentUserInputValue &&
currentUserInputValue !== valueFromExample,
(isSyntheticChange && userHasEditedBody) ||
(!!currentUserInputValue && currentUserInputValue !== valueFromExample),
})
// we never want to send up value updates from synthetic changes
@@ -157,7 +172,12 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
componentWillReceiveProps(nextProps) {
// update `lastUserEditedValue` as new currentUserInput values come in
const { currentUserInputValue: newValue, examples, onSelect } = nextProps
const {
currentUserInputValue: newValue,
examples,
onSelect,
userHasEditedBody,
} = nextProps
const {
lastUserEditedValue,
@@ -170,7 +190,7 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
)
const exampleMatchingNewValue = examples.find(
example =>
(example) =>
example.get("value") === newValue ||
// sometimes data is stored as a string (e.g. in Request Bodies), so
// let's check against a stringified version of our example too
@@ -186,15 +206,23 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
newValue !== lastUserEditedValue && // value isn't already tracked
newValue !== lastDownstreamValue // value isn't what we've seen on the other side
) {
this.props.setRetainRequestBodyValueFlag(true)
this._setStateForNamespace(nextProps.currentNamespace, {
lastUserEditedValue: nextProps.currentUserInputValue,
isModifiedValueSelected: newValue !== valueFromCurrentExample,
isModifiedValueSelected:
userHasEditedBody || newValue !== valueFromCurrentExample,
})
}
}
render() {
const { currentUserInputValue, examples, currentKey, getComponent } = this.props
const {
currentUserInputValue,
examples,
currentKey,
getComponent,
userHasEditedBody,
} = this.props
const {
lastDownstreamValue,
lastUserEditedValue,
@@ -212,9 +240,10 @@ export default class ExamplesSelectValueRetainer extends React.PureComponent {
!!lastUserEditedValue && lastUserEditedValue !== lastDownstreamValue
}
isValueModified={
currentUserInputValue !== undefined &&
isModifiedValueSelected &&
currentUserInputValue !== this._getCurrentExampleValue()
(currentUserInputValue !== undefined &&
isModifiedValueSelected &&
currentUserInputValue !== this._getCurrentExampleValue()) ||
userHasEditedBody
}
/>
)

View File

@@ -75,30 +75,29 @@ export default class Parameters extends Component {
}
onChangeMediaType = ({ value, pathMethod }) => {
let { specSelectors, specActions, oas3Selectors, oas3Actions } = this.props
let targetMediaType = value
let currentMediaType = oas3Selectors.requestContentType(...pathMethod)
let schemaPropertiesMatch = specSelectors.isMediaTypeSchemaPropertiesEqual(pathMethod, currentMediaType, targetMediaType)
if (!schemaPropertiesMatch) {
oas3Actions.clearRequestBodyValue({ pathMethod })
let { specActions, oas3Selectors, oas3Actions } = this.props
const userHasEditedBody = oas3Selectors.hasUserEditedBody(...pathMethod)
const shouldRetainRequestBodyValue = oas3Selectors.shouldRetainRequestBodyValue(...pathMethod)
oas3Actions.setRequestContentType({ value, pathMethod })
oas3Actions.initRequestBodyValidateError({ pathMethod })
if (!userHasEditedBody) {
if(!shouldRetainRequestBodyValue) {
oas3Actions.setRequestBodyValue({ value: undefined, pathMethod })
}
specActions.clearResponse(...pathMethod)
specActions.clearRequest(...pathMethod)
specActions.clearValidateParams(pathMethod)
}
oas3Actions.setRequestContentType({ value, pathMethod })
oas3Actions.initRequestBodyValidateError({ pathMethod })
}
render() {
let {
onTryoutClick,
onCancelClick,
parameters,
allowTryItOut,
tryItOutEnabled,
specPath,
fn,
getComponent,
getConfigs,
@@ -131,6 +130,7 @@ export default class Parameters extends Component {
}, {}))
.reduce((acc, x) => acc.concat(x), [])
const retainRequestBodyValueFlagForOperation = (f) => oas3Actions.setRetainRequestBodyValueFlag({ value: f, pathMethod })
return (
<div className="opblock-section">
<div className="opblock-section-header">
@@ -155,7 +155,13 @@ export default class Parameters extends Component {
</div>
)}
{allowTryItOut ? (
<TryItOutButton enabled={tryItOutEnabled} onCancelClick={onCancelClick} onTryoutClick={onTryoutClick} />
<TryItOutButton
isOAS3={specSelectors.isOAS3()}
hasUserEditedBody={oas3Selectors.hasUserEditedBody(...pathMethod)}
enabled={tryItOutEnabled}
onCancelClick={this.props.onCancelClick}
onTryoutClick={onTryoutClick}
onResetClick={() => oas3Actions.setRequestBodyValue({ value: undefined, pathMethod })}/>
) : null}
</div>
{this.state.parametersVisible ? <div className="parameters-container">
@@ -219,6 +225,8 @@ export default class Parameters extends Component {
</div>
<div className="opblock-description-wrapper">
<RequestBody
setRetainRequestBodyValueFlag={retainRequestBodyValueFlagForOperation}
userHasEditedBody={oas3Selectors.hasUserEditedBody(...pathMethod)}
specPath={specPath.slice(0, -1).push("requestBody")}
requestBody={requestBody}
requestBodyValue={oas3Selectors.requestBodyValue(...pathMethod)}

View File

@@ -5,24 +5,35 @@ export default class TryItOutButton extends React.Component {
static propTypes = {
onTryoutClick: PropTypes.func,
onResetClick: PropTypes.func,
onCancelClick: PropTypes.func,
enabled: PropTypes.bool, // Try it out is enabled, ie: the user has access to the form
hasUserEditedBody: PropTypes.bool, // Try it out is enabled, ie: the user has access to the form
isOAS3: PropTypes.bool, // Try it out is enabled, ie: the user has access to the form
};
static defaultProps = {
onTryoutClick: Function.prototype,
onCancelClick: Function.prototype,
onResetClick: Function.prototype,
enabled: false,
hasUserEditedBody: false,
isOAS3: false,
};
render() {
const { onTryoutClick, onCancelClick, enabled } = this.props
const { onTryoutClick, onCancelClick, onResetClick, enabled, hasUserEditedBody, isOAS3 } = this.props
const showReset = isOAS3 && hasUserEditedBody
return (
<div className="try-out">
<div className={showReset ? "try-out btn-group" : "try-out"}>
{
enabled ? <button className="btn try-out__btn cancel" onClick={ onCancelClick }>Cancel</button>
: <button className="btn try-out__btn" onClick={ onTryoutClick }>Try it out </button>
}
{
showReset && <button className="btn try-out__btn reset" onClick={ onResetClick }>Reset</button>
}
</div>
)

View File

@@ -3,6 +3,7 @@
export const UPDATE_SELECTED_SERVER = "oas3_set_servers"
export const UPDATE_REQUEST_BODY_VALUE = "oas3_set_request_body_value"
export const UPDATE_REQUEST_BODY_VALUE_RETAIN_FLAG = "oas3_set_request_body_retain_flag"
export const UPDATE_REQUEST_BODY_INCLUSION = "oas3_set_request_body_inclusion"
export const UPDATE_ACTIVE_EXAMPLES_MEMBER = "oas3_set_active_examples_member"
export const UPDATE_REQUEST_CONTENT_TYPE = "oas3_set_request_content_type"
@@ -26,6 +27,14 @@ export function setRequestBodyValue ({ value, pathMethod }) {
}
}
export const setRetainRequestBodyValueFlag = ({ value, pathMethod }) => {
return {
type: UPDATE_REQUEST_BODY_VALUE_RETAIN_FLAG,
payload: { value, pathMethod }
}
}
export function setRequestBodyInclusion ({ value, pathMethod, name }) {
return {
type: UPDATE_REQUEST_BODY_INCLUSION,

View File

@@ -17,6 +17,7 @@ export default class RequestBodyEditor extends PureComponent {
static defaultProps = {
onChange: NOOP,
userHasEditedBody: false,
};
constructor(props, context) {
@@ -65,7 +66,7 @@ export default class RequestBodyEditor extends PureComponent {
})
}
if(!nextProps.value && nextProps.defaultValue && !!this.state.value) {
// if new value is falsy, we have a default, AND the falsy value didn't
@@ -77,7 +78,7 @@ export default class RequestBodyEditor extends PureComponent {
render() {
let {
getComponent,
errors
errors,
} = this.props
let {

View File

@@ -4,7 +4,7 @@ import ImPropTypes from "react-immutable-proptypes"
import { Map, OrderedMap, List } from "immutable"
import { getCommonExtensions, getSampleSchema, stringify, isEmptyValue } from "core/utils"
function getDefaultRequestBodyValue(requestBody, mediaType, activeExamplesKey) {
export const getDefaultRequestBodyValue = (requestBody, mediaType, activeExamplesKey) => {
const mediaTypeValue = requestBody.getIn(["content", mediaType])
const schema = mediaTypeValue.get("schema").toJS()
@@ -32,6 +32,7 @@ function getDefaultRequestBodyValue(requestBody, mediaType, activeExamplesKey) {
const RequestBody = ({
userHasEditedBody,
requestBody,
requestBodyValue,
requestBodyInclusionSetting,
@@ -47,6 +48,7 @@ const RequestBody = ({
onChangeIncludeEmpty,
activeExamplesKey,
updateActiveExamplesKey,
setRetainRequestBodyValueFlag
}) => {
const handleFile = (e) => {
onChange(e.target.files[0])
@@ -222,6 +224,7 @@ const RequestBody = ({
{
examplesForMediaType ? (
<ExamplesSelectValueRetainer
userHasEditedBody={userHasEditedBody}
examples={examplesForMediaType}
currentKey={activeExamplesKey}
currentUserInputValue={requestBodyValue}
@@ -229,6 +232,7 @@ const RequestBody = ({
updateValue={onChange}
defaultToFirstExample={true}
getComponent={getComponent}
setRetainRequestBodyValueFlag={setRetainRequestBodyValueFlag}
/>
) : null
}
@@ -276,6 +280,7 @@ const RequestBody = ({
}
RequestBody.propTypes = {
userHasEditedBody: PropTypes.bool.isRequired,
requestBody: ImPropTypes.orderedMap.isRequired,
requestBodyValue: ImPropTypes.orderedMap.isRequired,
requestBodyInclusionSetting: ImPropTypes.Map.isRequired,
@@ -291,6 +296,8 @@ RequestBody.propTypes = {
specPath: PropTypes.array.isRequired,
activeExamplesKey: PropTypes.string,
updateActiveExamplesKey: PropTypes.func,
setRetainRequestBodyValueFlag: PropTypes.func,
oas3Actions: PropTypes.object.isRequired
}
export default RequestBody

View File

@@ -10,7 +10,7 @@ import {
UPDATE_RESPONSE_CONTENT_TYPE,
SET_REQUEST_BODY_VALIDATE_ERROR,
CLEAR_REQUEST_BODY_VALIDATE_ERROR,
CLEAR_REQUEST_BODY_VALUE,
CLEAR_REQUEST_BODY_VALUE, UPDATE_REQUEST_BODY_VALUE_RETAIN_FLAG,
} from "./actions"
export default {
@@ -42,6 +42,10 @@ export default {
})
return state.setIn(["requestData", path, method, "bodyValue"], newVal)
},
[UPDATE_REQUEST_BODY_VALUE_RETAIN_FLAG]: (state, { payload: { value, pathMethod } } ) =>{
let [path, method] = pathMethod
return state.setIn(["requestData", path, method, "retainBodyValue"], value)
},
[UPDATE_REQUEST_BODY_INCLUSION]: (state, { payload: { value, pathMethod, name } } ) =>{
let [path, method] = pathMethod
return state.setIn( [ "requestData", path, method, "bodyInclusion", name ], value)

View File

@@ -1,5 +1,7 @@
import { OrderedMap, Map } from "immutable"
import { OrderedMap, Map, List } from "immutable"
import { isOAS3 as isOAS3Helper } from "./helpers"
import { getDefaultRequestBodyValue } from "./components/request-body"
import { stringify } from "../../utils"
// Helpers
@@ -54,6 +56,43 @@ export const requestBodyValue = onlyOAS3((state, path, method) => {
}
)
export const shouldRetainRequestBodyValue = onlyOAS3((state, path, method) => {
return state.getIn(["requestData", path, method, "retainBodyValue"]) || false
}
)
export const hasUserEditedBody = (state, path, method) => (system) => {
const {oas3Selectors, specSelectors} = system.getSystem()
const spec = specSelectors.specJson()
if(isOAS3Helper(spec)) {
let userHasEditedBody = false
const currentMediaType = oas3Selectors.requestContentType(path, method)
let userEditedRequestBody = oas3Selectors.requestBodyValue(path, method)
if (Map.isMap(userEditedRequestBody)) {
// context is not application/json media-type
userEditedRequestBody = stringify(userEditedRequestBody.mapEntries((kv) => Map.isMap(kv[1]) ? [kv[0], kv[1].get("value")] : kv).toJS())
}
if(List.isList(userEditedRequestBody)) {
userEditedRequestBody = stringify(userEditedRequestBody)
}
if (currentMediaType) {
const currentMediaTypeDefaultBodyValue = getDefaultRequestBodyValue(
specSelectors.specResolvedSubtree(["paths", path, method, "requestBody"]),
currentMediaType,
oas3Selectors.activeExamplesMember(
path, method,
"requestBody",
"requestBody",
)
)
userHasEditedBody = !!userEditedRequestBody && userEditedRequestBody !== currentMediaTypeDefaultBodyValue
}
return userHasEditedBody
} else {
return null
}
}
export const requestBodyInclusionSetting = onlyOAS3((state, path, method) => {
return state.getIn(["requestData", path, method, "bodyInclusion"]) || Map()
}

View File

@@ -525,7 +525,7 @@ export const isMediaTypeSchemaPropertiesEqual = ( state, pathMethod, currentMedi
}
let currentMediaTypeSchemaProperties = requestBodyContent.getIn([currentMediaType, "schema", "properties"], fromJS([]))
let targetMediaTypeSchemaProperties = requestBodyContent.getIn([targetMediaType, "schema", "properties"], fromJS([]))
return currentMediaTypeSchemaProperties.equals(targetMediaTypeSchemaProperties) ? true: false
return !!currentMediaTypeSchemaProperties.equals(targetMediaTypeSchemaProperties)
}
function returnSelfOrNewMap(obj) {

View File

@@ -13,6 +13,10 @@
flex-direction: column;
}
.try-out.btn-group {
padding: 0;
}
.opblock-tag
{
display: flex;
@@ -668,7 +672,7 @@
overflow-y: auto;
max-height: 400px;
min-height: 6em;
code {
white-space: pre-wrap !important;
word-break: break-all;

View File

@@ -22,13 +22,17 @@ describe("OpenAPI 3.0 Multiple Media Types with different schemas", () => {
cy.get(".opblock-section-request-body .content-type").as("selectMediaType")
})
// In all cases,
// In all cases,
// - assume that examples are populated based on schema (not explicitly tested)
// - assume validation passes based on successful "execute"
// - expect final cURL command result doees not contain unexpected artifacts from other content-type schemas
describe("multipart/form-data (only 'bar')", () => {
it("should execute multipart/form-data", () => {
cy.get("@selectMediaType")
.select(mediaTypeUrlencoded)
.get("@executeBtn")
.click()
.get("@selectMediaType")
.select(mediaTypeFormData)
.get("@executeBtn")
.click()

View File

@@ -0,0 +1,87 @@
const getRequestBodyFromCY = (page = null) =>
(page || cy.visit(
"/?url=/documents/features/petstore-only-pet.openapi.yaml",
))
.get("#operations-pet-addPet")
.click()
// Expand Try It Out
.get(".try-out__btn")
.click()
// get textarea
.get(".opblock-body .opblock-section .opblock-section-request-body .body-param textarea")
const xmlIndicator = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
const userEditXmlSample = xmlIndicator +
"<pet>\n" +
"\t<id>420</id>\n" +
"\t<name>doggie<3</name>\n" +
"\t<category>\n" +
"\t\t<id>99999999999</id>\n" +
"\t\t<name>Dogiiiiiiiieeee</name>\n" +
"\t</category>\n" +
"\t<photoUrls>\n" +
"\t\t<photoUrl>string</photoUrl>\n" +
"\t</photoUrls>\n" +
"\t<tags>\n" +
"\t\t<tag>\n" +
"\t\t\t<id>0</id>\n" +
"\t\t\t<name>string</name>\n" +
"\t\t</tag>\n" +
"\t</tags>\n" +
"\t<status>available</status>\n" +
"</pet>"
describe("OAS3 Request Body user edit flows", () => {
// Case: Copy xml from email, paste into request body editor, change media-type to xml
it("it should never overwrite user edited value in case of media-type change", () => {
getRequestBodyFromCY()
// replace default sample with xml edited value
.type(`{selectall}${userEditXmlSample}`)
// change media type to xml, because I have forgotten it
.get(".opblock-section .opblock-section-request-body .body-param-content-type > select")
.select("application/xml")
// Ensure user edited body is not overwritten
.get(".opblock-section-request-body")
.within(() => {
cy
.get("textarea")
.should(($div) => {
expect($div.get(0).textContent).to.eq(userEditXmlSample)
})
})
})
// Case: User really wants to try out the brand new xml content-type
it("it should overwrite default value in case of content-type change, even within request body editor(#6836)", () => {
getRequestBodyFromCY()
// change media type to xml, because I have forgotten it (sry really wanted to try out the new xml content-type)
.get(".opblock-section .opblock-section-request-body .body-param-content-type > select")
.select("application/xml")
// Ensure default value is xml after content type change
.get(".opblock-section-request-body")
.within(() => {
cy
.get("textarea")
.should(($div) => {
expect($div.get(0).textContent).to.contain(xmlIndicator)
})
})
})
// Case: User wants to get the default value back
it("it reset the user edited value and render the default value in case of try out reset. (#6517)", () => {
getRequestBodyFromCY()
// replace default sample with bad value
.type("{selectall}ups that should not have happened")
// Cancel Try It Out
.get(".try-out__btn.reset")
.click()
// Ensure default value is xml after content type change
.get(".opblock-section-request-body")
.within(() => {
cy
.get("textarea")
.should(($div) => {
expect($div.get(0).textContent).to.not.contain("ups that should not have happened")
})
})
})
})