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

Check correct model on other side of many to many reverse filtering #2283

Merged
merged 5 commits into from
Jul 28, 2024

Conversation

flaeppe
Copy link
Member

@flaeppe flaeppe commented Jul 28, 2024

Depending on the manager type there could be different model arguments. For a ManyRelatedManager the argument is the through model, but we want to check the to model.

As a simple starter we stash the to model in the manager's metadata and set a precedence on fetching the to model from metadata.

Related issues

flaeppe added 2 commits July 28, 2024 16:09
Depending on the manager type there could be different model arguments.
For a `ManyRelatedManager` the argument is the `through` model, but we
want to check the `to` model.

As a simple starter we stash the `to` model in the manager's metadata
and set a precedence on fetching the `to` model from metadata.
Comment on lines +1483 to +1485
other.mymodel_set.filter(xyz__isnull=True) # E: Cannot resolve keyword 'xyz' into field. Choices are: id, others [misc]
other.mymodel_set.get(xyz__isnull=True) # E: Cannot resolve keyword 'xyz' into field. Choices are: id, others [misc]
other.mymodel_set.exclude(xyz__isnull=True) # E: Cannot resolve keyword 'xyz' into field. Choices are: id, others [misc]
Copy link
Member Author

Choose a reason for hiding this comment

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

We actually had incorrect tests for the issue fixed here .. 🙂

@flaeppe
Copy link
Member Author

flaeppe commented Jul 28, 2024

@sobolevn it might be a bunch of false negatives due to the issue here that will come from 5.0.3. I suppose we could chose to do a release if we get a bunch of reported issues about incorrect errors reported for model fields via many to many field filtering.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

tests/typecheck/fields/test_related.yml Outdated Show resolved Hide resolved
@flaeppe flaeppe merged commit f48e722 into typeddjango:master Jul 28, 2024
36 checks passed
@flaeppe flaeppe deleted the fix/check-reverse-m2m-to-fields branch July 28, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ManyToMany relation wrongly validates values_list field name
2 participants