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

Cannot resolve a custom through model passed as app_label.ModelName for m2m relations #1266

Open
antoineauger opened this issue Sep 28, 2023 · 8 comments

Comments

@antoineauger
Copy link

PS: this is a corner case of #1093. I made some experiments in #1149 before creating this issue because I don't have the required knowledge to fix it.

Describe the bug

django-simple-history cannot resolve a custom through model passed as app_label.ModelName for m2m relations.

To Reproduce

When trying to pass the Membership model as a tests.Membership string, this produces an error when testing inside the project repository with python3 runtests.py:

class Membership(models.Model):
    group = models.ForeignKey("Group", on_delete=models.CASCADE)
    person = models.ForeignKey("Person", on_delete=models.CASCADE)
    inviter = models.ForeignKey(
        "Person",
        on_delete=models.CASCADE,
        related_name="membership_invites",
    )
    invite_reason = models.CharField(max_length=64)


class Group(models.Model):
    name = models.CharField(max_length=128)
    members = models.ManyToManyField(
        "Person",
        through="tests.Membership",
        through_fields=("group", "person"),
    )

    history = HistoricalRecords(m2m_fields=[members])

Test output:

python3 runtests.py                                                             
Traceback (most recent call last):
  File "/home/antoine/_Github/django-simple-history/runtests.py", line 193, in <module>
    main()
  File "/home/antoine/_Github/django-simple-history/runtests.py", line 180, in main
    django.setup()
  File "/home/antoine/.local/lib/python3.10/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/antoine/.local/lib/python3.10/site-packages/django/apps/registry.py", line 116, in populate
    app_config.import_models()
  File "/home/antoine/.local/lib/python3.10/site-packages/django/apps/config.py", line 269, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/antoine/_Github/django-simple-history/simple_history/tests/models.py", line 159, in <module>
    class Group(models.Model):
  File "/home/antoine/.local/lib/python3.10/site-packages/django/db/models/base.py", line 365, in __new__
    new_class._prepare()
  File "/home/antoine/.local/lib/python3.10/site-packages/django/db/models/base.py", line 428, in _prepare
    class_prepared.send(sender=cls)
  File "/home/antoine/.local/lib/python3.10/site-packages/django/dispatch/dispatcher.py", line 176, in send
    return [
  File "/home/antoine/.local/lib/python3.10/site-packages/django/dispatch/dispatcher.py", line 177, in <listcomp>
    (receiver, receiver(signal=self, sender=sender, **named))
  File "/home/antoine/_Github/django-simple-history/simple_history/models.py", line 223, in finalize
    m2m_model = self.create_history_m2m_model(
  File "/home/antoine/_Github/django-simple-history/simple_history/models.py", line 258, in create_history_m2m_model
    fields = self.copy_fields(through_model)
  File "/home/antoine/_Github/django-simple-history/simple_history/models.py", line 330, in copy_fields
    for field in self.fields_included(model):
  File "/home/antoine/_Github/django-simple-history/simple_history/models.py", line 313, in fields_included
    for field in model._meta.fields:
AttributeError: 'str' object has no attribute '_meta'

Expected behavior

It correctly works when passing the Membership model as a Python class:

class Membership(models.Model):
    group = models.ForeignKey("Group", on_delete=models.CASCADE)
    person = models.ForeignKey("Person", on_delete=models.CASCADE)
    inviter = models.ForeignKey(
        "Person",
        on_delete=models.CASCADE,
        related_name="membership_invites",
    )
    invite_reason = models.CharField(max_length=64)


class Group(models.Model):
    name = models.CharField(max_length=128)
    members = models.ManyToManyField(
        "Person",
        through=Membership,
        through_fields=("group", "person"),
    )

    history = HistoricalRecords(m2m_fields=[members])

Environment (please complete the following information):

  • OS: Ubuntu 22.04.3 LTS
  • Django Simple History Version: 3.4.0
  • Django Version: 4.2.5
@antoineauger
Copy link
Author

/cc @ddabble

@antoineauger
Copy link
Author

This is still a problem despite the fixes brought by release 3.5.0.

@ddabble
Copy link
Member

ddabble commented Feb 26, 2024

Yeah, this wasn't addressed in 3.5.0, but it's on my personal TODO list - provided that no one fixes it before I get to it 🙂

@skyward-luke
Copy link

any updates on this?

trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
@trumpet2012
Copy link
Contributor

I have a proposed fix for this here: #1390. Feedback and any additional testing are welcome.

trumpet2012 added a commit to trumpet2012/django-simple-history that referenced this issue Sep 12, 2024
…ings

jazzband#1266 resolving through tables defined as strings on m2m relations
@Arti3DPlayer
Copy link

+1, not working with through models defined as string

@whiffsalot
Copy link

I have a proposed fix for this here: #1390. Feedback and any additional testing are welcome.

Would love to see this get merged! This was blocking us from having history working reliably on some of our models.

@nagababa123
Copy link

m2m_fields is working with through if your model class is defined like this i.e. non string format:

slides = models.ManyToManyField( "MetaSlide", related_name="+", through=MetaSlideCourse, )

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

No branches or pull requests

7 participants