peeringdb/django-peeringdb

verbose_name for models and fields

Closed this issue · 12 comments

It would be super useful to add verbose_names to all models, but also on all fields as well. Right now there's only verbose_name_plural in a handful of models.

In their absence, these are automatically created by Django (from Django docs: "If the verbose name isn’t given, Django will automatically create it using the field’s attribute name, converting underscores to spaces"), but in most cases that name isn't the right one, i.e. the one listed in PeeringDB's web interface. For example, info_traffic becomes Info Traffic instead of Traffic Levels, asn becomes Asn instead of ASN or Primary ASN, etc.

As friendly names are likely needed by every user that is building UX around this library and there are actually well-defined friendly names in use by upstream, I think it would make sense to include these in this library, rather than have everyone replicate PeeringDB's labels by hand.

Also note that verbose_names are automagically used by many third-party libraries as well, like e.g. django-tables2 and django-filters, which right now require writing a lot of boilerplate code to make e.g. the table headings, forms etc. show something meaningful.

I'd submit a pull request, but harvesting those labels is a lot of work, and I'm kinda hoping that you have access to PeeringDB source code or its authors :)

Well… I actually prepared a commit for it after all, as can be shown above :) It depends on #25 and #26, I'll push a PR as soon as those two get hammered out and (hopefully) merged.

grizz commented

We'll have to verify how this affects the translations, but looking good, thanks.

vegu commented

I merged the pull request too early, reopening this.

Any way I can help move this forward?

grizz commented

Nope, we're working on the new translation software now, will check this after that is complete. Thanks!

Ping! Can I help?

grizz commented

@paravoid have you tested this against the translations to see if it breaks them?

It's been a year and a half so... I don't remember. Which translations are we talking about here?

grizz commented

changing verbose names invalidates the translations (https://github.com/peeringdb/translations) for those columns

I'm actually not sure how this translation pipeline works. It seems like that translation repository has strings that are found not in this codebase. As an example:

class FacilityBase(HandleRefModel, AddressModel):
…
    npanxx = models.CharField(max_length=21, blank=True)

The change above is adding a verbose_name of "NPA-NXX" to the field which is how this is titled on peeringdb.com. The Django generated verbose name for this would be "npanxx".

The string "NPA-NXX" does not show up in the django-peeringdb project at all (i.e. "git grep -i" does not find it). In the translation repository, however, there is a translation string for "NPA-NXX".

So my guess is that peeringdb.com is naming the fields somewhere else outside of the database models (and this project). This commit tried to add these labels to models.py where they belong, so that other projects can use that consistently. I also tried to use the language used in peeringdb.com (while browsing the site), and I also used gettext as you can see, so all in all I think this has the potential to increase translation coverage, not reduce it :)

Does that make sense?

I should mention: model fields are not supporting translations at all right now. Per the Django documentation:

It is recommended to always provide explicit verbose_name and verbose_name_plural options rather than relying on the fallback English-centric and somewhat naïve determination of verbose names Django performs by looking at the model’s class name:

from django.db import models
from django.utils.translation import gettext_lazy as _

class MyThing(models.Model):
   name = models.CharField(_('name'), help_text=_('This is the help text'))

   class Meta:
       verbose_name = _('my thing')
       verbose_name_plural = _('my things')

TL;DR: this is not a l10n/i18n regression, but actually the contrary.

OK, so I rebased on current master, ran black again, and changed all the strings to match the PeeringDB translation strings.

Note that this is still a no-op for the main PeeringDB code with regards to localization (or otherwise), as it's not using those verbose names anywhere but rather (presumably) embed those strings (like "NPA-NXX") somewhere else in the code, maybe in the templates?

As a future step, it'd be nice if this project gained its own django.po, and the translations for these strings were moved to this repository so that other downstream users can benefit from it. This can be tracked in another issue, however.