Honor a custom admin site_header in the listing and details pages. #448
Conversation
Codecov Report
@@ Coverage Diff @@
## master #448 +/- ##
==========================================
+ Coverage 97.35% 97.36% +<.01%
==========================================
Files 15 15
Lines 680 682 +2
Branches 93 93
==========================================
+ Hits 662 664 +2
Misses 9 9
Partials 9 9
Continue to review full report at Codecov.
|
|
Thanks for this @tbeadle. Did you see my comment in the original issue about using |
tbeadle
force-pushed
the
admin-site-header
branch
from
93122e7
to
3e4b274
Compare
rossmechanic
added
the
DjangoConUS2018
| @@ -84,6 +84,7 @@ def history_view(self, request, object_id, extra_context=None): | |||
| 'admin_user_view': admin_user_view, | |||
| 'history_list_display': history_list_display, | |||
| } | |||
| context.update(self.admin_site.each_context(request)) | |||
There was a problem hiding this comment.
Looks good, but why not include in the initial context definition? Like https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1608
There was a problem hiding this comment.
That syntax is not supported for python2.7.
There was a problem hiding this comment.
Looks good to me, @rossmechanic do you want to approve as well?
|
Oops sorry I don't like merge commits but I hit the "Update branch" button in the Github UI to resolve the conflict and it did that |
|
When we merge we can just use Github's "Squash and merge" to preserve a clean commit history. |
|
Can you add a note to CHANGES.rst as well, following this convention (and with the PR number in parens |
tbeadle
force-pushed
the
admin-site-header
branch
from
5bfcbf6
to
17f3e14
Compare
CHANGES.rst
Outdated
| - Add `'+'` as the `history_type` for each instance in `bulk_history_create` (gh-449) | ||
| - Add support for `history_change_reason` for each instance in `bulk_history_create` (gh-449) | ||
| - Fix header on history pages when custom site_header is used (gh-448) |
|
Just need to fix conflict and I'll merge @tbeadle |
|
Hey @tbeadle. We had some weirdness with codecov and travis, but everything seems to be working now. If you want to resolve the conflicts, I'll merge |
tbeadle
force-pushed
the
admin-site-header
branch
2 times, most recently
from
0ef7a2e
to
78541b0
Compare
|
@tbeadle failing some flake8 style tests. Want to fix those? And then I'll merge |
|
The flake8 failures seem to be due to the new version of flake8 that was released 11 hours ago. I'm guessing it may have something to do with https://gitlab.com/pycqa/flake8/merge_requests/230. The new version doesn't allow for line breaks around a binary operator, which makes formatting those 2 lines pretty ugly or require putting in an |
tbeadle
force-pushed
the
admin-site-header
branch
from
14626e0
to
cd76a54
Compare
|
Checking now |
| @@ -51,6 +51,7 @@ Authors | |||
| - Ross Rogers | |||
| - Steven Klass | |||
| - Steeve Chailloux | |||
| - Tommy Beadle (@tbeadle) | |||
Description
Include the admin site's site_header when generating the context for the pages used in django-simple-history.
Related Issue
Fixes #205
Motivation and Context
Previously, the default "Django Administration" header was being displayed.
How Has This Been Tested?
Used a sample project and set
admin.site.site_header = 'Something'in urls.py. Verified that the history listing and details pages show that header. Previously, they showed the default.Types of changes
Checklist: