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

Enable/ disable revert using only a settings attribute #632

Merged
merged 12 commits into from
Apr 23, 2020
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ htmlcov/
MANIFEST
test_files/
venv/
.DS_Store
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Unreleased
- import model `ContentType` in `SimpleHistoryAdmin` using `django_apps.get_model`
to avoid possible `AppRegistryNotReady` exception (gh-630)
- Fix `utils.update_change_reason` when user specifies excluded_fields
- settings.SIMPLE_HISTORY_REVERT_DISABLED if True removes the Revert
button from the history form for all historical models (gh-632))

2.8.0 (2019-12-02)
------------------
Expand Down
13 changes: 13 additions & 0 deletions docs/admin.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,16 @@ admin class


.. image:: screens/5_history_list_display.png

Disabling the option to revert an object
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

By default, an object can be reverted to its previous version. To disable this option update your settings with the following:

.. code-block:: python

SIMPLE_HISTORY_REVERT_DISABLED=True

When ``SIMPLE_HISTORY_REVERT_DISABLED`` is set to ``True``, the revert button is removed from the form.

.. image:: screens/10_revert_disabled.png
Binary file added docs/screens/10_revert_disabled.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 5 additions & 3 deletions runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@ def __getitem__(self, item):


def main():

if not settings.configured:
settings.configure(**DEFAULT_SETTINGS)
django.setup()
failures = DiscoverRunner(failfast=False).run_tests(["simple_history.tests"])
failures |= DiscoverRunner(failfast=False).run_tests(
tags = [t.split("=")[1] for t in sys.argv if t.startswith("--tag")]
failures = DiscoverRunner(failfast=False, tags=tags).run_tests(
["simple_history.tests"]
)
failures |= DiscoverRunner(failfast=False, tags=tags).run_tests(
["simple_history.registry_tests"]
)
sys.exit(failures)
Expand Down
22 changes: 20 additions & 2 deletions simple_history/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def history_view(self, request, object_id, extra_context=None):
content_type.model,
)
context = {
"title": _("Change history: %s") % force_str(obj),
"title": self.history_view_title(obj),
"action_list": action_list,
"module_name": capfirst(force_str(opts.verbose_name_plural)),
"object": obj,
Expand All @@ -97,6 +97,7 @@ def history_view(self, request, object_id, extra_context=None):
"opts": opts,
"admin_user_view": admin_user_view,
"history_list_display": history_list_display,
"revert_disabled": self.revert_disabled,
}
context.update(self.admin_site.each_context(request))
context.update(extra_context or {})
Expand All @@ -105,6 +106,12 @@ def history_view(self, request, object_id, extra_context=None):
request, self.object_history_template, context, **extra_kwargs
)

def history_view_title(self, obj):
if self.revert_disabled and not SIMPLE_HISTORY_EDIT:
return _("View history: %s") % force_str(obj)
else:
return _("Change history: %s") % force_str(obj)

def response_change(self, request, obj):
if "_change_history" in request.POST and SIMPLE_HISTORY_EDIT:
verbose_name = obj._meta.verbose_name
Expand Down Expand Up @@ -175,7 +182,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
model_name = original_opts.model_name
url_triplet = self.admin_site.name, original_opts.app_label, model_name
context = {
"title": _("Revert %s") % force_str(obj),
"title": self.history_form_view_title(obj),
"adminform": admin_form,
"object_id": object_id,
"original": obj,
Expand All @@ -188,6 +195,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
"change_url": reverse("%s:%s_%s_change" % url_triplet, args=(obj.pk,)),
"history_url": reverse("%s:%s_%s_history" % url_triplet, args=(obj.pk,)),
"change_history": change_history,
"revert_disabled": self.revert_disabled,
# Context variables copied from render_change_form
"add": False,
"change": True,
Expand All @@ -212,6 +220,12 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
request, self.object_history_form_template, context, **extra_kwargs
)

def history_form_view_title(self, obj):
if self.revert_disabled:
return _("View %s") % force_str(obj)
else:
return _("Revert %s") % force_str(obj)

def render_history_view(self, request, template, context, **kwargs):
"""Catch call to render, to allow overriding."""
return render(request, template, context, **kwargs)
Expand All @@ -226,3 +240,7 @@ def content_type_model_cls(self):
"""Returns the ContentType model class.
"""
return django_apps.get_model("contenttypes.contenttype")

@property
def revert_disabled(self):
return getattr(settings, "SIMPLE_HISTORY_REVERT_DISABLED", False)
6 changes: 2 additions & 4 deletions simple_history/templates/simple_history/object_history.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@

{% block content %}
<div id="content-main">

<p>{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}</p>

