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

Rlecellier/request response serializers #235

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rlecellier
Copy link
Collaborator

Purpose

Improve backend endpoint typing.

@rlecellier rlecellier force-pushed the rlecellier/request_response_serializers branch 2 times, most recently from 3eb67b9 to 69c4d22 Compare March 2, 2023 13:57
Copy link
Collaborator Author

@rlecellier rlecellier left a comment

Choose a reason for hiding this comment

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

--misstake--

@MorganeAD
Copy link
Contributor

Hello !
In this commit I made a proposition of solution on Create Order route

I try to use your proposed request and response serializers for the creation of an Order.
All the cases keeps the same returned response, except the error on billing_address, now it return a dict of error and not a single string, because it's a field with multiple values.
All the old test past (modification of the billing_address error).
The generation of openAPI also works.
The usage of new request serializer add also checks on billing_address and credit_card_id values that was not done before.

I have also two questions on the existing code,

  • if paiement fail because we didn't find the card, we didn't cancel the order ? (see commentary in the code)
  • in joanie, address has a "title" field that is mandatory, does the front return this field when using this route to create an Order ?

@rlecellier rlecellier force-pushed the rlecellier/request_response_serializers branch from dc4e122 to f06533c Compare March 14, 2023 16:28
@rlecellier rlecellier force-pushed the rlecellier/request_response_serializers branch from f06533c to 41f977c Compare March 15, 2023 16:56
We decide to generate our javascript client and typescript types
from swagger open api schema.
That push us to rework our serialization.
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.

2 participants