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

Set attribute on history list entries, from evaluated admin methods #390

Merged
merged 6 commits into from May 15, 2018

Conversation

blawson
Copy link
Contributor

@blawson blawson commented May 15, 2018

This change will allow admin pages that extend SimpleHistoryAdmin to provide admin methods that will be evaluated on each object in the change list.

I.e., you can update history_list_display with a method, defined in the admin, that will be evaluated and set on each object.

@codecov-io
Copy link

codecov-io commented May 15, 2018

Codecov Report

Merging #390 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #390      +/-   ##
=========================================
+ Coverage   97.47%   97.5%   +0.02%     
=========================================
  Files          15      15              
  Lines         595     600       +5     
  Branches       73      76       +3     
=========================================
+ Hits          580     585       +5     
  Misses          9       9              
  Partials        6       6
Impacted Files Coverage Δ
simple_history/admin.py 97.93% <100%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 149911e...bcd009a. Read the comment docs.

@blawson
Copy link
Contributor Author

blawson commented May 15, 2018

Adding tests.

# Set attribute on each action_list entry from admin methods
for x in history_list_display:
if getattr(self, x, None) and callable(getattr(self, x)):
history_method = getattr(self, x)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three getattr()s are a little wasteful. Can you pull it into a local instead?

for to_display in history_list_display:
  history_method = getattr(self, to_display, None)
  if history_method and callable(history_method):
    for list_entry in action_list:
      setattr(list_entry, to_display, history_method(list_entry))

Copy link
Collaborator

@kseever kseever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@rossmechanic
Copy link
Collaborator

Failing flake8 style @blawson

@@ -61,6 +61,13 @@ def history_view(self, request, object_id, extra_context=None):
if not self.has_change_permission(request, obj):
raise PermissionDenied

# Set attribute on each action_list entry from admin methods
for x in history_list_display:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change from x to a more descriptive var name

Copy link
Collaborator

@rossmechanic rossmechanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rossmechanic rossmechanic merged commit 4c60246 into jazzband:master May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants