Opened 12 years ago
Closed 10 years ago
#20133 closed New feature (fixed)
Admin: Add deletion summary
Reported by: | Jonas H. | Owned by: | Areski Belaid |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Areski Belaid | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description
Here's a first version of a patch that adds a short modelname => number-of-deleted-objects-of-that-model
map to the admin object deletion confirmation page.
The patch is by no means RFC, it's only meant to be a first sketch in order to get feedback about the idea in general. I've also done some refactoring as preparation for implementing this feature. To be specific, there was some code that mixed logic and presentation that is now a bit more separated; although there's still potential to reduce complexity by factoring get_deleted_objects
*into* the views (i.e. removing the helper function entirely).
Attachments (1)
Change History (17)
by , 12 years ago
Attachment: | patch1.patch added |
---|
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 12 years ago
Any comments on the patch or the idea in general? Feature requests? Site design/structure?
comment:3 by , 12 years ago
Btw there's some serious DRY violation in that particular area of the admin, both in the views and templates. If anyone with a bit more knowledge of the admin code base has any tips on how to refactor this, please let me know.
comment:6 by , 10 years ago
+1
I quite like the idea, pity we didn't get much traction on this ticket.
Maybe you should send a Pull Request on github, it might be a easier way for core dev to rebase the patch on master and test it.
comment:7 by , 10 years ago
Cc: | added |
---|
comment:8 by , 10 years ago
Patch needs improvement: | set |
---|
Anyone is welcome to rebase the patch and test it. I would just say that I'd prefer that commits separated refactoring of logic from the adding of new features if possible as this makes the diffs smaller and easier to review.
comment:9 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 10 years ago
here the new PR https://github.com/django/django/pull/3052
I followed timgraham advice and so removed the refactoring part, I will send an other PR for it
comment:13 by , 10 years ago
Ignore the previous PR, it breaks many things on Django.
This new one is more straightforward and should pass all Django tests.
https://github.com/django/django/pull/3063
comment:14 by , 10 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | unset |
Looks simple enough, but needs tests and release notes.
comment:15 by , 10 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Tests + release notes added
comment:16 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Great idea!