{% if not revert_disabled %}<p>
{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}</p>{% endif %}
<div class="module">
{% if action_list %}
{% display_list %}
Expand All @@ -19,4 +18,3 @@
</div>
</div>
{% endblock %}

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<a href="{{changelist_url}}">{{opts.verbose_name_plural|capfirst}}</a> &rsaquo;
<a href="{{change_url}}">{{original|truncatewords:"18"}}</a> &rsaquo;
<a href="../">{% trans "History" %}</a> &rsaquo;
{% blocktrans with original_opts.verbose_name as verbose_name %}Revert {{verbose_name}}{% endblocktrans %}
{% if revert_disabled %}{% blocktrans with original_opts.verbose_name as verbose_name %}View {{verbose_name}}{% endblocktrans %}{% else %}{% blocktrans with original_opts.verbose_name as verbose_name %}Revert {{verbose_name}}{% endblocktrans %}{% endif %}
</div>
{% endblock %}

Expand All @@ -18,5 +18,5 @@
{% endblock %}

{% block form_top %}
<p>{% blocktrans %}Press the 'Revert' button below to revert to this version of the object.{% endblocktrans %} {% if change_history %}{% blocktrans %}Or press the 'Change History' button to edit the history.{% endblocktrans %}{% endif %}</p>
<p>{% if not revert_disabled %}{% blocktrans %}Press the 'Revert' button below to revert to this version of the object. {% endblocktrans %}{% endif %}{% if change_history %}{% blocktrans %}Press the 'Change History' button below to edit the history.{% endblocktrans %}{% endif %}</p>
{% endblock %}
7 changes: 5 additions & 2 deletions simple_history/templates/simple_history/submit_line.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{% load i18n %}
<div class="submit-row">
<input type="submit" value="{% trans 'Revert' %}" class="default" name="_save" {{ onclick_attrib }}/>
{% if change_history %}<input type="submit" value="{% trans 'Change History' %}" class="default" name="_change_history" {{ onclick_attrib }}/>{% endif %}
{% if not revert_disabled %}
<input type="submit" value="{% trans 'Revert' %}" class="default" name="_save" {{ onclick_attrib }}/>{% endif %}
{% if change_history %}
<input type="submit" value="{% trans 'Change History' %}" class="default" name="_change_history" {{ onclick_attrib }}/>{% endif %}
<a href="{{ history_url }}" class="closelink">{% trans 'Close' %}</a>
</div>
9 changes: 9 additions & 0 deletions simple_history/tests/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
FileModel,
Paper,
Person,
Planet,
Poll,
)

Expand All @@ -33,6 +34,13 @@ def test_method(self, obj):
history_list_display = ["title", "test_method"]


class PlanetAdmin(SimpleHistoryAdmin):
def test_method(self, obj):
return "test_method_value"

history_list_display = ["title", "test_method"]


admin.site.register(Poll, SimpleHistoryAdmin)
admin.site.register(Choice, ChoiceAdmin)
admin.site.register(Person, PersonAdmin)
Expand All @@ -43,3 +51,4 @@ def test_method(self, obj):
admin.site.register(ConcreteExternal, SimpleHistoryAdmin)
admin.site.register(ExternalModelWithCustomUserIdField, SimpleHistoryAdmin)
admin.site.register(FileModel, FileModelAdmin)
admin.site.register(Planet, PlanetAdmin)
13 changes: 11 additions & 2 deletions simple_history/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,17 @@ class City(models.Model):
history = HistoricalRecords()


class Planet(models.Model):
star = models.CharField(max_length=30)
history = HistoricalRecords()

def __str__(self):
return self.star

class Meta:
verbose_name = "Planet"


class Contact(models.Model):
name = models.CharField(max_length=30)
email = models.EmailField(max_length=255, unique=True)
Expand Down Expand Up @@ -576,7 +587,6 @@ class UUIDRegisterModel(models.Model):

register(UUIDRegisterModel, history_id_field=models.UUIDField(default=uuid.uuid4))


# Set the SIMPLE_HISTORY_HISTORY_ID_USE_UUID
setattr(settings, "SIMPLE_HISTORY_HISTORY_ID_USE_UUID", True)

Expand All @@ -589,7 +599,6 @@ class UUIDDefaultModel(models.Model):
# Clear the SIMPLE_HISTORY_HISTORY_ID_USE_UUID
delattr(settings, "SIMPLE_HISTORY_HISTORY_ID_USE_UUID")


# Set the SIMPLE_HISTORY_HISTORY_CHANGE_REASON_FIELD
setattr(settings, "SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD", True)

Expand Down
63 changes: 61 additions & 2 deletions simple_history/tests/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
Person,
Poll,
State,
Planet,
)

if django.VERSION < (2,):
Expand Down Expand Up @@ -65,8 +66,13 @@ def tearDown(self):
except AttributeError:
pass

def login(self, user=None):
self.client.force_login(user or self.user)
def login(self, user=None, superuser=None):
user = user or self.user
if superuser is not None:
user.is_superuser = True if superuser is None else superuser
user.is_active = True
user.save()
self.client.force_login(user)

