Skip to content

[5.4] Update notification code improvements#46071

Merged
muhme merged 4 commits intojoomla:5.4-devfrom
joomdonation:update_notification_improvements
Sep 12, 2025
Merged

[5.4] Update notification code improvements#46071
muhme merged 4 commits intojoomla:5.4-devfrom
joomdonation:update_notification_improvements

Conversation

@joomdonation
Copy link
Contributor

@joomdonation joomdonation commented Sep 11, 2025

Pull Request for Issue # .

Summary of Changes

This PR makes several improvements to the code which handles sending update notification:

  • Introduce a new state filter.receiveSystemEmail to UsersModel to allow us to get only users which have Receive System Email parameter set to Yes or No (the current code has to get all users, and filter out the un-wanted users by PHP, an unnecessary extra process).
  • Optimize the code of getEmailReceivers method, uses filter.groups state from UsersModel to get users from all groups at one instead having to loop over every user group like in current code.
  • Only calls code to get user groups with Super User right if there is no user groups configured in Send Email to User Groups parameter in Joomla update component.

Testing Instructions

  • Use Joomla 5.4 nightly build
  • Open the file administrator/components/com_content/src/Controller/DisplayController.php, add these lines of code before return parent::display(); statement
$notificationModel = \Joomla\CMS\Factory::getApplication()->bootComponent('com_joomlaupdate')
		    ->getMVCFactory()->createModel('Notification', 'Administrator');
$notificationModel->sendNotification('success', '5.3.0' , '5.34.0');
  • Access to Articles Management screen to trigger the code to send update notification emails above
  • Check and make sure update notification email is works like before, mean all enabled users who have Receive System Email set to Yes from configured user groups receive update notification emails

Actual result BEFORE applying this Pull Request

  • Works

Expected result AFTER applying this Pull Request

  • Works faster with cleaner code

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

@tecpromotion
Copy link
Contributor

Just to make sure I'm testing it correctly.

I have created a few users:

  • A super user
  • An administrator
  • A manager

All users have the “Receive system emails” checkbox selected.

Scenario 1
In the first case, I do not have automatic updates enabled.
I insert the code and call up the articles overview for testing.
Result: “Only” the super administrator receives the email.
I think that's the expected behavior.

Scenario 2
Here, I have automatic updates enabled and added all three groups as recipients.
Result: All three users receive the email.
I think that's the expected behavior.

Scenario 3
Here, I have automatic updates enabled and have not explicitly added any groups as recipients.
Result: “Only” the super administrator receives the email.
I think that's the expected behavior.

Scenario 4
Because the site was set up on localhost, I can activate automatic updates, store the groups, and when I save, I get the message: "The automated update service is unavailable for your site as it's not accessible from the internet. Automated updates have been disabled."
Result: the configured group is still saved and the users in the group receive the emails, even though automatic updates are then inactive.
I think that's NOT the expected behavior. Should there still be a check for the status of auto-updates here? I think so.

@richard67
Copy link
Member

@tecpromotion If scenario 4 is the same without and with this PR, then it's a separate issue. Of course it would be nice if it could be fixed by this PR here, too, but possibly @SniperSister can have a look.

@tecpromotion
Copy link
Contributor

@tecpromotion If scenario 4 is the same without and with this PR, then it's a separate issue. Of course it would be nice if it could be fixed by this PR here, too, but possibly @SniperSister can have a look.

Same with and without PR for Scenario 4.

@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on 01ad884

See my comments here #46071 (comment)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46071.

@richard67
Copy link
Member

@tecpromotion It's worth a question if scenario 4 is really valid. When the server is not reachable from the internet, the notification will never be triggered in the regular way.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 01ad884

I've tested with different users in different user groups with different admin language settings and different user groups selections in the automated updates options.

Basically I've verified that #46050 still works with this PR here applied.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46071.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46071.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 11, 2025
@muhme
Copy link
Contributor

muhme commented Sep 12, 2025

@tecpromotion If scenario 4 is the same without and with this PR, then it's a separate issue. Of course it would be nice if it could be fixed by this PR here, too, but possibly @SniperSister can have a look.

Same with and without PR for Scenario 4.

From my external perspective, scenario 4 is not a use case in practice. I guess you only receive emails through the hack. I believe that if Automatic Updates are disabled, users should not receive emails. It would be great if @joomdonation or @SniperSister could give a definitive answer.

@SniperSister
Copy link
Contributor

Without enabled automated updates, users shall and will not get emails.

@muhme
Copy link
Contributor

muhme commented Sep 12, 2025

Final test before merge with manual PRs full package installation in public available site:

  • With 'Debug System' and 'Log Almost Everything' enabled
  • System - Alpha Update Server 0.6.1 and de-DE_joomla_lang_full_5.4.0v1-beta2 installed
  • Preventing the PR from being overinstalled: chmod 555 tmp
  • For each test enabled Automated Updates, received failed update emails and disabled Automated Updates

Test cases:

  • ✅ 1. Default installation (en-GB, one Super User admin, no groups) > English email for admin
  • ✅ 2. Setting Administrator default language as German > Backend is German > German email for admin
  • ✅ 3. Created users (system-emails enabled, Super Users group) heiko with explicit German set as backend language and gary with backend language explicit set to English > German email for admin and heiko, English email for gary
  • ✅ 4. Created group 'update-to-be-informed' and created with this group the users richard (system-emails disabled) and phil (explicit set backend language to English, enabled system-emails), configured 'update-to-be-informed' group in Automated Updates > English email for phil
  • ✅ 5. Configured group 'Editors' (w/o members) in Automated Updates > No emails (same result, as with nightly build)
  • ✅ Nothing unusual in the Joomla logs and PHP log

@muhme muhme merged commit ffbf854 into joomla:5.4-dev Sep 12, 2025
41 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 12, 2025
@muhme muhme added this to the Joomla! 5.4.0 milestone Sep 12, 2025
@muhme
Copy link
Contributor

muhme commented Sep 12, 2025

Thank you @joomdonation for your contribution. Thank you @SniperSister for your support. Thank you @tecpromotion and @richard67 for testing.

@joomdonation joomdonation deleted the update_notification_improvements branch September 12, 2025 10:41
dgrammatiko pushed a commit to dgrammatiko/joomla-cms that referenced this pull request Jan 25, 2026
* Improve sends update notification code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments