Email templates offer first alphabetically available installed language even if language is not active

[jmvezic]

Describe the bug Don't know if this is exactly a bug or intended behavior, but maybe it should be an option:

To Reproduce Steps to reproduce the behavior:

1. Install some languages as administrator, such as Croatian and Arabic
2. Switch UI to Croatian and under website settings, choose Reload defaults for Croatian
3. Go to workflow settings, e-mail templates
4. Templates which don't have the current language translation available show Arabic (first available alphabetically) instead of English

Disabling Arabic, for example, doesn't help. The e-mail templates still show up as Arabic (I guess since it was installed). I couldn't find the piece of code which handles this, but it seems to be in the API, probably under emailTemplates.

I think it makes sense to always default to English if the current translation is not available, or at least offer the option which language to "baseline" in that case.

What application are you using? OJS 3.2.1-2

Additional information

[NateWr]

Hi @jmvezic, thanks for the report. The email templates should fallback to the journal's primary language. Which language is selected as the primary language under Settings > Website > Setup > Languages?

When you edit one of the email templates that does not have Croatian support, for example Announcement, do you see the email template's subject and body shown in English?

[jmvezic]

Hi @NateWr

Yes, choosing English does fallback the templates to English.

What I described is when using another primary language like Croatian. This is especially pronounced in a multijournal installation where there are numerous languages installed, each journal using another primary locale. From that perspective, it might be a good idea to offer a journal-wide or even a site-wide "fallback" language setting for templates. I think it's a legitimate scenario to have, for example, Croatian as a primary language and not have Arabic as a fallback simply because another journal is using it.

Maybe this can be labeled as a "feature request" in that case, rather than a bug.

When you edit one of the email templates that does not have Croatian support, for example Announcement, do you see the email template's subject and body shown in English?

So in that case, if I have enabled for example, Arabic, English and Croatian, Croatian as the primary language, the preview and description on the list of templates shows up as Arabic but the edit window shows up correctly (English and Croatian). Attaching the screenshots:

[NateWr]

Hi @jmvezic,

I'm not sure that I fully understand the issue yet. Here's what should be happening:

1. Every journal has a primary language and additional languages.
2. When an email template does not have a subject or description in one language, it performs the following steps to find a fallback: a. Use the subject or description in the primary language. b. Use the subject or description in the first available language of this journal.

Are you seeing something different from this behaviour? There should never be a fallback to a language that is not enabled on that journal, but it sounds like maybe you're seeing that?

[jmvezic]

@NateWr

That's right, it falls back to the language that's disabled. It seems like the only thing it checks is if it's installed, and doesn't check if it's enabled or not. Additionally, it seems there are no checks on a journal level whether that specific journal is using the language at all.

This is especially problematic in a multi-journal installation. Say you have the following situation:

• Installed languages in OJS: English, Croatian, French, Arabic
• First journal using Croatian as primary, English as secondary
• Second journal using French as primary, English as secondary
• Third using English as primary, Arabic as secondary

The first two journals will have untranslated e-mail template previews and descriptions shown up in Arabic, even though they have nothing to do with that language.

Additionally, say we also have Catalan installed. In that case, the first two journals will have:

1. E-mail templates showing up in Arabic
2. If that's not available, they'll show up in Catalan
3. Etc. etc.

It creates a jungle of languages in the templates previews which makes it hard to get around to translating them. Not to mention it's additionally confusing that once you click "edit" it suddenly shows up correctly - using the languages you defined - like in the screenshot above.

[NateWr]

Thanks @jmvezic! I understand now and I was able to reproduce the problem. You're right that the locale fallback will find email templates in any installed language, even if the journal has not enabled the language.

So the fallback code is working correctly, but the email templates are loading data for all locales, even those that are not enabled in the journal.

[mpbraendle]

Please have a look at #6549 as well.

[mpbraendle]

It is even worse - an editor of one of our journals (we have a multi-journal / multi-lang installation) complaint that templates that he had changed or added are reset to their default text. That happened several times. He's pretty upset now. His assumption that this happens always after a new submission has been handed in and the submission was confirmed by e-mail.

