Skip to content
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

Fixes #922: Clear Content-Type when selecting 'No Body' #1214

Merged
merged 2 commits into from Oct 20, 2018

Conversation

@thewheat
Copy link
Contributor

thewheat commented Oct 15, 2018

Closes #922

Summary

  • Removes Content-Type header when selecting No Body from the request dropdown

image

Demo
remove-content-type

Implementation details and notes

  • Created CONTENT_TYPE_NO_BODY constant to make code a bit clearer (no longer needing to check for typeof mimeType
  • Auto linting seems to cause most of the changes seen
  • There was a keeps content-type test that I had to remove as this change removes that behaviour for null mimeTypes (unsure if this behaviour is something we'd like to keep, if we do want to keep this, I'll need to rethink this approach)
@ryanprior

This comment has been minimized.

Copy link

ryanprior commented Oct 15, 2018

Thanks for the patch @thewheat, this was an annoyance for me a few times.

Copy link
Contributor

gschier left a comment

Thanks for the fix @thewheat! Just a minor comment.

@@ -294,8 +282,9 @@ export function updateMimeType(
// 1. Update Content-Type header //
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ //

const hasBody = typeof mimeType === 'string';
if (!hasBody || mimeType === CONTENT_TYPE_OTHER) {
if (mimeType === CONTENT_TYPE_NO_BODY) {

This comment has been minimized.

Copy link
@gschier

gschier Oct 17, 2018

Contributor

You pretty much got this right, although it's a bit simpler than what you did. Technically the Request.body attribute can be {}. This is why the hasBody variable was set to typeof mimeType === 'string'.

That means that your change can be simplified to the following:

Suggested change
if (mimeType === CONTENT_TYPE_NO_BODY) {
if (!hasBody) {
headers = headers.filter(h => h !== contentTypeHeader);
if (mimeType === CONTENT_TYPE_OTHER) {

This comment has been minimized.

Copy link
@gschier

gschier Oct 17, 2018

Contributor

I think this is actually the only change needed for this PR besides updating the tests.

@thewheat thewheat force-pushed the thewheat:remove-content-type-for-no-body branch from 6099abe to c661dcd Oct 20, 2018
@thewheat thewheat force-pushed the thewheat:remove-content-type-for-no-body branch from c661dcd to c97277f Oct 20, 2018
Copy link
Contributor

gschier left a comment

Awesome! Thanks for this 😄 🎆

@gschier gschier merged commit 2b3591f into Kong:develop Oct 20, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
luizmariz pushed a commit to luizmariz/insomnia that referenced this pull request Jan 22, 2020
* Clear Content-Type when selecting 'No Body'

* Remove CONTENT_TYPE_NO_BODY and simplify filter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.