Context object name is translated string

[szczav]

I'm using latest trunk. While I was using generic views all was perfect until I made some translations. Then some of my views crashed with VariableDoesNotExist("Failed lookup for key... I found out that when there is no context_object_name defined in generic class view then default value of this variable is not a object name, but its translation. In my case model name is Company so default context_object_name should be "company" and so it is so until I make translations, then it is (in my case) "firma". Context variables can't be dependand on translations and should be lowercased model name. Now it's inconsistent and may cause crash after modifying translations.

It seems that fix is quite simple: in all methods get_context_object_name in ObjectMixins there is line with obj._meta.verbose_name.lower(). It should be replaced by obj._meta.object_name.lower().

Tomorrow I'll try to do patch & tests.

[anonymous]

This looks like it might be a release blocker.

Using object_name is a good suggestion, but it won't work, because we need a plural version too (for list views).

[Russell Keith-Magee]

That last comment was me. Trac logged me out again...

[szczav]

Patch and tests

[szczav]

I think that using verbose name as context variables names is bad idea. Verbose name is used to provide translations, not to provide variables names. We can create some default translations using variables, but it shouldn't work in the other direction. I can even imagine case when somebody creates verbose name containing only national letters which now would be replaced by underscores so we would have context variables like 'a_' or '_'. I don't see better way to create names of context variables than adding postfix '_list' to object_name. This pattern is often used in Django. So model Article for detail list view will have article_list variable. It's easy to create, and guaranties safety.

There is also other inconsistency in generic views. Error messages for 404 aren't translated but object name passed to them is again verbose_name:

raise Http404("No %s available" % model._meta.verbose_name)

Translating only part of the string makes no sense. For developer such message is confusing, especially when he doesn't know language used in translation. This also should be model._meta.object_name.

I made patch with fixes & tests for booth cases: context variables and error messages.

[Łukasz Rekucki]

This is a bit related to #14018. In both cases, it would be great to have a pluralized version of the model's name, but a one that's a valid variable name. Falling back to "%s_list" is a good idea. Adding name_plural to model's Meta would allow for optional nicer names. Either way, this also needs an update to the docs. Current docs don't even describe the current behaviour (maybe just remove it then, as undocumented ?).

As for error messages, it's a separate issue that should be dealt with a separate ticket. IMHO, they should be translated, so using verbose name is ok there.

[szczav]

fix, tests, docs

[szczav]

I've removed error message fixes (I'll create separate ticket on this) and added docs. Way how context variables are created is described in docs, so I've just rewritten it. By the way get_context_object_name() description for the SingleObjectMixin was completely wrong before.

Idea of #14018 is good but as I see its milestone is 2.0 so probably we have to stay with object_name concept.

[Łukasz Rekucki]

Replying to szczav:

I've removed error message fixes (I'll create separate ticket on this) and added docs. Way how context variables are created is described in docs, so I've just rewritten it. By the way get_context_object_name() description for the SingleObjectMixin was completely wrong before.

Looks good to me, but didn't actually run it, so someone should take a look at it.

Idea of #14018 is good but as I see its milestone is 2.0 so probably we have to stay with object_name concept.

Not everything people set on Trac is accurate. IMHO, adding "name_plural" to Meta is fully backwards compatible. If it's actually worth the small improvements, is a different matter. But it's also out of scope for 1.3 as a new feature (could be added in 1.4). It definitely shouldn't block a release, so going with object_name is the right way here :)

[Łukasz Rekucki]

Forgot about the flags again.

[Adam Nelson]

All tests pass for me and the patch looks consistent. I don't know what's up with the milestone though - up to somebody else.

[Russell Keith-Magee]

In [15531]:

Fixed #15272 -- Altered generic views to use the guaranteed untranslated object_name, rather than the possibly translated verbose_name(_plural) for default context objects. Thanks to szczav for the report and patch.

This is BACKWARDS INCOMPATIBLE for anyone relying on the default context object names for class-based Detail and List views. To migrate, either update your templates to use the new default names, or add a context_object_name argument to your generic views.

[Jacob]

Milestone 1.3 deleted