Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15997 closed Cleanup/optimization (fixed)

Allow customization of MAX_SHOW_ALL_ALLOWED in admin views

Reported by: jsdalton Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: jsdalton, adehnert Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

(This is essentially a re-opening of #1342, which was marked as fixed despite the fact that the ability to customize MAX_SHOW_ALL_ALLOWED was never actually implemented. Some discussion here.)

In a nutshell, MAX_SHOW_ALL_ALLOWED is currently fixed at 200 and hard coded in contrib/admin/views/main.py. Minimally, this should be configurable in some way shape or form outside of modifying Django source code. Ideally, we should be able to modify the value on a per site and per model basis.

I will be working on a patch to address this. Any comments or suggestions are welcome.

Some discussion here.

Attachments (2)

max_show_all_in_model_admin.diff (14.6 KB) - added by jsdalton 3 years ago.
max_show_all_in_model_admin.v2.diff (14.7 KB) - added by jsdalton 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by lukeplant

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by jsdalton

  • Cc jsdalton added
  • Has patch set

Okay, patch attached.

  • Added an attribute to ModelAdmin, list_max_show_all, with a default value of 200, which for all intents and purposes replaces MAX_SHOW_ALL_ALLOWED.
  • Added validation tests as well as ChangeList tests which exercise the show all behavior (previously, no such tests existed).
  • Fixed a few other tests to account for the new initialization parameter added to ChangeList.
  • Added documentation for the new feature.

Changed 3 years ago by jsdalton

comment:3 follow-up: Changed 3 years ago by adehnert

  • Cc adehnert added
  • UI/UX unset

Does anything need to be done to make this patch acceptable?

comment:4 in reply to: ↑ 3 Changed 3 years ago by julien

Replying to adehnert:

Does anything need to be done to make this patch acceptable?

The proposed patch looks good and it contains documentation and tests. However, it doesn't apply cleanly to trunk so at the very least it needs to be updated and reposted here. Then one needs to review the patch more closely to verify that it fixes the issue properly and that the tests are correct and pass. If that's the case, then the ticket can be moved to the "Ready for checkin" stage, otherwise a comment should be left here explaining how the patch could be improved. You can find more detailed info on the various ticket triage stages here: https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#triage-stages

comment:5 Changed 3 years ago by jsdalton

Clean patched attached.

@adenhert please do feel free to examine the patch, and if it looks good mark as ready_for_checkin. Otherwise, you can comment here if you spot any issues and I'll take a look and hopefully address them.

Changed 3 years ago by jsdalton

comment:6 Changed 3 years ago by jezdez

  • Triage Stage changed from Accepted to Ready for checkin

LGTM

comment:7 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [16725]:

Fixed #15997 -- Added list_max_show_all option to ModelAdmin in replacement for a global module level variable. Thanks, jsdalton.

comment:8 Changed 3 years ago by jacob

  • milestone 1.4 deleted

Milestone 1.4 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.