Opened 11 years ago

Closed 11 years ago

#20911 closed Cleanup/optimization (wontfix)

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

Reported by: German Larrain Owned by: nobody
Component: contrib.admin Version: dev
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.

Change History (2)

comment:1 by Tim Graham, 11 years ago

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 by Simon Charette, 11 years ago

Resolution: wontfix
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top