Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow lazy translation strings in email contexts #1442

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

realsuayip
Copy link
Contributor

No description provided.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I cannot find any mentions of this in either:

Can you please point to exact places in the docs / source?

@realsuayip
Copy link
Contributor Author

I could not find a reference to it either. However, I think it is a valid (and common) use case. In my experience, any user-facing string tends to be translatable in Django, and in the case they are not, some exception is raised, making it necessary to use force_str.

@sobolevn
Copy link
Member

@adamchainz can you please take a look?

@intgr
Copy link
Collaborator

intgr commented Apr 17, 2023

It has been many years since I used translations in Django, but for back when I used them, my experience matches realsuayip's description:

In my experience, any user-facing string tends to be translatable in Django

The relevant documentation is https://docs.djangoproject.com/en/4.2/topics/i18n/translation/#working-with-lazy-translation-objects, quote:

The result of a gettext_lazy() call can be used wherever you would use a string (a str object) in other Django code, but it may not work with arbitrary Python code.

I think the Django documentation overstates it, reality is more nuanced. Obviously, there are str params for machine identifiers (e.g. IntegerField(db_column=_("translate this")) or .prefetch_related(_("translate this"))) that don't reasonably support lazy translations. But for user-readable message strings like email titles, the Django intends to allow lazy strings everywhere.

Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the Django code, but it's not straightforward. @realsuayip have you tested that these functions work with lazy strings?

As described in my previous message, I'm inclined to accept this PR, but I'll leave final decision to sobolevn.

@realsuayip
Copy link
Contributor Author

@intgr Yes, eventually all functions end up creating EmailMessage instances, which itself handles translatable strings.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks like it works locally! Thanks everyone!

@sobolevn sobolevn merged commit 67ec985 into typeddjango:master Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants