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

fix changes_display_dict to display added/deleted objects for m2m updates #464

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

a-cesari
Copy link

In one of my projects I needed to use the changes_display_dict function in a Django template to display changes of a m2m field.
Apparently for such field the changes_display_dict function is not storing the added/deleted object while the changes_dict function does. The changes_display_dict function just store a list like this: ['type', 'operation', 'objects'] .
See screenshots attached.

changes_dict
changes_display_dict

I slightly modified the changes_display_dict function to also store the list of added/removed objects. The result is the following:

changes_display_dict_after

I hope you could consider merging this if appropriate. Happy to implement any changes/improvements you might suggest on my current proposal.

@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Merging #464 (6403f34) into master (1674aca) will decrease coverage by 0.57%.
The diff coverage is 16.66%.

❗ Current head 6403f34 differs from pull request most recent head 7bf1798. Consider uploading reports for the commit 7bf1798 to get more accurate results

@@            Coverage Diff             @@
##           master     #464      +/-   ##
==========================================
- Coverage   92.74%   92.17%   -0.58%     
==========================================
  Files          26       26              
  Lines         786      792       +6     
==========================================
+ Hits          729      730       +1     
- Misses         57       62       +5     
Impacted Files Coverage Δ
auditlog/models.py 85.89% <16.66%> (-1.77%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aleh-rymasheuski aleh-rymasheuski self-requested a review November 29, 2022 11:02
@aleh-rymasheuski
Copy link
Contributor

aleh-rymasheuski commented Nov 29, 2022

@a-cesari, thanks for your patch! I indeed missed changes_display_dict logic when implementing m2m tracking because it's a feature which I don't use myself.

Could you please cover the added logic with a test? Can you also add a changelog entry for this feature?

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.

2 participants