Is anything going on further with this issue here? I think it is a major bug and should be investigated as soon as possible.

From a database perspective, I still think that it is a bad idea to use the same field name for the id field in the default mail templates table and the mail templates table (see my comment in https://github.com/pkp/pkp-lib/issues/6549 ) . It think this is one of the culprits.

[mpbraendle]

Also, with the OJS 3.2 upgrade (we upgraded from 3.1.2-4) , we had reports of editors-in-chief by two other journals that mails are now being sent to editors that should not receive those. We had to adjust the roles. It looks like the OJS 3.2 upgrade had some software changes to its mail system with negative consequences.

[asmecher]

@mpbraendle, I think it's quite unlikely that submission-related activity (e.g. a new submission being confirmed) is the culprit for submission data getting reset; what specific version of OJS are you using? Can you try to track down the steps that were responsible for the email text being reset, or narrow down the conditions? (This would be a very good potential bug to fix, but it's also really hard for us to debug these situations remotely!)

[gabriele-h]

We have seen a similar issue after our upgrade from OJS 3.1.2-1 to OJS 3.2.1-3 two days ago: We had a look at the database and solved this one occurance of the issue by adding a new line to email_templates_default_data for locale "de_DE" and e_mail_key "ANNOUNCEMENT".

Some info on our system:

• ar_IQ is not installed on the system backend-wise, but we do have it in the locale-files of course.
• We have a multi-journal instance and all of the journals were affected in the same way, most of them have de_DE set as their default.
• We have seen the issue for de_DE only so far, en_US looks fine. The only thing that comes close is a journal using fr_FR besides de_DE and en_US, where some e-mail-templates are shown in de_DE (e. g. ANNOUNCEMENT and CITATION_EDITOR_AUTHOR_QUERY)
• We installed the default translation plugin after the upgrade, but this had no effect on the issue.
• Our production instance was affected for de_DE ANNOUNCEMENT, while our test instance was not (this is where we took the line for the database from).
• The template shown in the screenshot is - as far as we can tell - the only one that was affected with ar_IQ values.
• There were no indications of issues in the output of tools/upgrade.php
• Translations for the missing text were present in the de_DE and en_US po files at the time of the upgrade. We did not check for fr_FR yet.

Please let me know if you need any further information.

[jmvezic]

@asmecher @NateWr perhaps this might also be a good issue to resolve with 3.4. milestone: https://github.com/pkp/pkp-lib/milestone/49

[heike-r]

Hi all, we ran into the same problem after upgrading OJS from 3.2.1-4 to 3.3.0.6. It happened in a multi-journal installation to the journals with German as primary language and English as second one. Most Email templates are in German, but some are now in Arabic and some in Czech. All journals in this installation have either English or German as primary or secondary language choice. None have Arabic or Czech.

[heike-r]

How can I fix this issue? I checked the emails.po file for German locale on our server and the affected email templates seen to lack a German translation. However, this file is "PO-Revision-Date: 2020-12-15 09:06+0000\n" If I replace it with a newer version, which includes the respective translations, would this fix the issue? Can I replace it with a downloaded .po file from weblate or are there any additions neccessary? The current one has the complete German translation for emails.

[heike-r]

It gets more strange the deeper I dig. The same upgrade in the test system a few days prior to the production system upgrade had some untranslated email templates in the German journals as well, but they all fell back to English (the second enabled language) except one that now has a French description and no text or subject (none of our journals has French enabled)?!

[diegomejia071]

Hi,

I am using OJS version 3.2.1.4. Is there any solution for this. Can you delete these email templates in these languages ​​like Arabic from a database query, or what can I do?

[gabriele-h]

As stated in my comment above, for us the issue was resolved by adding missing locale information for the languages we use in the database. For us the change needed to be done in the table email_templates_default_data. But it depends on where in the system the translations are missing which table you need to look at, I suppose. I also can't remember how we found this table.

