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

Handling content types #109

Merged
merged 9 commits into from
Oct 26, 2023

Conversation

elenaferr0
Copy link
Contributor

I implemented the fix about the content type of the request which was discussed in issue #104. Let me know what you think of this solution, if everything is fine I can implement some more tests @Carapacik.

@Carapacik
Copy link
Owner

Carapacik commented Oct 16, 2023

How about accepting the content type from yaml config so that it is more convenient to make the default type? To avoid writing @Header every time.

And it is better to make it a String type, because there may be many combinations, and it will not be possible to sort everything out.

@elenaferr0
Copy link
Contributor Author

So you mean like setting a default content-type for all requests of a given type? Like for example setting application/json-patch+json for all PATCH requests? Is there a way to do this globally for retrofit (or dio)?
In regards to keeping the content-type as string then yes you're right, I'll fix that as soon as I can.
Thanks!

@Carapacik
Copy link
Owner

So you mean like setting a default content-type for all requests of a given type? Like for example setting application/json-patch+json for all PATCH requests? Is there a way to do this globally for retrofit (or dio)?

Globally in dio init

@elenaferr0
Copy link
Contributor Author

I suppose you mean something like this:

Dio dio = Dio();
dio.options.headers['Content-Type'] = 'application/json';

Or equivalently setting it in Dio constructor.
If there is another way let me know, I searched a bit the documentation but I didn't find anything else.
I want all PATCH endpoints to use application/json-patch+json but all the others should use application/json. The problem with this solution is that (if I understand correctly) this would set the Content-Type header globally (which is not what I want, I'd like to set it only for some specific endpoints). I'm currently creating a single Dio instance with my configuration and interceptors and passing it to all clients.
My other alternative would be to write a Dio interceptor that adds the Content-Type header I want only for PATCH requests, but this is a bit of a workaround and it's not really elegant.

@Carapacik
Copy link
Owner

I mean to make a default value for everything, and generate a header annotation for something different from it.

@Carapacik
Copy link
Owner

If there are any problems, then I can supplement your pr at the end of the week.

@Carapacik Carapacik added the enhancement New feature or request label Oct 19, 2023
@elenaferr0
Copy link
Contributor Author

Oh okay sorry now I get what you're saying. I will implement this in the next days then.
Thank you very much

@Carapacik Carapacik self-assigned this Oct 20, 2023
@Carapacik
Copy link
Owner

@elenaferr0 Can I start my implementation?

@elenaferr0
Copy link
Contributor Author

Yes. Sorry I've been really busy, if you want you can start.

@Carapacik Carapacik requested a review from Sancene October 26, 2023 14:50
@Carapacik Carapacik changed the title Feat/handling content types Handling content types Oct 26, 2023
@Carapacik Carapacik self-requested a review October 26, 2023 14:56
@Carapacik Carapacik requested a review from Sancene October 26, 2023 15:00
@Carapacik Carapacik merged commit bf00eb6 into Carapacik:main Oct 26, 2023
@elenaferr0
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants