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

Add show deleted objects filter #1169

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mjsir911
Copy link
Contributor

Description

Code taken from #72, combined into a commit & tidied up a bit.

Related Issue

Fixes #72

Motivation and Context

Allows to see deleted objects from admin page.

How Has This Been Tested?

Manually (for now).

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.

Of note is that this is a joint PR, most of the code has been contributed by @marco-silva0000, @rhaver, & @duebbert.
I don't know if we'll need their explicit sign-off for this, but I figure the attribution would be good.

@mjsir911 mjsir911 force-pushed the msirabella/admin_panel branch from c218865 to 9b9bd0a Compare April 27, 2023 17:12
@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Merging #1169 (e0015cc) into master (a768673) will decrease coverage by 1.04%.
The diff coverage is 48.14%.

@@            Coverage Diff             @@
##           master    #1169      +/-   ##
==========================================
- Coverage   97.30%   96.26%   -1.04%     
==========================================
  Files          23       23              
  Lines        1260     1286      +26     
  Branches      204      206       +2     
==========================================
+ Hits         1226     1238      +12     
- Misses         16       30      +14     
  Partials       18       18              
Files Changed Coverage Δ
simple_history/admin.py 90.69% <48.14%> (-7.94%) ⬇️

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

@mjsir911
Copy link
Contributor Author

There's another peculiarity in this patchset that I'de like to see if I can find a work around for: the SimpleHistoryShowDeletedFilter.queryset() is returning a differently-typed queryset when enabled: HistoricalQuerySet[HistoricalChanges] when self.value() vs QuerySet[Model] when not.

Would there be any way to coerce a queryset of historical changes to a queryset of the underlying history_object?

Functionally, what I want is this:

queryset.model.history.filter(history_type='-').latest_of_each().values("history_object")

but this doesn't work since HistoricalChanges.history_object is a descriptor.

Additionally, since the database objects that we're filtering by are deleted anyways, would it be possible to get a queryset containing them? How .history_object works is by creating an ephemeral object in-memory, not sure if this will work with querysets, but I know django-simple-history already creates it own modified(mock?) querysets for things.

@mjsir911 mjsir911 force-pushed the msirabella/admin_panel branch 2 times, most recently from 0c116b1 to 2ccf550 Compare July 28, 2023 22:47
snps-msilva and others added 6 commits July 28, 2023 15:47
Note that if you are using any custom methods on the model for the admin
display, you need to use a History Base class for this to work(jazzband#311).
I think the ChangeList.apply_select_related() method might have changed.
.distinct() -> .latest_of_each() prevents multiple deletes from adding
multiple rows for the same object.
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.

View deleted instances from admin page
4 participants