Code

#20911 closed Cleanup/optimization (wontfix)

There are still references to `module_name` in contrib.admin

Reported by: glarrain Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: module_name, model_name
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi.

In app django.contrib.admin, deprecated model meta class attribute module_name was used a lot. That was a very bad name for what it really was (lowercased model name) and some cleanup efforts were performed. However, in master branch module_name is still present albeit in a different form (as a template context variable).

It is calculated in module options, methods changelist_view (line 1410) and history_view (line 1532). It is used in templates actions (line 11) and object_history (line 8).

Relevant related tickets: 20853, 20160, 15294, 6903#comment:71 but particularly 19689.

Attachments (0)

Change History (2)

comment:1 Changed 11 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Agreed, it isn't a good name, but I'm inclined to say "won't fix" since it seems difficult to do this and maintain backwards compatibility:

  1. Have to continue passing the old name in the context
  2. Can't really flag it as deprecated
  3. If we change the name that's used in the template, if someone is still passing the old name, it will no longer be used.

comment:2 Changed 11 months ago by charettes

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

Backward compatibility is the main reason it wasn't changed in the first place but the fact the injected module_name is not opts.model_name.lower() but opts.verbose_name_plural was also part of it.

The initial goal behind #19689 was to avoid computing opts.model_lower() all over the code base not to find a better wording for module_name.

I also agree that module_name is poorly chosen in this case too but as @timo said it seems quite hard to maintain backward compatibility while deprecating it.

The only solution I could come up with that addresses the concerns raised above is quite convoluted:

  1. Inject model_name instead of module_name
  2. Create a template tag (e.g. {% renamed old new %}) that raises a deprecation warning if old is present in the context and renders it if it's the case or defaults to displaying new if it's not.
  3. Create a TemplateResponse subclass that walks the parsed template to find module_name references and raise deprecation warnings if it's the case.

All in all I don't think it's worth the trouble and the maintenance burden thus I'll won't fix this ticket.

If you find a more elegant solution which maintain backward compatibility while accounting for template extending and the concerns raised above by @timo feel free to re-open.

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.