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

feat(generator): initial support for typing of Composite Types #1045

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

Conversation

arthur-fontaine
Copy link

Change Summary

This is a draft PR that aims to bring support for composite types (#314).

As I don't know the project well, I am open to receive reviews and guidance on this.

Checklist

  • Unit tests for the changes exist
  • Tests pass without significant drop in coverage
  • Documentation reflects changes where applicable
  • Test snapshots have been updated if applicable

Agreement

By submitting this pull request, I confirm that you can use, modify, copy and redistribute this contribution, under the terms of your choice.

@arthur-fontaine
Copy link
Author

arthur-fontaine commented Nov 18, 2024

@RobertCraigie Can you clarify the purposes of the models defined in types.py and those defined in models.py?
After a little more digging, I have the feeling that the composite types I am injecting in types.py a7f6263 should be injected in their own file composite_types.py.

@RobertCraigie
Copy link
Owner

thanks for working on this!!

Can you clarify the purposes of the models defined in types.py and those defined in models.py?
After a little more digging, I have the feeling that the composite types I am injecting in types.py a7f6263 should be injected in their own file composite_types.py.

Ah yeah models.py is just for the models defined in the schema and types.py is for every other auxilary type we need to generate, e.g. the update params types.

Other types that come directly from the schema have their own file as well, e.g. enums.py so outputting to a separate prisma/composite_types.py file sounds good to me!

@arthur-fontaine
Copy link
Author

Thanks for the clarification. I think that's what I finally did, so it's perfect.

FYI, I just managed to pass the test that validates if we can create and find a record with a composite type. Next I'm going to check if there are any specifications about Prisma composite types, or determine them if there aren't, write the tests and I think we'll be ready for a review.

Capture d’écran 2024-11-19 à 20 07 27

@arthur-fontaine
Copy link
Author

In fact, I don't know if I really need to write tests specifically for MongoDB, since the query is handled by the Prisma engine after all. Maybe it is better to write tests for the query builder?
What do you think about this?

@RobertCraigie
Copy link
Owner

yeah the tests you write for this should just treat composite types as a DB feature and then you'd write tests for the client query methods, e.g. see how the enum tests are written

@arthur-fontaine
Copy link
Author

arthur-fontaine commented Nov 22, 2024

The problem is that there are too many differences between relational databases and MongoDB when using Prisma. This led me to create a new tests_mongodb.
However, since tests for operations like create and find are already implemented for relational databases—and both relational databases and MongoDB rely on the same Prisma Query Engine—I’m questioning whether separate tests for MongoDB are truly necessary. (the MongoDB tests work, but I am not sure it is relevant)

Would it make more sense to focus on unit tests for composite types in test_builder.py instead?

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

Successfully merging this pull request may close these issues.

2 participants