Change "object_id" to be a "CharField" (#138)

* [-] CrudEvent object_id as a `CharField`

* [-] Use `pk` instead of `id` in pre_save object lookup

* [-] Fixes issue w/ post_delete / Adds delete test

* [-] Adds updates from #141 PR

* [-] Removes the need for an explicit query

- Uses the internal `_state` object

* [-] Adds `_state` compat test

* [-] Removes sqlite3 test db

Author: jsoa

easyaudit/migrations/0014_auto_20200513_0008.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.0.6 on 2020-05-13 00:08
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('easyaudit', '0013_auto_20190723_0126'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='crudevent',
+            name='object_id',
+            field=models.CharField(max_length=255),
+        ),
+    ]

easyaudit/models.py

@@ -19,7 +19,7 @@ class CRUDEvent(models.Model):
     )
 
     event_type = models.SmallIntegerField(choices=TYPES)
-    object_id = models.IntegerField()  # we should try to allow other ID types
+    object_id = models.CharField(max_length=255)
     content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE, db_constraint=False)
     object_repr = models.TextField(null=True, blank=True)
     object_json_repr = models.TextField(null=True, blank=True)

easyaudit/signals/model_signals.py

@@ -6,6 +6,7 @@
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.contenttypes.models import ContentType
 from django.core import serializers
+from django.core.exceptions import ObjectDoesNotExist
 from django.db import transaction
 from django.db.models import signals
 from django.utils import timezone
@@ -62,10 +63,8 @@ def pre_save(sender, instance, raw, using, update_fields, **kwargs):
                 # We need a better way for this to work. ManyToMany will fail on pre_save on create
                 return None
 
-            if instance.pk is None:
-                created = True
-            else:
-                created = False
+            # Determine if the instance is a create
+            created = instance.pk is None or instance._state.adding
 
             # created or updated?
             if not created:
@@ -300,6 +299,9 @@ def post_delete(sender, instance, using, **kwargs):
                 user = None
             c_t = ContentType.objects.get_for_model(instance)
 
+            # object id to be used later
+            obj_id = instance.pk
+
             def crud_flow():
                 try:
                     with transaction.atomic(using=DATABASE_ALIAS):
@@ -309,7 +311,7 @@ def crud_flow():
                             'object_repr': str(instance),
                             'object_json_repr': object_json_repr,
                             'content_type_id': c_t.id,
-                            'object_id': instance.pk,
+                            'object_id': obj_id,
                             'user_id': getattr(user, 'id', None),
                             'datetime': timezone.now(),
                             'user_pk_as_string': str(user.pk) if user else user

easyaudit/tests/db.sqlite3

@@ -0,0 +1,61 @@
+# Generated by Django 3.0.6 on 2020-05-14 17:28
+
+from django.db import migrations, models
+import django.db.models.deletion
+import uuid
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('test_app', '0002_auto_20180220_1533'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='TestBigIntModel',
+            fields=[
+                ('id', models.BigAutoField(primary_key=True, serialize=False)),
+                ('name', models.CharField(default='test data', max_length=50)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='TestUUIDModel',
+            fields=[
+                ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True)),
+                ('name', models.CharField(default='test data', max_length=50)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='TestUUIDM2M',
+            fields=[
+                ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True)),
+                ('name', models.CharField(max_length=50)),
+                ('test_m2m', models.ManyToManyField(to='test_app.TestUUIDModel')),
+            ],
+        ),
+        migrations.CreateModel(
+            name='TestUUIDForeignKey',
+            fields=[
+                ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True)),
+                ('name', models.CharField(max_length=50)),
+                ('test_fk', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='test_app.TestUUIDModel')),
+            ],
+        ),
+        migrations.CreateModel(
+            name='TestBigIntM2M',
+            fields=[
+                ('id', models.BigAutoField(primary_key=True, serialize=False)),
+                ('name', models.CharField(max_length=50)),
+                ('test_m2m', models.ManyToManyField(to='test_app.TestBigIntModel')),
+            ],
+        ),
+        migrations.CreateModel(
+            name='TestBigIntForeignKey',
+            fields=[
+                ('id', models.BigAutoField(primary_key=True, serialize=False)),
+                ('name', models.CharField(max_length=50)),
+                ('test_fk', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='test_app.TestBigIntModel')),
+            ],
+        ),
+    ]

easyaudit/tests/test_app/migrations/0003_testbigintforeignkey_testbigintm2m_testbigintmodel_testuuidforeignkey_testuuidm2m_testuuidmodel.py

@@ -1,3 +1,5 @@
+import uuid
+
 from django.db import models
 
 
