'Can access Wagtail admin' is not localized

[children1987]

I found there is a problem about localization, as this img:

wagtail/wagtail/admin/migrations/0001_create_admin_access_permissions.py

Line 21 in 882f8f3

name='Can access Wagtail admin'

I think it should be change to:

from django.utils.translation import gettext_lazy as _
# Create admin permission
admin_permission, created = Permission.objects.get_or_create(
    content_type=wagtailadmin_content_type,
    codename='access_admin',
    name=_('Can access Wagtail admin')
)

[kaedroho]

This is tricky because this string is stored in the database. Running gettext on it in a migration will have no effect as gettext won't translate things in a migration (it doesn't do anything unless a language has been activated by something like LocaleMiddleware).

A possible but hacky looking solution could be to pass the database value into gettext when it's displayed, for example:

_(permission.name)

or:

{% trans permission.name %}

As long as the literal value is defined somewhere (eg the migration) so that gettext can extract it, the string should have a translation that it could use. The only issue here would be if someone manually changed this string in the database.

[thibaudcolas]

@kaedroho when you have the chance, could you add more info on where to do what you suggested? Is it in formatted_permissions.html? This seems like a good "good first issue" if we can tell people exactly where to change this.

[gasman]

See also #7749.

I can see a possible gotcha with @kaedroho's suggestion - if the translated string is part of the migration, and a non-English locale is active when running migrations (e.g. LANGUAGE_CODE is set), it'll end up inserting the non-English text in the database, which then won't be translated by {% trans permission.name %}. Given the untried nature of this, I don't think it's a good fit for "good first issue".

[thibaudcolas]

As far as I understand, this is exactly what @kaedroho flags, and his suggestion is to call gettext when the database value is displayed while the migration would be kept as-is.

[gasman]

It's more subtle than that - there needs to be an occurrence of _('Can access Wagtail admin') somewhere in the codebase so that it gets picked up by makemessages, but that occurrence can NOT be anywhere that would be encountered when creating the permission record in the database (because that would result in it inserting the translated record into the database rather than the English one).

[thibaudcolas]

I understood Karl as suggesting we’d have a dummy _('Can access Wagtail admin') somewhere for the purpose of extraction of the message. So:

1. No changes to the value stored in the database
2. Somewhere, we have a dummy gettext call with the same literal value as what’s stored in the database, for the purpose of collecting by makemessages.
3. We also call gettext on the database value, which will leverage the fact we have a dummy translation string elsewhere

[SebCorbin]

When declaring custom model permissions as documented in Django,

class Mymodel(models.Model):
    class Meta:
        permissions = [("can_deliver_pizzas", _("Can deliver pizzas"))]

the migration generated is:

migrations.AlterModelOptions(
    name="mymodel",
    options={
        "permissions": [("can_deliver_pizzas", "Can deliver pizzas")],
        "verbose_name": "Mymodel",
    },
),

so this seems quite safe to only try to translate in the template:

{% for custom_perm in content_perms_dict.custom %}
    {% trans custom_perm.name as perm_name %}
    {% include "wagtailadmin/shared/forms/single_checkbox.html" with name="permissions" value=custom_perm.perm.id checked=custom_perm.selected text=perm_name %}
{% endfor %}

[gasman]

@SebCorbin The question is, are we 100% confident that there will never be a situation - in anyone's Wagtail project - where a language other than English is active while makemigrations is running, causing the value

("access_admin", _("Can access Wagtail admin"))

to be resolved to something like

("access_admin", "Kann Wagtail-Admin zugreifen")

while it's scanning for changes to the model state, making it think that there's been a change from what the existing migrations say, and generating a bogus migration in the wagtailadmin app?

I've just been trying this out, and haven't been able to break it in that way - but I couldn't be confident that it could never happen, without some very careful reading of the Django source code.

Having said that, I see no downside to placing _("Can access Wagtail admin") somewhere totally benign in the code as per @thibaudcolas's suggestion - so I'll go ahead and create a PR for that.

[gasman]

Actually, I now see that we've been using that pattern on the Page model's permissions since at least 5.1, so a) if it wasn't safe we'd probably have seen it break by now, and b) we have nothing to gain by avoiding it here :-)

[SebCorbin]

@SebCorbin The question is, are we 100% confident that there will never be a situation - in anyone's Wagtail project - where a language other than English is active while makemigrations is running

I would say yes, because the makemigrations command is decorated to not take translations into account but I'm only 99% sure :)