def test_history_list(self):
model_name = self.user._meta.model_name
Expand Down Expand Up @@ -465,6 +471,7 @@ def test_history_form_view_without_getting_history(self):
"has_add_permission": admin.has_add_permission(request),
"has_change_permission": admin.has_change_permission(request, poll),
"has_delete_permission": admin.has_delete_permission(request, poll),
"revert_disabled": admin.revert_disabled,
"has_file_field": True,
"has_absolute_url": False,
"form_url": "",
Expand Down Expand Up @@ -518,6 +525,7 @@ def test_history_form_view_getting_history(self):
"has_add_permission": admin.has_add_permission(request),
"has_change_permission": admin.has_change_permission(request, poll),
"has_delete_permission": admin.has_delete_permission(request, poll),
"revert_disabled": admin.revert_disabled,
"has_file_field": True,
"has_absolute_url": False,
"form_url": "",
Expand Down Expand Up @@ -571,6 +579,7 @@ def test_history_form_view_getting_history_with_setting_off(self):
"has_add_permission": admin.has_add_permission(request),
"has_change_permission": admin.has_change_permission(request, poll),
"has_delete_permission": admin.has_delete_permission(request, poll),
"revert_disabled": admin.revert_disabled,
"has_file_field": True,
"has_absolute_url": False,
"form_url": "",
Expand Down Expand Up @@ -626,6 +635,7 @@ def test_history_form_view_getting_history_abstract_external(self):
"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),
"revert_disabled": admin.revert_disabled,
"has_file_field": True,
"has_absolute_url": False,
"form_url": "",
Expand Down Expand Up @@ -683,6 +693,7 @@ def test_history_form_view_accepts_additional_context(self):
"has_add_permission": admin.has_add_permission(request),
"has_change_permission": admin.has_change_permission(request, poll),
"has_delete_permission": admin.has_delete_permission(request, poll),
"revert_disabled": admin.revert_disabled,
"has_file_field": True,
"has_absolute_url": False,
"form_url": "",
Expand All @@ -696,3 +707,51 @@ def test_history_form_view_accepts_additional_context(self):
mock_render.assert_called_once_with(
request, admin.object_history_form_template, context
)

def test_history_view__title_suggests_revert_by_default(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet))
self.assertContains(response, "Change history: Sun")

@override_settings(SIMPLE_HISTORY_REVERT_DISABLED=False)
def test_history_view__title_suggests_revert(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet))
self.assertContains(response, "Change history: Sun")
self.assertContains(response, "Choose a date")

@override_settings(SIMPLE_HISTORY_REVERT_DISABLED=True)
def test_history_view__title_suggests_view_only(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet))
self.assertContains(response, "View history: Sun")
self.assertNotContains(response, "Choose a date")

def test_history_form_view__shows_revert_button_by_default(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet, 0))
self.assertContains(response, "Revert Planet")
self.assertContains(response, "Revert Sun")
self.assertContains(response, "Press the 'Revert' button")

@override_settings(SIMPLE_HISTORY_REVERT_DISABLED=False)
def test_history_form_view__shows_revert_button(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet, 0))
self.assertContains(response, "Revert Planet")
self.assertContains(response, "Revert Sun")
self.assertContains(response, "Press the 'Revert' button")

@override_settings(SIMPLE_HISTORY_REVERT_DISABLED=True)
def test_history_form_view__does_not_show_revert_button(self):
self.login()
planet = Planet.objects.create(star="Sun")
response = self.client.get(get_history_url(planet, 0))
self.assertContains(response, "View Planet")
self.assertContains(response, "View Sun")
self.assertNotContains(response, "Revert")
14 changes: 8 additions & 6 deletions simple_history/tests/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,12 @@ def test_foreignkey_still_allows_reverse_lookup_via_set_attribute(self):
)

def test_file_field(self):
model = FileModel.objects.create(file=get_fake_file("name"))
self.assertEqual(model.file.name, "files/name")
filename = str(uuid.uuid4())
model = FileModel.objects.create(file=get_fake_file(filename))
self.assertEqual(model.file.name, "files/{}".format(filename))
model.file.delete()
update_record, create_record = model.history.all()
self.assertEqual(create_record.file, "files/name")
self.assertEqual(create_record.file, "files/{}".format(filename))
self.assertEqual(update_record.file, "")

def test_file_field_with_char_field_setting(self):
Expand All @@ -323,11 +324,12 @@ def test_file_field_with_char_field_setting(self):
self.assertIs(type(file_field), models.CharField)
self.assertEqual(file_field.max_length, 100)
# file field works the same as test_file_field()
model = CharFieldFileModel.objects.create(file=get_fake_file("name"))
self.assertEqual(model.file.name, "files/name")
filename = str(uuid.uuid4())
model = CharFieldFileModel.objects.create(file=get_fake_file(filename))
self.assertEqual(model.file.name, "files/{}".format(filename))
model.file.delete()
update_record, create_record = model.history.all()
self.assertEqual(create_record.file, "files/name")
self.assertEqual(create_record.file, "files/{}".format(filename))
self.assertEqual(update_record.file, "")

def test_inheritance(self):
Expand Down