@@ -13,3 +15,43 @@ class TestForeignKey(models.Model):
 class TestM2M(models.Model):
     name = models.CharField(max_length=50)
     test_m2m = models.ManyToManyField(TestModel)
+
+
+class TestUUIDModel(models.Model):
+    id = models.UUIDField(
+        primary_key=True, unique=True, editable=False, default=uuid.uuid4
+    )
+    name = models.CharField(max_length=50, default='test data')
+
+
+class TestUUIDForeignKey(models.Model):
+    id = models.UUIDField(
+        primary_key=True, unique=True, editable=False, default=uuid.uuid4
+    )
+    name = models.CharField(max_length=50)
+    test_fk = models.ForeignKey(TestUUIDModel, on_delete=models.CASCADE)
+
+
+class TestUUIDM2M(models.Model):
+    id = models.UUIDField(
+        primary_key=True, unique=True, editable=False, default=uuid.uuid4
+    )
+    name = models.CharField(max_length=50)
+    test_m2m = models.ManyToManyField(TestUUIDModel)
+
+
+class TestBigIntModel(models.Model):
+    id = models.BigAutoField(primary_key=True)
+    name = models.CharField(max_length=50, default='test data')
+
+
+class TestBigIntForeignKey(models.Model):
+    id = models.BigAutoField(primary_key=True)
+    name = models.CharField(max_length=50)
+    test_fk = models.ForeignKey(TestBigIntModel, on_delete=models.CASCADE)
+
+
+class TestBigIntM2M(models.Model):
+    id = models.BigAutoField(primary_key=True)
+    name = models.CharField(max_length=50)
+    test_m2m = models.ManyToManyField(TestBigIntModel)

easyaudit/tests/test_app/models.py

@@ -11,7 +11,11 @@
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 import bs4
-from test_app.models import TestModel, TestForeignKey, TestM2M
+from test_app.models import (
+    TestModel, TestForeignKey, TestM2M,
+    TestBigIntModel, TestBigIntForeignKey, TestBigIntM2M,
+    TestUUIDModel, TestUUIDForeignKey, TestUUIDM2M
+)
 from easyaudit.models import CRUDEvent
 from easyaudit.middleware.easyaudit import set_current_user, clear_request
 
@@ -22,38 +26,49 @@
 TEST_ADMIN_PASSWORD = 'password'
 
 
+@override_settings(TEST=True)
+class TestDjangoCompat(TestCase):
+
+    def test_model_state(self):
+        """Ensures models have the internal `_state` object."""
+        inst = TestModel()
+        self.assertTrue(hasattr(inst, '_state'))
+
+
 @override_settings(TEST=True)
 class TestAuditModels(TestCase):
+    Model = TestModel
+    FKModel = TestForeignKey
+    M2MModel = TestM2M
 
     def test_create_model(self):
-        obj = TestModel.objects.create()
-        self.assertEqual(obj.id, 1)
+        obj = self.Model.objects.create()
         crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
         self.assertEqual(1, crud_event_qs.count())
         crud_event = crud_event_qs[0]
         data = json.loads(crud_event.object_json_repr)[0]
         self.assertEqual(data['fields']['name'], obj.name)
 
     def test_fk_model(self):
-        obj = TestModel.objects.create()
-        obj_fk = TestForeignKey(name='test', test_fk=obj)
+        obj = self.Model.objects.create()
+        obj_fk = self.FKModel(name='test', test_fk=obj)
         obj_fk.save()
         crud_event = CRUDEvent.objects.filter(object_id=obj_fk.id, content_type=ContentType.objects.get_for_model(obj_fk))[0]
         data = json.loads(crud_event.object_json_repr)[0]
-        self.assertEqual(data['fields']['test_fk'], obj.id)
+        self.assertEqual(str(data['fields']['test_fk']), str(obj.id))
 
     def test_m2m_model(self):
-        obj = TestModel.objects.create()
-        obj_m2m = TestM2M(name='test')
+        obj = self.Model.objects.create()
+        obj_m2m = self.M2MModel(name='test')
         obj_m2m.save()
         obj_m2m.test_m2m.add(obj)
         crud_event = CRUDEvent.objects.filter(object_id=obj_m2m.id, content_type=ContentType.objects.get_for_model(obj_m2m))[0]
         data = json.loads(crud_event.object_json_repr)[0]
-        self.assertEqual(data['fields']['test_m2m'], [obj.id])
+        self.assertEqual([str(d) for d in data['fields']['test_m2m']], [str(obj.id)])
 
     @override_settings(DJANGO_EASY_AUDIT_CRUD_EVENT_NO_CHANGED_FIELDS_SKIP=True)
     def test_update_skip_no_changed_fields(self):
