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

feature: Multi-part requests: user should be able to set content-type for each part in a multi-part request. #1602 #2121

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

end3rbyte
Copy link
Contributor

@end3rbyte end3rbyte commented Apr 18, 2024

Multi-part requests: user should be able to set content-type for each part in a multi-part request (Issues: #1602)

Fixes #1602

Description

An optional column "Content-Type" was added.

image

Example how the content type is saved in the .bru file:

image

multi-body

Demo:

multipart.bruno.demo.mp4

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

@end3rbyte end3rbyte marked this pull request as ready for review April 18, 2024 13:47
@end3rbyte
Copy link
Contributor Author

Ready for Review

@end3rbyte
Copy link
Contributor Author

The CI is failing because I have added a new endpoint to the CLI backend, end the CI run against the current prod backend which has not yet the endpoint I added
image
Can we continue the workflow to merge the PR ?

@end3rbyte
Copy link
Contributor Author

Hi @Its-treason
Would it be possible to review this PR please ? It's assigned to you.
The issue fixed by this PR is pretty annoying with java backends because json needs to have the proper content type or it fails.
Thanks

Copy link
Member

@Its-treason Its-treason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, functionality wise.

One thing I don't really like is how the (Content-Type=text/plain) is just appended to the end. But I also understand, that it cannot be done better with the current Bru-Lang. Hope this can be refactored with the new Bru-Lang.

@MrFishchev
Copy link

Hello, will be there a possibility to upload a file as application/octet-stream (as an entire request), not like multipart-form data?

E.g. as Postman has:
image

@end3rbyte
Copy link
Contributor Author

No, but it makes sens, so i suggest you to create an enhancement ticket for it, if there is no ticket already.

It should be added in this list

image

With a file selector to select the file to upload

@timothystone-knsl
Copy link

excited for this feature... it's almost a blocker for us in our abandonment of Postman.

@helloanoop helloanoop self-assigned this Jun 11, 2024
@end3rbyte end3rbyte closed this Jul 1, 2024
@end3rbyte end3rbyte force-pushed the feature/1602-multipart-content-type branch from 51afc5f to 02e23df Compare July 1, 2024 13:06
# Conflicts:
#	packages/bruno-lang/v2/src/bruToJson.js
#	packages/bruno-lang/v2/src/jsonToBru.js
@end3rbyte end3rbyte reopened this Jul 1, 2024
@end3rbyte
Copy link
Contributor Author

end3rbyte commented Jul 1, 2024

The tests are failing but they will pass as soon as this PR is merged. It is because I added a new route in the CLI test backend:
https://github.com/usebruno/bruno/pull/2121/files#diff-28dd2fe431464ec2945e9ca3470f5ae58a0be3b7c46b65980625150314b2988b

image

@end3rbyte
Copy link
Contributor Author

Hi @helloanoop I fixed the conflicts and retested.
Could we please have this PR merged ?

@danishmughal
Copy link

Looking forward to this feature being merged - thanks for working on this @end3rbyte !

@lee-dongyeop
Copy link

lee-dongyeop commented Aug 6, 2024

@end3rbyte Hi, I'm waiting for this feature, can I know when it will be reflected?

@timothystone-knsl
Copy link

🙏 @end3rbyte do the conflicts need to be resolved before the merge can happen?

@tktsour
Copy link

tktsour commented Aug 23, 2024

This feature and also sending raw files as a request, are our only holdbacks from switching to bruno, hopefully we can have these on main asap <3 @helloanoop

@helloanoop
Copy link
Contributor

Thank you all for your patience. The PR looks good overall. We are currently reviewing it and will provide an update in the next few days.

# Conflicts:
#	packages/bruno-lang/v2/src/jsonToBru.js
@end3rbyte
Copy link
Contributor Author

I've sync and fixed the conflict

@lee-dongyeop
Copy link

@end3rbyte @helloanoop The �confict is said to have been fixed, so please merge.
I'm waiting for the feature. Thank you. <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi-part requests: user should be able to set content-type for each part in a multi-part request.
9 participants