-
Notifications
You must be signed in to change notification settings - Fork 1
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/6 product types api #14
Conversation
@@ -32,10 +32,8 @@ class Tag(BaseModel): | |||
) | |||
type = models.ForeignKey( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nog heel even voor mijn begrip: wat moet ik me voorstellen bij een TagType
? Voelt een beetje als een tag op een tag, was even benieuwd naar de use-case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja klopt dat is het eigenlijk ook. Het komt vanuit Open Inwoner omdat het mij handig leek om de modellen zo veel mogelijk overeen te laten komen voor de integratie straks. Maar er zou ook gewoon één type aan alle tags vanuit Open producten worden gelinkt waardoor het hier weg zou kunnen.
@transaction.atomic() | ||
def create(self, validated_data): | ||
options = validated_data.pop("options") | ||
product_type = validated_data.pop("product_type") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dit veld staat wel vermeld onder exclude
: is veld wel definieren met write_only=True
misschien leesbaarder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er zijn een twee product_type
attributes, price zelf heeft product_type
maar die worden geexclude omdat het al genest is via product_type. de product_type die hier uit de validated_data wordt gehaald word via perform_create
in ProductTypeChildViewSet
toegevoegd en is de parent.
Ik zou het liever niet willen hernoemen omdat ik bang ben dat ik dan alle andere serializers (die geen create/update override nodig hebben) die dit ook gebruiken moet aanpassen.
src/open_producten/producttypes/tests/api/test_category_question.py
Outdated
Show resolved
Hide resolved
src/open_producten/producttypes/tests/api/test_product_type_price.py
Outdated
Show resolved
Hide resolved
Ik heb ook aangepast dat alle errors in de |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ziet er goed uit, nog een klein dingetje, dan kan ik deze snel approven.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional nitpick left, but otherwise LGTM
0789364
to
8235e09
Compare
Co-authored-by: Sidney Richards <[email protected]>
8235e09
to
be49dd2
Compare
Adds initial drf setup, endpoints for the product_types app and api tests