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
Email templates offer first alphabetically available installed language even if language is not active #6432
Comments
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? |
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.
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: |
Hi @jmvezic, I'm not sure that I fully understand the issue yet. Here's what should be happening:
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? |
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:
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:
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. |
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. |
Please have a look at #6549 as well. |
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. 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 #6549 ) . It think this is one of the culprits. |
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. |
@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!) |
@asmecher @NateWr perhaps this might also be a good issue to resolve with 3.4. milestone: https://github.com/pkp/pkp-lib/milestone/49 |
Hi all, |
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. |
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)?! |
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? |
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. |
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 |
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. |
I've confirmed that this is still happening in the pkp-lib/classes/emailTemplate/DAO.php Lines 157 to 159 in b7aa8b5
@asmecher I can think of two ways to handle this:
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:
|
Is this issue maybe related to this one: #7300 ? I will need to double check, at the first glance it sounds so... |
@NateWr, good news -- Laravel does abstract JSON queries: https://laravel.com/docs/9.x/queries#json-where-clauses |
@asmecher it looks like the column has to be a JSON column. But this data is stored as JSON in a generic |
@NateWr, maybe ca. this way, using LIKE:
|
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:
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. |
@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? |
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). 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. |
Looking a bit more deeply into it, I believe we'd need the equivalent of the MySQL 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, '"%') |
PRs |
@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. |
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:
I'm not prepared to make this change for 3.4, but I wonder what the result would be of:
I'm OK to proceed with your proposal for 3.4. |
I think these are ready to go -- thanks, @Vitaliy-1! |
#6432 Retrieve emailTemplates data for supported locales only
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? |
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 |
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:
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
The text was updated successfully, but these errors were encountered: