django-simple-history icon indicating copy to clipboard operation
django-simple-history copied to clipboard

Use tuple for m2m_models

Open martimarkov opened this issue 10 months ago • 3 comments
trafficstars

Changes how the m2m_models is defined.

Description

Uses a tuple containing the object name and the field.

Related Issue

https://github.com/jazzband/django-simple-history/issues/1435

Motivation and Context

Ensures that there are no clashes when the m2m relationship is in a parent model and there are multiple child models.

How Has This Been Tested?

Ran all the tests in the suite and no issues came up.

Screenshots (if appropriate):

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x] I have run the pre-commit run command to format and lint.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [x] I have added my name and/or github handle to AUTHORS.rst
  • [ ] I have added my change to CHANGES.rst
  • [x] All new and existing tests passed.

martimarkov avatar Jan 12 '25 11:01 martimarkov

Until the tests run for this, we can't move forward with it. This likely means it's blocked by https://github.com/jazzband/help/issues/382

That said, this will need tests confirming the fix from #1435, documentation changes and maybe some way to avoid being backwards incompatible.

tim-schilling avatar Jan 24 '25 13:01 tim-schilling

@tim-schilling so you want me to add tests that cause #1435 as an issue and this to fix them? I'm not sure I'll be able to reproduce it correctly but will think about how to do it

martimarkov avatar Feb 07 '25 11:02 martimarkov

Yes, that's correct. Thank you!

tim-schilling avatar Feb 08 '25 12:02 tim-schilling