-        obj = TestModel.objects.create()
+        obj = self.Model.objects.create()
         crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
         self.assertEqual(1, crud_event_qs.count())
         obj.name = 'changed name'
@@ -63,7 +78,7 @@ def test_update_skip_no_changed_fields(self):
         self.assertIn('name', last_change.changed_fields)
 
     def test_update(self):
-        obj = TestModel.objects.create()
+        obj = self.Model.objects.create()
         crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
         self.assertEqual(1, crud_event_qs.count())
         obj.name = 'changed name'
@@ -74,17 +89,39 @@ def test_update(self):
 
     @override_settings(DJANGO_EASY_AUDIT_CRUD_EVENT_NO_CHANGED_FIELDS_SKIP=True)
     def test_fake_update_skip_no_changed_fields(self):
-        obj = TestModel.objects.create()
+        obj = self.Model.objects.create()
         crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
         obj.save()
         self.assertEqual(1, crud_event_qs.count())
 
     def test_fake_update(self):
-        obj = TestModel.objects.create()
+        obj = self.Model.objects.create()
         crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
         obj.save()
         self.assertEqual(2, crud_event_qs.count())
 
+    def test_delete(self):
+        obj = self.Model.objects.create()
+        crud_event_qs = CRUDEvent.objects.filter(object_id=obj.id, content_type=ContentType.objects.get_for_model(obj))
+        self.assertEqual(1, crud_event_qs.count())
+
+        obj_id = obj.pk
+        obj.delete()
+        crud_event_qs = CRUDEvent.objects.filter(object_id=obj_id, content_type=ContentType.objects.get_for_model(obj))
+        self.assertEqual(2, crud_event_qs.count())
+
+
+class TestAuditUUIDModels(TestAuditModels):
+    Model = TestUUIDModel
+    FKModel = TestUUIDForeignKey
+    M2MModel = TestUUIDM2M
+
+
+class TestAuditBigIntModels(TestAuditModels):
+    Model = TestBigIntModel
+    FKModel = TestBigIntForeignKey
+    M2MModel = TestBigIntM2M
+
 
 @override_settings(TEST=True)
 class TestMiddleware(TestCase):

easyaudit/tests/test_app/tests.py

@@ -6,4 +6,10 @@
 urlpatterns = [
     url("create-obj", views.create_obj_view, name="create-obj"),
     url("update-obj", views.update_obj_view, name="update-obj"),
+
+    url("create-uuid-obj", views.create_uuid_obj_view, name="create-uuid-obj"),
+    url("update-uuid-obj", views.update_uuid_obj_view, name="update-uuid-obj"),
+
+    url("create-big-obj", views.create_big_obj_view, name="create-big-obj"),
+    url("update-big-obj", views.update_big_obj_view, name="update-big-obj"),
 ]

easyaudit/tests/test_app/urls.py

@@ -1,13 +1,48 @@
+from datetime import datetime
+
 from django.http import HttpResponse
-from test_app.models import TestModel
+from test_app.models import TestModel, TestUUIDModel, TestBigIntModel
+
+
+def create_obj(Model):
+    return Model.objects.create()
+
+
+def update_obj(Model, pk, name):
+    tm = Model.objects.get(pk=pk)
+    tm.name = name
+    tm.save()
+    return tm
 
 
 def create_obj_view(request):
-    return HttpResponse(TestModel.objects.create().id)
+    return HttpResponse(create_obj(TestModel).id)
 
 
 def update_obj_view(request):
-    tm = TestModel.objects.get(pk=request.GET['id'])
-    tm.name = request.GET['id']
-    tm.save()
-    return HttpResponse(tm.id)
+    name = datetime.now().isoformat()
+    return HttpResponse(update_obj(
+        TestModel, request.GET['id'], name
+    ).id)
+
+
+def create_uuid_obj_view(request):
+    return HttpResponse(create_obj(TestUUIDModel).id)
+
+
+def update_uuid_obj_view(request):
+    name = datetime.now().isoformat()
+    return HttpResponse(update_obj(
+        TestUUIDModel, request.GET['id'], name
+    ).id)
+
+
+def create_big_obj_view(request):
+    return HttpResponse(create_obj(TestBigIntModel).id)
+
+
+def update_big_obj_view(request):
+    name = datetime.now().isoformat()
+    return HttpResponse(update_obj(
+        TestBigIntModel, request.GET['id'], name
+    ).id)