Skip to content

Conversation

YPCrumble
Copy link

Thanks for maintaining this great repo! This is really an issue, but the fix is easy so thought it better to post as a PR. If I've missed something feel free to close.

Per the docs It's possible to call all() on a many to many attribute. For example, the docs call a1.publications.all() in their example.

I'm seeing this error pop up which I don't believe is a configuration error.

Copy link
Contributor

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Change looks good but you need to add the tests for it as well.

@coveralls
Copy link

coveralls commented Mar 5, 2021

Pull Request Test Coverage Report for Build 1382

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-86.9%) to 0.0%

Totals Coverage Status
Change from base Build 1349: -86.9%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@YPCrumble YPCrumble force-pushed the feature/many-to-many-all branch from b5460ec to c254b57 Compare March 5, 2021 20:35
@YPCrumble YPCrumble force-pushed the feature/many-to-many-all branch from c254b57 to ad8d6b2 Compare March 5, 2021 20:37
@YPCrumble
Copy link
Author

@atodorov thank you! Added the test - I believe this is correct but let me know if it's not? Also added to the CHANGELOG for release today but happy to remove or edit that if you'd prefer.

@YPCrumble
Copy link
Author

@atodorov I'm actually going to close this for now - I'm not 100% sure it's necessary as this is actually an issue with my factories, not a plain Django model.

@YPCrumble YPCrumble closed this Mar 5, 2021
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.

3 participants