[NateWr]

I hesitate to recommend a solution without having tested it at all. However, if you have backed up your database, you can try to remove entries for unused languages by removing any rows in email_templates_default_data and email_templates_settings that have a locale column with a language that you don't use.

[gabriele-h]

For us the issue was caused by missing entries in the database, not superfluous ones. Removing missing entries might have you end up with no text at all, I'm afraid.

[NateWr]

I've confirmed that this is still happening in the main branch (3.4 pre-release). This happens because PKP\emailTemplate\DAO::fromRow() gets the default template data for all locales and assigns it to the EmailTemplate:

https://github.com/pkp/pkp-lib/blob/b7aa8b56f0f6ea814c6cc503d296f5387076f21d/classes/emailTemplate/DAO.php#L157-L159

@asmecher I can think of two ways to handle this:

1. Override EmailTemplate::getLocalizedData() so that it doesn't look in unsupported locales. This would mean that wherever we use EmailTemplate objects, we'd have to pass in the valid locales according to the context we're working in. It feels like that could get messy.
2. Change how the DAO and Collector classes work for email templates, so that a context id is always passed in. Then change PKP\emailTemplate\DAO::fromRow() to only fetch the data for locales supported in that context. Repo::emailTemplate()->getByKey() already expects a context id, so we'd just have to modify the Collector -- maybe by making the context id a required argument in the constructor.

At the moment I'm leaning towards #2. However, it's not so straightforward because the supported locales are stored as a JSON string. That means we can't do a simple subquery like this:

DB::table($this->defaultTable)
    ->where('email_key', '=', $emailTemplate->getData('key'))
    ->whereIn('locale', function(Builder $q) use ($contextId) {
        $q->select('setting_value')
            ->from('journal_settings')
            ->where('journal_id', $contextId)
            ->where('setting_name', 'supported_locales');
    })
    ->get();

Or maybe you know some DB magic to do this? Any ideas? cc @Vitaliy-1

Reproduction Steps: From the main branch:

1. Use the test dataset to have English and French templates.
2. Delete the English row for an email template. This is the command for postgres: delete from email_templates_default_data where email_key='DISCUSSION_NOTIFICATION_COPYEDITING' and locale='en_US';
3. Insert a row for another language, like Bosnian. This is the command for postgres: insert into email_templates_default_data (email_key,locale,name,subject,body) values ('DISCUSSION_NOTIFICATION_COPYEDITING', 'bs_BA', 'bs discussion', 'bs subject', 'bs body');
4. Go to Settings > Workflow > Emails > Add and Edit Template.
5. Click Edit for the "Discussion (Copyediting)" email.
6. See that bs discussion is the template name.

[bozana]

Is this issue maybe related to this one: https://github.com/pkp/pkp-lib/issues/7300 ? I will need to double check, at the first glance it sounds so...

[asmecher]

@NateWr, good news -- Laravel does abstract JSON queries: https://laravel.com/docs/9.x/queries#json-where-clauses

[NateWr]

@asmecher it looks like the column has to be a JSON column. But this data is stored as JSON in a generic setting_value column. Should I migrate this data to a JSON column on the main journals table?

[bozana]

@NateWr, maybe ca. this way, using LIKE:

SELECT etdd.* FROM `email_templates_default_data` etdd
JOIN journal_settings js ON (journal_id = 1 and setting_name = 'supportedLocales')
WHERE js.setting_value LIKE concat("%", etdd.locale, '%')

[asmecher]

...it looks like the column has to be a JSON column...

Disclaimer: I haven't used JSON functions in a SQL database ever, and probably nobody else on the team has either, so this is considered experimental until we've got it proven with our target variety of DBMSs.

The data is stored differently depending on DBMS:

• MariaDB: "JSON is an alias for LONGTEXT"
• MySQL: "MySQL supports a native JSON data type defined by RFC 7159 that enables efficient access..."
• PostgreSQL: "JSON data types are for storing JSON (JavaScript Object Notation) data, as specified in RFC 7159. Such data can also be stored as text..."

