[5.4] Use correct language for autoupdate notification mails#46050
[5.4] Use correct language for autoupdate notification mails#46050muhme merged 14 commits intojoomla:5.4-devfrom
Conversation
|
have you got the before and after the wrong way around? |
|
@brianteeman good catch. Lesson learned: FIRST coffee, THEN code. |
|
I have tested this item ✅ successfully on 8d16414 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
|
From what I see, the change here is not enough yet. You would need to do the same with how we handle mail template language in User - Joomla plugin https://github.com/joomla/joomla-cms/blob/5.4-dev/plugins/user/joomla/src/Extension/Joomla.php#L216-L228 My quick test with current code:
Result:
|
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
|
@tecpromotion Could you test again and in addition test also the scenario described in @joomdonation 's comment #46050 (comment) ? Thanks in advance. |
|
@SniperSister Now PHPStan complains about direct access to the |
|
I have tested this item ✅ successfully on 6c08ecf This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
|
I have tested this with several languages.
I then changed the subject line in the emails and received different emails. |
…om/SniperSister/joomla-cms into 54-autoupdate-notificationlanguage
|
Done @richard67 |
@SniperSister Where? |
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
|
I've restored the previous human test result in the issue tracker as the commits which have invalidated the test count were just a change of the phpstan baseline file and code style. |
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Show resolved
Hide resolved
|
@SniperSister Can it be tested by using the first code snippet from the testing instructions of PR #45721 to trigger the notification mail? |
|
@richard67 the code snippet, an API call, or reaching out to me - whatever works :) |
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
I also used that instructions for testing. |
administrator/components/com_joomlaupdate/src/Model/NotificationModel.php
Outdated
Show resolved
Hide resolved
…onModel.php Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Admin Application and API application differ in setup, might be worth to actually trigger the notification via curl instead to make sure the app context does not influence the result:
|
|
It really depends on the active language of the current running application. The issue will happen if:
I do not understand how the auto update trigger that email yet but I can see the potential issue by reading logic the code. But if no one else see the issue, you can ignore it. |
|
I have not tested this item. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
I have expanded my test scenario. I now have six differently configured users. @joomdonation which case is not covered and can be tested by me? |
|
@tecpromotion Please try with the case :
I'm unsure if you can re-procedure the issue because as I said, I test it by calling the code to send notification directly as described in #45721 (So I logged in using the super user account to trigger the code, mean the active application at that time running on English) |
Result: The email is in German (default backend language), but the only user has English as the backend language in their profile. |
…onModel.php Co-authored-by: Tuan Pham Ngoc <github@joomdonation.com>
|
thx @joomdonation |
|
@joomdonation Could you give it a final test and submit a test result? @tecpromotion Could you submit a test result? I assume you have already tested with the latest change. |
|
I have tested this item ✅ successfully on b3393a4 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
|
If you also want the versions to be displayed in the update email, you can use this command. |
|
I have tested this item ✅ successfully on b3393a4 Thanks @joomdonation and everyone else. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
1 similar comment
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46050. |
|
Final test before merge with manual installation in public available site
|
|
Thank you @SniperSister for your contribution. Thank you @brianteeman, @joomdonation and @richard67 for supporting. Thank you @joomdonation and @tecpromotion for testing. |
|
I made PR #46071 to clean up/improve code further. As you are familiar with update notification email, it would be great if you can help testing it @tecpromotion, @muhme. Thanks ! |
…46050) * Use correct default admin language for update notification mails --------- Co-authored-by: Richard Fath <richard67@users.noreply.github.com> Co-authored-by: Tuan Pham Ngoc <github@joomdonation.com>
Summary of Changes
The default admin language and the selected user language are correctly taken into consideration to determine the correct language for an autoupdate notification mail.
Testing Instructions
Actual result BEFORE applying this Pull Request
Notification mail is sent in EN, ignoring the selected default language.
Expected result AFTER applying this Pull Request
Notification mail is sent in the selected non-en language.
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed