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

Make AddRelatedManagers look for "objects" on parent model #730

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

RJPercival
Copy link
Contributor

@RJPercival RJPercival commented Oct 18, 2021

Previously, AddRelatedManagers would fail if a related model had inherited its objects field from a parent class. This would result in missing relation attributes. This is fixed by using get() instead of names; the former searches the MRO for the symbol, whereas the latter only looks for symbols declared directly on the class.

Related issues

Closes #652

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 a lot, let's see what CI will say.

tests/typecheck/fields/test_related.yml Outdated Show resolved Hide resolved
@RJPercival
Copy link
Contributor Author

Looks like CI is broken @sobolevn.

@sobolevn
Copy link
Member

sobolevn commented Oct 19, 2021

I am also fixing the CI failure in #734 🙂

Fails with:
```
pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output:
Expected:
  main:2: note: Revealed type is "myapp.models.MyUser*"
  main:3: note: Revealed type is "myapp.models.MyUser*"
  <45 (diff)
  <45 (diff)
Actual:
  main:2: note: Revealed type is "myapp.models.MyUser*"
  main:3: note: Revealed type is "myapp.models.MyUser*"
  main:6: error: "MyUser" has no attribute "book_set" (diff)
  main:6: note: Revealed type is "Any"          (diff)
  main:7: error: "MyUser" has no attribute "article_set" (diff)
  main:7: note: Revealed type is "Any"          (diff)
```
Previously, AddRelatedManagers would fail if a related model had inherited
its `objects` field from a parent class. This would result in missing
relation attributes. This is fixed by using `get()` instead of `names`;
the former searches the MRO for the symbol, whereas the latter only looks
for symbols declared directly on the class.
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 a lot for working on it! 👍

@sobolevn sobolevn merged commit 9938378 into typeddjango:master Oct 19, 2021
@RJPercival RJPercival deleted the inherited_objects branch October 19, 2021 15:58
@RJPercival
Copy link
Contributor Author

No problem 🙂 Any idea when the next release is likely to be?

@sobolevn
Copy link
Member

We need to fix #725 before releasing a new one 😞

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.

Reverse relation is lost for abstract models with custom querysets
2 participants