add changed_by field to WithHistory abstract model (#114)

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
 
 - A middleware `django_twc_toolbox.middleware.WwwRedirectMiddleware` for redirecting any request from a "www." subdomain to the bare domain. All credit to [Adam Johnson](https://github.com/adamchainz) -- [How to Make Django Redirect WWW to Your Bare Domain - Adam Johnson](https://adamj.eu/tech/2020/03/02/how-to-make-django-redirect-www-to-your-bare-domain/).
 - Now supporting Django 5.1.
+- Added a `changed_by` field to `WithHistory` abstract model with a relation to a application's `User` model to track the source of the change through time. This will require a run of `makemigrations` and `migrate` on any model inheriting from `WithHistory`.
 
 ### Changed
 

Justfile

@@ -30,8 +30,8 @@ testall *ARGS:
 coverage:
     python -m nox --session "coverage"
 
-types:
-    python -m nox --session "mypy"
+types *ARGS:
+    python -m nox --session "mypy" -- "{{ ARGS }}"
 
 # ----------------------------------------------------------------------
 # DJANGO

noxfile.py

@@ -115,4 +115,8 @@ def lint(session):
 @nox.session
 def mypy(session):
     session.install("django-twc-toolbox[dev] @ .")
-    session.run("python", "-m", "mypy", ".")
+
+    if session.posargs:
+        session.run("python", "-m", "mypy", ".", *session.posargs)
+    else:
+        session.run("python", "-m", "mypy", ".")

pyproject.toml

@@ -171,8 +171,8 @@ module = ["charidfield.*", "cuid.*", "simple_history.*"]
 ignore_missing_model_attributes = true
 
 [tool.pytest.ini_options]
+DJANGO_SETTINGS_MODULE = "tests.settings"
 addopts = "--create-db -n auto --dist loadfile --doctest-modules"
-django_find_project = false
 norecursedirs = ".* bin build dist *.egg htmlcov logs node_modules templates venv"
 python_files = "tests.py test_*.py *_tests.py"
 pythonpath = "src"

src/django_twc_toolbox/management/commands/copy_template.py

@@ -67,7 +67,7 @@ def handle(self, *args, **options):
         # 5/ else inspect TEMPLATES[DIRS] setting and use first option if available
         else:
             try:
-                template_dir = Path(settings.TEMPLATES[0]["DIRS"][0])
+                template_dir = Path(settings.TEMPLATES[0]["DIRS"][0])  # type: ignore[index]
                 destination_path = template_dir / source
             except IndexError:
                 # 6/ otherwise create project level template directory and dump file there.

src/django_twc_toolbox/models.py

@@ -66,13 +66,17 @@ def is_edited(self) -> bool:
 
 
 if find_spec("simple_history"):
+    from django.contrib.auth import get_user_model
+    from django.contrib.auth.models import AbstractBaseUser
     from simple_history.models import HistoricalRecords
 
     class WithHistory(models.Model):
         """
         Abstract model for adding historical records to a model.
         """
 
+        changed_by = models.ForeignKey(get_user_model(), on_delete=models.CASCADE)
+
         history = HistoricalRecords(inherit=True)
 
         class Meta:
@@ -90,3 +94,11 @@ def save_without_history(self, *args, **kwargs) -> None:
                 self.save(*args, **kwargs)
             finally:
                 del self.skip_history_when_saving
+
+        @property
+        def _history_user(self) -> AbstractBaseUser:
+            return self.changed_by
+
+        @_history_user.setter
+        def _history_user(self, user: AbstractBaseUser) -> None:
+            self.changed_by = user  # type: ignore[assignment]

tests/conftest.py

@@ -1,58 +1,16 @@
 from __future__ import annotations
 
 import logging
-from pathlib import Path
 
 import pytest
-from django.conf import settings
 from django.http import HttpRequest
 
-from .settings import DEFAULT_SETTINGS
-
 pytest_plugins = []
 
 
 def pytest_configure(config):
     logging.disable(logging.CRITICAL)
 
-    settings.configure(**DEFAULT_SETTINGS, **TEST_SETTINGS)
-
-
-TEST_SETTINGS = {
-    "INSTALLED_APPS": [
-        "django_twc_toolbox",
-        "django_twc_toolbox.crud",
-        "django.contrib.admin",
-        "django.contrib.auth",
-        "django.contrib.contenttypes",
-        "django_tables2",
-        "simple_history",
-        "template_partials",
-        "tests.dummy",
-        "tests.test_crud",
-    ],
-    "TEMPLATES": [
-        {
-            "BACKEND": "django.template.backends.django.DjangoTemplates",
-            "DIRS": [Path(__file__).parent / "templates"],
-            "OPTIONS": {
-                "context_processors": [
-                    "django.template.context_processors.request",
-                ],
-                "loaders": [
-                    (
-                        "template_partials.loader.Loader",
-                        [
-                            "django.template.loaders.filesystem.Loader",
-                            "django.template.loaders.app_directories.Loader",
-                        ],
-                    )
-                ],
-            },
-        }
-    ],
-}
-
 
 @pytest.fixture
 def req():

tests/settings.py

@@ -1,27 +1,68 @@
 from __future__ import annotations
 
+from pathlib import Path
+
 import django_stubs_ext
 
 django_stubs_ext.monkeypatch()
 
-DEFAULT_SETTINGS = {
-    "ALLOWED_HOSTS": ["*"],
-    "DEBUG": False,
-    "CACHES": {
-        "default": {
-            "BACKEND": "django.core.cache.backends.dummy.DummyCache",
-        }
-    },
-    "DATABASES": {
-        "default": {
-            "ENGINE": "django.db.backends.sqlite3",
-            "NAME": ":memory:",
-        }
-    },
-    "EMAIL_BACKEND": "django.core.mail.backends.locmem.EmailBackend",
-    "LOGGING_CONFIG": None,
-    "PASSWORD_HASHERS": [
-        "django.contrib.auth.hashers.MD5PasswordHasher",
-    ],
-    "SECRET_KEY": "not-a-secret",
+ALLOWED_HOSTS = ["*"]
+
+CACHES = {
+    "default": {
+        "BACKEND": "django.core.cache.backends.dummy.DummyCache",
+    }
+}
+
+DEBUG = False
+
+DATABASES = {
+    "default": {
+        "ENGINE": "django.db.backends.sqlite3",
+        "NAME": ":memory:",
+    }
 }
+
+EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend"
+
+INSTALLED_APPS = [
+    "django_twc_toolbox",
+    "django_twc_toolbox.crud",
+    "django.contrib.admin",
+    "django.contrib.auth",
+    "django.contrib.contenttypes",
+    "django_tables2",
+    "simple_history",
+    "template_partials",
+    "tests.dummy",
+    "tests.test_crud",
+]
+
+LOGGING_CONFIG = None
+
+PASSWORD_HASHERS = [
+    "django.contrib.auth.hashers.MD5PasswordHasher",
+]
+
+SECRET_KEY = "not-a-secret"
+
+TEMPLATES = [
+    {
+        "BACKEND": "django.template.backends.django.DjangoTemplates",
+        "DIRS": [Path(__file__).parent / "templates"],
+        "OPTIONS": {
+            "context_processors": [
+                "django.template.context_processors.request",
+            ],
+            "loaders": [
+                (
+                    "template_partials.loader.Loader",
+                    [
+                        "django.template.loaders.filesystem.Loader",
+                        "django.template.loaders.app_directories.Loader",
+                    ],
+                )
+            ],
+        },
+    }
+]

tests/test_models.py

@@ -5,6 +5,7 @@
 
 import pytest
 from django.conf import settings
+from django.contrib.auth import get_user_model
 from django.test import override_settings
 from model_bakery import baker
 
@@ -44,6 +45,75 @@ def test_save_without_history_method():
     assert dummy.history.count() == 1
 
 
+def test_changed_by_user():
+    user = baker.make(get_user_model())
+
+    dummy = baker.make("dummy.ModelWithHistory", changed_by=user)
+
+    assert dummy.changed_by == user
+    assert dummy.history.count() == 1
+    assert dummy.history.first().history_user == user
+
+
+def test_change_user():
+    user = baker.make(get_user_model())
+    another_user = baker.make(get_user_model())
+
+    dummy = baker.make("dummy.ModelWithHistory", changed_by=user)
+
+    assert dummy.changed_by == user
+    assert dummy.history.count() == 1
+
+    dummy.changed_by = another_user
+    dummy.save()
+
+    assert dummy.changed_by == another_user
+    assert dummy.history.count() == 2
+    assert dummy.history.first().history_user == another_user
+
+
+def test_history_user_property():
+    user = baker.make(get_user_model())
+
+    dummy = baker.make("dummy.ModelWithHistory", changed_by=user)
+
+    assert dummy._history_user == user
+
+
+def test_history_user_setter():
+    user = baker.make(get_user_model())
+    another_user = baker.make(get_user_model())
+
+    dummy = baker.make("dummy.ModelWithHistory", changed_by=user)
+
+    dummy._history_user = another_user
+    dummy.save()
+
+    assert dummy.changed_by == another_user
+    assert dummy.history.count() == 2
+    assert dummy.history.first().history_user == another_user
+
+
+def test_multiple_changes_with_different_users():
+    user = baker.make(get_user_model())
+    another_user = baker.make(get_user_model())
+
+    dummy = baker.make("dummy.ModelWithHistory", changed_by=user)
+
+    dummy.name = "Updated by first user"
+    dummy.save()
+
+    dummy._history_user = another_user
+    dummy.name = "Updated by second user"
+    dummy.save()
+
+    assert dummy.history.count() == 3
+    historical_records = list(dummy.history.all())
+    assert historical_records[0].history_user == another_user
+    assert historical_records[1].history_user == user
+    assert historical_records[2].history_user == user
+
+
 @override_settings(
     INSTALLED_APPS=[app for app in settings.INSTALLED_APPS if app != "simple_history"]
 )