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

M2M does not work with multi-model inheritance #1435

Open
martimarkov opened this issue Jan 9, 2025 · 1 comment
Open

M2M does not work with multi-model inheritance #1435

martimarkov opened this issue Jan 9, 2025 · 1 comment

Comments

@martimarkov
Copy link

Describe the bug
M2M does not work if inheriting more than once the base class

To Reproduce
Steps to reproduce the behavior:

class ModelBase(models.Model):
    contact_new = models.ManyToManyField(Contact, verbose_name=_('Contact'), blank=True)


class ApartmentOffer(ModelBase):
    pass

class HouseOffer(ModelBase):
    pass


simple_history.register(ModelBase, cascade_delete_history=True)
simple_history.register(ApartmentOffer, m2m_fields=["contact_new"], cascade_delete_history=True)
simple_history.register(HouseOffer, m2m_fields=["contact_new"], cascade_delete_history=True)

Error:

Cannot assign "<HistoricalApartmentOffer: Apartment Offer - 8988518 as of 2025-01-09 23:00:16.431927+00:00>": "HistoricalPropertyOffer_contact_new.history" must be a "HistoricalPropertyOffer" instance.

Expected behavior
The object should be saved successfully.

Environment (please complete the following information):

  • Django Simple History Version: 3.7.0
  • Django Version: 5.0
@martimarkov
Copy link
Author

martimarkov commented Jan 12, 2025

For some reason the m2m_fields is not specific to each instance of HistoricalRecords when using .register and I didn't go too much into figuring out why.

Mainly I changed how self.m2m_models is defined by using a tuple: [(instance._meta.object_name, field)] and then adding a custom function get_history_m2m_model_name which will get the name from inside the Through model.

I've managed to solve my problem with multi-table inheritance and polymorphic by changing the HistoricalRecords class as this:



class PolymorphicHistoricalRecords(HistoricalRecords):
    def create_historical_record_m2ms(self, history_instance, instance):
        for field in history_instance._history_m2m_fields:
            m2m_history_model = self.m2m_models[(instance._meta.object_name, field)]
            original_instance = history_instance.instance
            through_model = getattr(original_instance, field.name).through
            through_model_field_names = [f.name for f in through_model._meta.fields]
            through_model_fk_field_names = [
                f.name for f in through_model._meta.fields if isinstance(f, ForeignKey)
            ]

            insert_rows = []

            through_field_name = utils.get_m2m_field_name(field)
            rows = through_model.objects.filter(**{through_field_name: instance})
            rows = rows.select_related(*through_model_fk_field_names)
            for row in rows:
                insert_row = {"history": history_instance}

                for field_name in through_model_field_names:
                    insert_row[field_name] = getattr(row, field_name)
                insert_rows.append(m2m_history_model(**insert_row))

            pre_create_historical_m2m_records.send(
                sender=m2m_history_model,
                rows=insert_rows,
                history_instance=history_instance,
                instance=instance,
                field=field,
            )
            created_rows = m2m_history_model.objects.bulk_create(insert_rows)
            post_create_historical_m2m_records.send(
                sender=m2m_history_model,
                created_rows=created_rows,
                history_instance=history_instance,
                instance=instance,
                field=field,
            )

    def get_history_m2m_model_name(self, model, through_model):
        # Must be trying to use a custom history model name
        if callable(self.custom_model_name):
            name = self.custom_model_name(self, model._meta.object_name)
        elif callable(through_model.custom_historical_model_name):
            name = through_model.custom_historical_model_name(model, self.DEFAULT_MODEL_NAME_PREFIX)
        elif self.custom_model_name is not None:
            #  simple string
            name = self.custom_model_name
        else:
            return f"{self.DEFAULT_MODEL_NAME_PREFIX}{through_model._meta.object_name}"
        # Desired class name cannot be same as the model it is tracking
        if not (
                name.lower() == model._meta.object_name.lower()
                and model.__module__ == self.module
        ):
            return name
        raise ValueError(
            "The 'custom_model_name' option '{}' evaluates to a name that is the same "
            "as the model it is tracking. This is not permitted.".format(
                self.custom_model_name
            )
        )

    def get_extra_fields(self, model, fields):
        extra_fields = super().get_extra_fields(model, fields)

        def get_original_model_class_name():
            return model._meta.object_name

        extra_fields["get_original_model_class_name"] = get_original_model_class_name
        return extra_fields

    def create_history_m2m_model(self, model, through_model):
        attrs = {}

        fields = self.copy_fields(through_model)
        attrs.update(fields)
        attrs.update(self.get_extra_fields_m2m(model, through_model, fields))

        name = self.get_history_m2m_model_name(model, through_model)
        registered_models[through_model._meta.db_table] = through_model

        attrs.update(Meta=type("Meta", (), self.get_meta_options_m2m(through_model)))

        m2m_history_model = type(str(name), self.m2m_bases, attrs)

        return m2m_history_model

    def finalize(self, sender, **kwargs):
        inherited = False
        if self.cls is not sender:  # set in concrete
            inherited = self.inherit and issubclass(sender, self.cls)
            if not inherited:
                return  # set in abstract

        if hasattr(sender._meta, "simple_history_manager_attribute"):
            raise MultipleRegistrationsError(
                "{}.{} registered multiple times for history tracking.".format(
                    sender._meta.app_label, sender._meta.object_name
                )
            )
        history_model = self.create_history_model(sender, inherited)

        if inherited:
            # Make sure history model is in same module as concrete model
            module = importlib.import_module(history_model.__module__)
        else:
            module = importlib.import_module(self.module)
        setattr(module, history_model.__name__, history_model)

        # The HistoricalRecords object will be discarded,
        # so the signal handlers can't use weak references.
        models.signals.post_save.connect(self.post_save, sender=sender, weak=False)
        models.signals.post_delete.connect(self.post_delete, sender=sender, weak=False)

        m2m_fields = self.get_m2m_fields_from_model(sender)

        for field in m2m_fields:
            m2m_changed.connect(
                partial(self.m2m_changed, attr=field.name),
                sender=field.remote_field.through,
                weak=False,
            )

        descriptor = HistoryDescriptor(
            history_model,
            manager=self.history_manager,
            queryset=self.historical_queryset,
        )
        setattr(sender, self.manager_name, descriptor)
        sender._meta.simple_history_manager_attribute = self.manager_name

        for field in m2m_fields:
            m2m_model = self.create_history_m2m_model(
                history_model, field.remote_field.through
            )
            self.m2m_models[(sender._meta.object_name, field)] = m2m_model
            setattr(module, m2m_model.__name__, m2m_model)

            m2m_descriptor = HistoryDescriptor(m2m_model)
            setattr(history_model, field.name, m2m_descriptor)

and my Through model is defined like this:


class PropertyContact(models.Model):
    property= models.ForeignKey(Property, on_delete=models.CASCADE)
    contact = models.ForeignKey(Contact, on_delete=models.CASCADE)

    class Meta:
        unique_together = ('contact', 'property')

    @classmethod
    def custom_historical_model_name(cls, model, prefix=None):
        return f"{prefix}{model.get_original_model_class_name()}Contact"

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

1 participant