Merge remote-tracking branch 'upstream/master' into fix-mongodbengine-check

Author: singlerider

AUTHORS.rst

@@ -31,6 +31,7 @@ Authors
 - Rajesh Pappula
 - Rod Xavier Bondoc
 - Ross Lote
+- Ross Mechanic
 - Steven Klass
 - Steeve Chailloux
 - Trey Hunner
@@ -40,6 +41,7 @@ Authors
 - Michael England
 - Gregory Bataille
 - Jesse Shapiro
+- Kevin Foster
 - Shane Engelman
 
 Background

CHANGES.rst

@@ -5,6 +5,7 @@ Unreleased
 ----------
 - Use get_queryset rather than model.objects in history_view. (gh-303)
 - Change ugettext calls in models.py to ugettext_lazy
+- Resolve issue where model references itself (gh-278)
 
 1.9.0 (2017-06-11)
 ------------------

simple_history/admin.py

@@ -162,7 +162,7 @@ def history_form_view(self, request, object_id, version_id):
             'is_popup': False,
             'media': mark_safe(self.media + admin_form.media),
             'errors': helpers.AdminErrorList(form, formsets),
-            'app_label': model._meta.app_label,
+            'app_label': original_opts.app_label,
             'original_opts': original_opts,
             'changelist_url': reverse('%s:%s_%s_changelist' % url_triplet),
             'change_url': reverse('%s:%s_%s_change' % url_triplet,

simple_history/models.py

@@ -172,8 +172,17 @@ def copy_fields(self, model):
                     field_arguments['to_field'] = old_field.to_fields[0]
                 if getattr(old_field, 'db_column', None):
                     field_arguments['db_column'] = old_field.db_column
+
+                # If old_field.rel.to is 'self' then we have a case where object has a foreign key
+                # to itself. In this case we update need to set the `to` value of the field
+                # to be set to a model. We can use the old_field.model value.
+                if isinstance(old_field.rel.to, str) and old_field.rel.to == 'self':
+                    object_to = old_field.model
+                else:
+                    object_to = old_field.rel.to
+
                 field = FieldType(
-                    old_field.rel.to,
+                    object_to,
                     related_name='+',
                     null=True,
                     blank=True,

simple_history/tests/admin.py

@@ -3,7 +3,7 @@
 from django.contrib import admin
 
 from simple_history.admin import SimpleHistoryAdmin
-from .models import Poll, Choice, Person, Book, Document, Paper, Employee
+from .models import Poll, Choice, Person, Book, Document, Paper, Employee, ConcreteExternal
 
 
 class PersonAdmin(SimpleHistoryAdmin):
@@ -22,3 +22,4 @@ class ChoiceAdmin(SimpleHistoryAdmin):
 admin.site.register(Document, SimpleHistoryAdmin)
 admin.site.register(Paper, SimpleHistoryAdmin)
 admin.site.register(Employee, SimpleHistoryAdmin)
+admin.site.register(ConcreteExternal, SimpleHistoryAdmin)

simple_history/tests/external/models/model1.py

@@ -0,0 +1,12 @@
+from __future__ import unicode_literals
+
+from django.db import models
+from simple_history.models import HistoricalRecords
+
+
+class AbstractExternal(models.Model):
+    history = HistoricalRecords(inherit=True)
+
+    class Meta:
+        abstract = True
+        app_label = 'external'

simple_history/tests/models.py

@@ -1,12 +1,31 @@
 from __future__ import unicode_literals
 
 from django.db import models
+from django import VERSION
 
 from simple_history.models import HistoricalRecords
 from simple_history import register
 
 from .custom_user.models import CustomUser as User
 
+try:
+    from django.apps import apps
+except ImportError:  # Django < 1.7
+    from django.db.models import get_model
+else:
+    get_model = apps.get_model
+
+# 1.6 has different way of importing models
+if VERSION[:3] >= (1, 7, 0):
+    from .external.models.model1 import AbstractExternal
+else:
+    class AbstractExternal(models.Model):
+        history = HistoricalRecords(inherit=True)
+
+        class Meta:
+            abstract = True
+            app_label = 'external'
+
 
 class Poll(models.Model):
     question = models.CharField(max_length=200)
@@ -341,6 +360,13 @@ class UntrackedConcreteBase(models.Model):
     pass
 
 
+class ConcreteExternal(AbstractExternal):
+    name = models.CharField(max_length=50)
+
+    class Meta:
+        app_label = 'tests'
+
+
 class TrackedWithAbstractBase(TrackedAbstractBaseA):
     pass
 

simple_history/tests/tests/test_admin.py

@@ -13,7 +13,7 @@
 
 from simple_history.models import HistoricalRecords
 from simple_history.admin import SimpleHistoryAdmin, get_complete_version
-from ..models import Book, Person, Poll, State, Employee, Choice
+from ..models import Book, Person, Poll, State, Employee, Choice, ConcreteExternal
 
 try:
     from django.contrib.admin.utils import quote
@@ -551,3 +551,58 @@ def test_history_form_view_getting_history_with_setting_off(self):
         }
         mock_render.assert_called_once_with(
             request, admin.object_history_form_template, context, **extra_kwargs)
+
+    def test_history_form_view_getting_history_abstract_external(self):
+        request = RequestFactory().post('/')
+        request.session = 'session'
+        request._messages = FallbackStorage(request)
+        request.user = self.user
+        request.POST = {'_change_history': True}
+
+        obj = ConcreteExternal.objects.create(name='test')
+        obj.name = "new_test"
+        obj.save()
+        history = obj.history.all()[0]
+
+        admin_site = AdminSite()
+        admin = SimpleHistoryAdmin(ConcreteExternal, admin_site)
+
+        with patch('simple_history.admin.render') as mock_render:
+            with patch('simple_history.admin.SIMPLE_HISTORY_EDIT', True):
+                admin.history_form_view(request, obj.id, history.pk)
+
+        context = {
+            # Verify this is set for history object
+            'original': history.instance,
+            'change_history': True,
+
+            'title': 'Revert %s' % force_text(history.instance),
+            'adminform': ANY,
+            'object_id': obj.id,
+            'is_popup': False,
+            'media': ANY,
+            'errors': ANY,
+            'app_label': 'tests',
+            'original_opts': ANY,
+            'changelist_url': '/admin/tests/concreteexternal/',
+            'change_url': ANY,
+            'history_url': '/admin/tests/concreteexternal/{pk}/history/'.format(
+                pk=obj.pk),
+            'add': False,
+            'change': True,
+            'has_add_permission': admin.has_add_permission(request),
+            'has_change_permission': admin.has_change_permission(
+                request, obj),
+            'has_delete_permission': admin.has_delete_permission(
+                request, obj),
+            'has_file_field': True,
+            'has_absolute_url': False,
+            'form_url': '',
+            'opts': ANY,
+            'content_type_id': ANY,
+            'save_as': admin.save_as,
+            'save_on_top': admin.save_on_top,
+            'root_path': getattr(admin_site, 'root_path', None),
+        }
+        mock_render.assert_called_once_with(
+            request, admin.object_history_form_template, context, **extra_kwargs)