django-save-the-change icon indicating copy to clipboard operation
django-save-the-change copied to clipboard

Support for abstract model classes?

Open gregorypease280 opened this issue 7 years ago • 3 comments

Can I attach the STC Decorator to an abstract base class? I have over 20 or so models that all inherit from a base class and I would like to implement STC for all of them.

Currently receiving def save(self, *args, **kwargs): for save_hook in self._meta._stc_save_hooks: E AttributeError: 'Options' object has no attribute '_stc_save_hooks'

gregorypease280 avatar Apr 13 '17 18:04 gregorypease280

This looks like a result of how the Options (Model.meta) object is created for models. Some bookkeeping for STC is put in that object since it’s the sanest place for it to be, but that object isn’t inherited.

There’s definitely some way to make this work, but I’m not sure how best to do it without looking into it some more. Ideally we could have the needed attributes copied during class creation so you’ll only pay the cost on startup.

karanlyons avatar Apr 13 '17 21:04 karanlyons

If you have many subclass models, a workaround is to apply the decorator in the metaclass. This works for me with Python 3.6 and Django 1.8.8:

from django.db import models
from save_the_change.decorators import SaveTheChange
from django.db.models.base import ModelBase

class SaveTheChangeModelBase(ModelBase):
    def __new__(cls, name, bases, attrs):
        meta = attrs.get("Meta")
        is_abstract = getattr(meta, "abstract", False)

        new_class = super().__new__(cls, name, bases, attrs)

        if not is_abstract:
            new_class = SaveTheChange(new_class)

        return new_class


class BaseKnight(models.Model, metaclass=SaveTheChangeModelBase):
    class Meta:
        abstract = True


class Knight(BaseKnight):
    name = models.CharField(max_length=100)
    age = models.PositiveIntegerField()

Verified with test case:

from django.db import models
from django.test import TestCase
from .models import Knight
from unittest.mock import patch
# Create your tests here.

class TestSTC(TestCase):

    def test_stc(self):
        kn = Knight.objects.create(name='Lancelot', age=20)

        with patch.object(models.Model, 'save') as patched_save:
            kn.save()
            patched_save.assert_not_called()

            kn.age = 21
            kn.save()
            patched_save.assert_called_once_with(update_fields=['age'])

oTree-org avatar Aug 07 '17 09:08 oTree-org

What @oTree-org recommends is definitely what I would say to do as well given STC as it currently stands. STC used to use mixins as opposed to generators, but that required metaclass inheritance (as in the examples above) which becomes a problem for interoperability with other packages that want to do the same. The move to decorators seemed the best way out of these issues (I’m not a diehard fan of my current solution here, but it was the least bad one I could come up with. This history is also why the decorators are @SaveTheChange instead of @save_the_change).

Of course the decorators are just being passed the actual constructed class object, which is why they don’t work properly through descendant classes.

Honestly I’d probably just recommend that developers define their own custom metaclass when they want to use STC with model inheritance. It’s not pretty, but I can’t come up with another solution that has a clean syntax, and having STC do metaclass stuff by default again may introduce more problems than it solves.

karanlyons avatar Aug 07 '17 22:08 karanlyons