text collector task: automatically generated items for db fields

[wernerkrauss]

As discussed with @sminnee at #SilverstripeEU:

There is already an automagic method for translating db fields in https://github.com/silverstripe/silverstripe-framework/blob/3.1/model/DataObject.php#L3292

E.g. if i define a translation for MyDataObject.db_Title it gets grabbed and applied there.

But noone knows about it. Many modules use the fieldLabels() method to define translation keys which is just a double work.

Why not modify DataObject::provideI18nEntities() to provide all db_, has_one key also?

I'd be happy to provide a PR.

Take also #3581 in account if it gets merged

[chillu]

Just for clarification - at the moment, it looks like fieldLabels() would return duplicate keys for the same item, e.g. array('db_Title' => 'Titel', 'Title' => 'Titel'). You're right, I don't see how those would be used differently. summaryFields() and FormScaffolder use fieldLabel(), which in turn uses the unprefixed version (Title rather than db_Title). So while we could change both implementations to look for both prefixed and unprefixed versions, that'll leave other custom consumers of fieldLabel() broken. Personally, I think using the unprefixed version is more straightforward, and would recommend we remove the db_ prefixed stuff from the implementation. If we decide to keep it and make its use consistent, there's quite a bit of work connected to changing the existing translation keys for all languages and modules. And since those keys won't be backwards compatible, but the language files are merged back to older release branches (at least in the case of core), this would mean duplication in the translation files, which actually has a much bigger impact than the initial effect you described. To be clear, both removing db_ or changing use to db_ would be considered an API change. Sorry for not explaining this better, its too early for that stuff here still ;)

[wernerkrauss]

Hi Ingo, the prefixing saves us work so we don't need to define field label translations by hand.

DataObject's method fieldLabel() uses fieldLabels() and returns FormField::name_to_label() if no label is defined in fieldLabels(). So form scaffolder would benefit from this, i think.

In the end it 's all about defining a standard way for translating field labels, either way it's work and communication about the best practice. I'd go for the prefixed version, as it's already implemented (though undocumented) and easy to use. And prefix saves us headache if we use e.g. the Title key for anything else.

ATM the prefixed translations are overwritten by $this->stat('field_labels')

Of course we'd also have to modify DataExtension::updateFieldLabels to use prefixed (or whatever) labels automatically.

However, a "magic" way or at least less redundant work for translating field labels would be a great feature and would clean up a lot of code. And defining the field labes manually is just a boring and useless work ;)

So my proposal:

