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

Display a running diff in the object history list page #1309

Conversation

davidtoulmin
Copy link

@davidtoulmin davidtoulmin commented Feb 16, 2024

Description

Add a column to the _object_history_list.html template that displays a table of the specific field changes made in each history object in the list (i.e for each item in the _object_history_list display the item.diff_against(previous_item)

Related Issue

#1308

Motivation and Context

This is a new feature, that I think makes the product much more helpful in tracking the changes in an object. It allows the user to see what changes were made in each history object, and allows browsing of changes, and makes it much easier to track down a specific change in the history. This is a feature the admin team for our website asked me to add, and it seems like a nice piece of functionality to add back to the codebase. I'm very happy to take suggestions on ways I can approve this, and will go through the testing and linting etc if people like the suggestion

How Has This Been Tested?

I have only tested manually for now, I wanted to get feedback on the idea before I went too far with the changes or testing

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run the pre-commit run command to format and lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have added my name and/or github handle to AUTHORS.rst
  • I have added my change to CHANGES.rst
  • All new and existing tests passed.

@ddabble
Copy link
Member

ddabble commented Feb 18, 2024

Thank you for your contribution, this is definitely wanted and useful! 😄 However, #1128 already implements a similar solution, but with simple, unordered lists instead of tables. (I personally think that nested tables have a tendency to make the page look more cluttered and disorganized - even if it would technically have been more structured.) So if that PR gets merged, I'll close this. Feel free to chime in on the other PR in case you think something's missing 🙂

@davidtoulmin
Copy link
Author

Hey ddabble,
Thanks, that solution looks great. Remarkably similar to my solution in fact, haha! Including a number of other changes I was going to make too.
I looked through the issues to find someone else talking about a similar solution and couldn't find anything, I should have looked through the merge requests as well. I'll close this MR, and just contribute to the other one.
Thanks,
David

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

2 participants