From 935028a57e539629d89f03785695be9c90c3a6a0 Mon Sep 17 00:00:00 2001 From: Wesley van Lee Date: Thu, 14 Mar 2019 14:00:53 +0100 Subject: [PATCH 1/3] Use the label_for_field function to get the label for the fields in the InspectView --- wagtail/contrib/modeladmin/views.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/wagtail/contrib/modeladmin/views.py b/wagtail/contrib/modeladmin/views.py index 8ae5e1e311..465ac4ac58 100644 --- a/wagtail/contrib/modeladmin/views.py +++ b/wagtail/contrib/modeladmin/views.py @@ -7,7 +7,7 @@ from django.contrib.admin import FieldListFilter, widgets from django.contrib.admin.exceptions import DisallowedModelAdminLookup from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.utils import ( - get_fields_from_path, lookup_needs_distinct, prepare_lookup_value, quote, unquote) + get_fields_from_path, label_for_field, lookup_needs_distinct, prepare_lookup_value, quote, unquote) from django.contrib.auth.decorators import login_required from django.core.exceptions import ImproperlyConfigured, PermissionDenied, SuspiciousOperation from django.core.paginator import InvalidPage, Paginator @@ -842,16 +842,9 @@ class InspectView(InstanceSpecificView): def get_meta_title(self): return _('Inspecting %s') % self.verbose_name - def get_field_label(self, field_name, field=None): + def get_field_label(self, field_name): """ Return a label to display for a field """ - label = None - if field is not None: - label = getattr(field, 'verbose_name', None) - if label is None: - label = getattr(field, 'name', None) - if label is None: - label = field_name - return label + return label_for_field(field_name, model=self.model, model_admin=self.model_admin) def get_field_display_value(self, field_name, field=None): """ Return a display value for a field/attribute """ @@ -931,7 +924,7 @@ class InspectView(InstanceSpecificView): except FieldDoesNotExist: field = None return { - 'label': self.get_field_label(field_name, field), + 'label': self.get_field_label(field_name), 'value': self.get_field_display_value(field_name, field), } From e389a78e54e95bd352f1fce0f70d7478e3ebacef Mon Sep 17 00:00:00 2001 From: Wesley van Lee Date: Thu, 14 Mar 2019 17:55:14 +0100 Subject: [PATCH 2/3] Added a test for checking the short description and processed reviewer's feedback --- wagtail/contrib/modeladmin/tests/test_page_modeladmin.py | 6 +++++- wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py | 6 +++--- wagtail/contrib/modeladmin/views.py | 6 +++--- wagtail/tests/modeladmintest/models.py | 5 +++++ wagtail/tests/modeladmintest/wagtail_hooks.py | 2 +- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py index 7bb67346a6..eeac0f2f9e 100644 --- a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py @@ -103,7 +103,7 @@ class TestCreateView(TestCase, WagtailTestUtils): class TestInspectView(TestCase, WagtailTestUtils): - fixtures = ['test_specific.json'] + fixtures = ['test_specific.json', 'modeladmintest_test.json'] def setUp(self): self.login() @@ -157,6 +157,10 @@ class TestInspectView(TestCase, WagtailTestUtils): response = self.get(100) self.assertEqual(response.status_code, 404) + def test_label_display(self): + response = self.client.get('/admin/modeladmintest/author/inspect/1/') + self.assertContains(response, 'Birth information') + class TestEditView(TestCase, WagtailTestUtils): fixtures = ['test_specific.json'] diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index c23e471609..4845ea6674 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -222,11 +222,11 @@ class TestInspectView(TestCase, WagtailTestUtils): def test_author_name_present(self): """ - The author name should appear twice. Once in the header, and once - more in the field listing + The author name should appear three times. Once in the header, once + in the name field and once more in the birth string """ response = self.get_for_author(1) - self.assertContains(response, 'J. R. R. Tolkien', 2) + self.assertContains(response, 'J. R. R. Tolkien', 3) def test_author_dob_not_present(self): """ diff --git a/wagtail/contrib/modeladmin/views.py b/wagtail/contrib/modeladmin/views.py index 465ac4ac58..f61beb0cff 100644 --- a/wagtail/contrib/modeladmin/views.py +++ b/wagtail/contrib/modeladmin/views.py @@ -842,9 +842,9 @@ class InspectView(InstanceSpecificView): def get_meta_title(self): return _('Inspecting %s') % self.verbose_name - def get_field_label(self, field_name): + def get_field_label(self, field_name, field=None): """ Return a label to display for a field """ - return label_for_field(field_name, model=self.model, model_admin=self.model_admin) + return label_for_field(field_name, model=self.model) def get_field_display_value(self, field_name, field=None): """ Return a display value for a field/attribute """ @@ -924,7 +924,7 @@ class InspectView(InstanceSpecificView): except FieldDoesNotExist: field = None return { - 'label': self.get_field_label(field_name), + 'label': self.get_field_label(field_name, field), 'value': self.get_field_display_value(field_name, field), } diff --git a/wagtail/tests/modeladmintest/models.py b/wagtail/tests/modeladmintest/models.py index 0f7e2a1650..11e9ccd161 100644 --- a/wagtail/tests/modeladmintest/models.py +++ b/wagtail/tests/modeladmintest/models.py @@ -9,6 +9,11 @@ class Author(models.Model): name = models.CharField(max_length=255) date_of_birth = models.DateField() + def author_birth_string(self): + return '{} was born in pallet town'.format(self.name) + + author_birth_string.short_description = "Birth information" + def __str__(self): return self.name diff --git a/wagtail/tests/modeladmintest/wagtail_hooks.py b/wagtail/tests/modeladmintest/wagtail_hooks.py index 0e16c88372..acae4fbf8e 100644 --- a/wagtail/tests/modeladmintest/wagtail_hooks.py +++ b/wagtail/tests/modeladmintest/wagtail_hooks.py @@ -15,7 +15,7 @@ class AuthorModelAdmin(ModelAdmin): list_filter = ('date_of_birth', ) search_fields = ('name', ) inspect_view_enabled = True - inspect_view_fields = ('name', ) + inspect_view_fields = ('name', 'author_birth_string') def last_book(self, obj): # For testing use of modeladmin methods in list_display From 50b187b7575e7f6a05bec87a84a45b20eb012450 Mon Sep 17 00:00:00 2001 From: Wesley van Lee Date: Thu, 14 Mar 2019 18:32:29 +0100 Subject: [PATCH 3/3] Made the test more descriptive and changed the function so we do not have to change other tests --- wagtail/contrib/modeladmin/tests/test_page_modeladmin.py | 8 +++++++- .../contrib/modeladmin/tests/test_simple_modeladmin.py | 6 +++--- wagtail/tests/modeladmintest/models.py | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py index eeac0f2f9e..219802d8bb 100644 --- a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py @@ -157,9 +157,15 @@ class TestInspectView(TestCase, WagtailTestUtils): response = self.get(100) self.assertEqual(response.status_code, 404) - def test_label_display(self): + def test_short_description_is_used_as_field_label(self): + """ + A custom field has been added to the inspect view's `inspect_view_fields` and since + this field has a `short_description` we expect it to be used as the field's label, + and not use the name of the function. + """ response = self.client.get('/admin/modeladmintest/author/inspect/1/') self.assertContains(response, 'Birth information') + self.assertNotContains(response, 'author_birth_string') class TestEditView(TestCase, WagtailTestUtils): diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index 4845ea6674..c23e471609 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -222,11 +222,11 @@ class TestInspectView(TestCase, WagtailTestUtils): def test_author_name_present(self): """ - The author name should appear three times. Once in the header, once - in the name field and once more in the birth string + The author name should appear twice. Once in the header, and once + more in the field listing """ response = self.get_for_author(1) - self.assertContains(response, 'J. R. R. Tolkien', 3) + self.assertContains(response, 'J. R. R. Tolkien', 2) def test_author_dob_not_present(self): """ diff --git a/wagtail/tests/modeladmintest/models.py b/wagtail/tests/modeladmintest/models.py index 11e9ccd161..fbd7b3ff73 100644 --- a/wagtail/tests/modeladmintest/models.py +++ b/wagtail/tests/modeladmintest/models.py @@ -10,7 +10,7 @@ class Author(models.Model): date_of_birth = models.DateField() def author_birth_string(self): - return '{} was born in pallet town'.format(self.name) + return 'This author was born in pallet town' author_birth_string.short_description = "Birth information"