Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#23098 closed Bug (fixed)

ugettext_lazy gets translated too early in modelForm field choices

Reported by: Mathieu Agopian Owned by: nobody
Component: Forms Version: 1.7-rc-1
Severity: Release blocker Keywords:
Cc: Florian Apolloner 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 ticket first was submitted against the pytest_django repository: https://github.com/pelme/pytest_django/issues/135

However, I think the maintainer is right and it's a Django bug: because of the new feature allowing to customize the "empty choice", the lazy translation of named groups in a choice list for a modelForm field gets translated at import time.

The steps to reproduce are in the ticket linked above, and consists in:

  • have a choice list for a model field which contains name groups, with the groups' names being translated (using ugettext_lazy)
  • create a ModelForm for this model
  • import the ModelForm from an external script that has the settings configured, but not called django.setup() yet

I understand the steps to reproduce are really a corner case and may seem irrelevant, but this bug means that we might have lazy translations that are translated way to early (at import time) instead of staying lazy.

With the normal Django usage, this will translate too early silently, and not raise any error exception.

From what I understand, this happens in the code that checks if the choice (in our case, the name of a group) is the empty choice: https://github.com/django/django/blob/master/django/db/models/fields/__init__.py#L732

I can imagine two solutions to this issue:
1/ only do the check if the "choice" isn't a group name
2/ do something like choice is None or isinstance(choice, six.text_type) and choice == ''

Change History (10)

comment:1 Changed 4 years ago by Claude Paroz

Component: TranslationsForms
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:2 Changed 4 years ago by Claude Paroz

Has patch: set

comment:3 Changed 4 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Should the version be 1.7?

comment:4 Changed 4 years ago by Tim Graham

Version: master1.7-rc-1

Confirmed it's a regression in 1.7.

comment:5 Changed 4 years ago by Aymeric Augustin

Resolution: invalid
Status: newclosed

This report seems invalid to me. The reproduction instructions include:

import the ModelForm from an external script that has the settings configured, but not called django.setup() yet

But the 1.7 release notes explain that you should call django.setup() before doing anything else.

comment:6 Changed 4 years ago by Aymeric Augustin

Resolution: invalid
Status: closednew

Mmm, regardless, if the translation is resolved at import time, that's bad. We should at least fix that.

comment:7 Changed 4 years ago by Mathieu Agopian

Aymeric, I'm sorry that I confused things by talking about the pytest usage in the STR, while it actually has nothing to do with the issue. It's just that I discovered this issue in this particular case, while it was just "failing" (translating to early) silently otherwise.

As you said, the issue is that the translation is resolved at import time, and that's the point of this ticket (hence the title), sorry if I made it harder to understand by providing to much (or confusing) details.

comment:8 Changed 4 years ago by Florian Apolloner

Cc: Florian Apolloner added

comment:9 Changed 4 years ago by Florian Apolloner <florian@…>

Resolution: fixed
Status: newclosed

In 2f73b527dda6683868fac2791f7f07ccb01ea0d9:

Fixed #23098 -- Checked that lazy choices are not evaluated too soon

Thanks Matthieu Agopian for the report.

comment:10 Changed 4 years ago by Florian Apolloner <florian@…>

In 99c2c917c3f859f2cc3fc1f7d5abb72730aa6c48:

[1.7.x] Fixed #23098 -- Checked that lazy choices are not evaluated too soon

Thanks Matthieu Agopian for the report.

Backport of 2f73b527dda6683868fac2791f7f07ccb01ea0d9 from master.

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