By my read, all 3 DBMSs support JSON-in-a-string, but MySQL and PostgreSQL also support RFC7159.

I believe we could probably go with either approach -- a JSON-type column, or a string containing JSON as content -- and the DBMS JSON functions will behave well either way. We would probably get a slight performance bump when using RFC7159 but at the moment it'll be negligible in terms of overall app performance.

If you want to avoid biting off more than necessary, I'd suggest just going with the string form; we can reassess later.

[NateWr]

@asmecher Laravel says the JSON support is only in MySQL 5.7+, but our server requirements say we support 4.1+. Can I reassign this to the infastructure team and leave it on your plate to resolve or assign to someone more comfortable with the database requirements?

[jonasraoni]

Just my two cents 😁

When I have stable data structures, I tend to use relationships/extra tables instead of storing data as a JSON column, even though it's more verbose/annoying to deal with (basically I avoid having dynamic/uncontrolled data at all costs). This way, my fast solution would be the @bozana's approach (just would add the enclosing quotes: ") and in the long run I would normalize the data.

---

Given that MySQL 5 has been around for ~10 years, and that our tests aren't even using MySQL 4, I think it's better to raise the version, mostly to not promise something that we're not sure that works. Also, Laravel 7 mentions support for MySQL 5.6 (while Laravel 8+ got it bumped to 5.7): https://laravel.com/docs/7.x/database#:~:text=supports%20four%20databases%3A-,MySQL%205.6%2B,-(Version%20Policy

[asmecher]

Looking a bit more deeply into it, I believe we'd need the equivalent of the MySQL MEMBER OF function to be supported in all required DBMSs (plus Laravel, if we don't want to have to code it individually for each syntax). And unfortunately even MySQL only supports MEMBER OF with 8.0.17, which is a lot newer than our current baseline.

So I agree with @jonasraoni that we should use @bozana's formulation for the moment, except as he already noted, the locale name needs to be quoted, so:

SELECT etdd.* FROM `email_templates_default_data` etdd
JOIN journal_settings js ON (journal_id = 1 and setting_name = 'supportedLocales')
WHERE js.setting_value LIKE concat('%"', etdd.locale, '"%')

[Vitaliy-1]

PRs pkp-lib: https://github.com/pkp/pkp-lib/pull/8572 tests: https://github.com/pkp/ojs/pull/3726

[Vitaliy-1]

@asmecher, can you take a look at the PR? Following the suggestion, instead of filter, context ID should be passed to the Collector's constructor. I've slightly changed implementation discussed here.

[asmecher]

Thanks, @Vitaliy-1, I've reviewed the pkp-lib PR above. I really wanted to find another solution that wasn't invasive, since I think this is fundamentally a display issue and we're reaching pretty far into the back-end to fix it. We have two places (I think) where we have a last-ditch fallback to the first piece of data available:

• In PHP, in DataObject: https://github.com/pkp/pkp-lib/blob/8d57e935ecea86381f8e40f1b285a663378e7fe9/classes/core/DataObject.php#L73
• In JS, https://github.com/pkp/ui-library/blob/04b56f3e20e931a40b27034102bc13c7d741c70d/src/mixins/global.js#L98

I'm not prepared to make this change for 3.4, but I wonder what the result would be of:

• Dropping the "if all else fails, take the first data" fallback in both cases, and
• Adding a concept of locale precedence to the JS implementation, as DataObject has in _getLocalePrecedence.

I'm OK to proceed with your proposal for 3.4.

[asmecher]

I think these are ready to go -- thanks, @Vitaliy-1!

[diegomejia071]

Hi @asmecher and @Vitaliy-1,

Can I apply these changes to OJS version 3.3.0.13, or are they only for version 3.4?

[Vitaliy-1]

Unfortunately not, the way how database queries are managed, as well as some design patterns around public API to interact with data objects has changed in 3.4. I think, it should be enough to adapt correspondent changes in EmailTemplateDAO::_fromRow() but not sure.