• define standard (english) $field_labels in static array in DO
• export prefixed labels with static $field_labels['Field'] as default value, fallback to FormField::name_to_label() in text collector task
• in fieldLabels() method use translation if exists, fallback to $field_labels array (the other way round like atm, otherwise translations won't work at all)

[chillu]

ATM the prefixed translations are overwritten by $this->stat('field_labels')

All translations are overwritten by this, not only prefixed ones.

define standard (english) $field_labels in static array in DO

So you mean continue to allow defining these labels without prefix?

export prefixed labels with static $field_labels['Field'] as default value, fallback to FormField::name_to_label() in text collector task

I think it's going to be quite confusing if the keys are different between $field_labels, the YAML file and the key you're supposed to use in fieldLabels().

class MyObject extends DataObject {
  private static $field_labels = array('ZIP' => 'ZIP Code');
  function someMethod() {
    echo $this->fieldLabel('ZIP'); // doesn't work with your proposal, has to be 'db_ZIP'?
  }
}

Does your proposal address the fact I've outlined around duplication in the YAML files for backwards compat? I don't think any solution which requires us to duplicate hundreds of keys across dozens of modules will be feasible.

in fieldLabels() method use translation if exists, fallback to $field_labels array (the other way round like atm, otherwise translations won't work at all)

API breaking change which in my opinion unnecessarily complicates things.

I've written a section for dataobject.md to the effect of my original suggestion (document the status quo of using unprefixed labels):

### Field Labels

`DataObject` subclasses can have autogenerated interfaces such as search and edit forms in [ModelAdmin](/reference/modeladmin).
In order to control the field labels displayed there, you can create a mapping in a `$field_labels` static.
Fields from `$db` are mapped directly, for relation fields (`$has_one`, `$has_many`, `$many_many`) you'll need a prefix.

    :::php
    <?php
    class MyObject extends DataObject {
        private static $db = array(
            'ProductCode' => 'Varchar'
        );
        private static $many_many = array(
            'Tags' => 'Tag'
        );
        private static $field_labels = array(
            'ProductCode' => 'Unique SKU',
            'many_many_Tags' => 'Keywords'
        );
    }

For more advanced cases, you can also overwrite the `fieldLabels()` method and determine the mapping programmatically.

The `$field_labels` mapping can also help to keep your database schema labels in English (good development practice),
while showing labels in the language your users speak. SilverStripe will export all labels into its language files
for you to translate into multiple languages (see [i18n](/topics/i18n)). This means instead of writing 
out a label string, you can use the `fieldLabel()` shortcut.

    :::php
    class MyObject extends DataObject {
        // ...
        public function getCMSFields() {
            $fields = parent::getCMSFields();
            // same result as _t('MyDataObject.ProductCode')
            $fields->push(TextField::create('ProductCode', $this->fieldLabel('ProductCode')));
            return $fields;
        }
    }

[wernerkrauss]

SilverStripe will export all labels into its language files for you to translate into multiple languages (see i18n

really? I just tried (in 3.1.6) but the content of $field_labels array isn't grabbed by text collector task. Only $singular_name and $plural_name.

Though i don't get from current code where the translation for unprefixed db fields is grabbed... if not manually in each fieldLables() by defining a ton of _t() statements i have to code manually.

in fieldLabels() method use translation if exists, fallback to $field_labels array (the other way round like atm, otherwise translations won't work at all)

API breaking change which in my opinion unnecessarily complicates things.

IMHO it's more a bugfix than an api breaking change... ATM we grab the translation and overwrite it with the english original string. This way $this->fieldLabel('Field') doesn't work without manually defining translating it in my DO's fieldLabels() method.

I think it's going to be quite confusing if the keys are different between $field_labels, the YAML file and the key you're supposed to use in fieldLabels().

and the key already used in fieldLabels().

define standard (english) $field_labels in static array in DO
So you mean continue to allow defining these labels without prefix?

In your documentation example some fields (relations) are prefixed, db fields not... What are you referencing to? FormScaffolder? If we use prefix for everything exept $db then we need to grab translation without prefix.

However, i don't know what to do with the double strings. Many of them are surely caused because translation strings are created in different ways manually (Wildwuchs)... not by a defined schema everyone is supposed to use. All i've seen is the code in DataObject::FieldLabels(), is this not the same schema / system as used in other (newer) code, scaffolding etc.???

[chillu]

SilverStripe will export all labels into its language files

You're right, I've checked en.yml in framework/ for Member to verify this (first core class I could think of), which of course is a bad example because of an overwritten Member->fieldLabels() implementation.

IMHO it's more a bugfix than an api breaking change... ATM we grab the translation and overwrite it with the english original string.

Its breaking existing implementations where both $field_labels and custom YAML configs or custom fieldLabels() is used, because it changes the order - so it's an API change. If you don't want $field_labels to interfere with your translations, then don't define that static. You can set your english strings in en.yml. That being said, we could change this API in 4.0 if its clearly documented.

OK, I think we're getting to the root of the problem, thanks for your patience. FormScaffolder is assuming unprefixed fieldLabel() use, while the default fieldLabels() implementation suggests prefixed keys. Core uses unprefixed labels in its custom fieldLabels() implementations (e.g. in Member). Then there's $summary_fields which is kind of similar and uses unprefixed keys as well.

On adding those prefixed fields to provideI18nEntities(), that's been removed eb4c950 back in 2008. I can't really remember why I did that to be honest, but we'd need to have a think about how this works with DataExtension.

[GuySartorelli]

This was implemented somewhat recently

[wernerkrauss]

😍😍 Cant wait to test it in real life! Thanks a lot!