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

chore(docs): added example for m2m through fields #116

Closed
wants to merge 1 commit into from

Conversation

Plebbimon
Copy link
Contributor

No description provided.

@Plebbimon
Copy link
Contributor Author

Looks like flake8 has moved repo and the config needs to be updated:
https://stackoverflow.com/questions/74843772/pre-commit-suddenly-fails-to-install-flake8

Comment on lines +288 to +289
Using `graphene-django-cud`, a `through` model is more akin to a many to one relationship, as it is
an intermediate model that connects two other models with itself.
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should rewrite this to specify that this is how Django handles through fields internally (via a ManyToOneRel, although we don't need to specify that). This leads to many_to_one_extras being a more viable option to use with through intermediary models.

It is not something we enforce, it is something Django itself enforces.

Comment on lines +293 to +294
There's also usually no need to specify the `many_to_many_extras` field, as
the `through` model will be used instead.
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe expand on this to note that the m2m-field will be autogenerated with the input schema, but that it is less useful when using a through-model—and that excluding it might be a good idea.

We should also probably consider auto-excluding the m2m-variants from the schema when encountering a m2m-relation with a through.

@tOgg1
Copy link
Owner

tOgg1 commented Jan 15, 2024

Closing due to inactivity. Feel free to continue on the branch and file a new PR when the comments are adressed.

@tOgg1 tOgg1 closed this Jan 15, 2024
@Plebbimon Plebbimon mentioned this pull request Jan 15, 2024
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