Skip to content

[5.4] Send Automated Update Notifications to all super users.#45685

Merged
muhme merged 2 commits intojoomla:5.4-devfrom
chmst:5.4/features/auto-update-notofcation
Jul 12, 2025
Merged

[5.4] Send Automated Update Notifications to all super users.#45685
muhme merged 2 commits intojoomla:5.4-devfrom
chmst:5.4/features/auto-update-notofcation

Conversation

@chmst
Copy link
Contributor

@chmst chmst commented Jul 3, 2025

Pull Request for Issue # .

Summary of Changes

At the moment, a comma separated list of e-mail addresses of super users, who get a notification of auto updates, must be entered in a text field. This could be a source for errors.
This PR removes the field completely, so all super users get an information of automated updates.

Testing Instructions

Quickicon "Automated Update" Or System - joomla - update - options

Actual result BEFORE applying this Pull Request

grafik

All super users whose e-mail adresses are in the list get an update notification for automated updates.

Expected result AFTER applying this Pull Request

grafik

All super users who are enabled for system e-mails get an update notification for automated updates.

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

@chmst chmst requested a review from rdeutz as a code owner July 3, 2025 10:17
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.4-dev labels Jul 3, 2025
@chmst chmst added the UI/UX label Jul 3, 2025
@brianteeman
Copy link
Contributor

At the moment, a comma separated list of e-mail addresses of super users, who get a notification of auto updates, must be entered in a text field. This could be a source for errors.

That is not true as explained in the field description - see the emphasised text

A comma separated list of the email addresses which will receive the update notification emails. The addresses in the list MUST belong to existing users of your site who have the Super User privilege. If none of the listed emails belongs to Super Users, or if it's left blank, all Super Users of this site will receive the update notification email.

I admit I hadnt looked at this before but I note that the task notification has this same option

image

So I guess it doesnt make sense to remove the option from one place and keep it in the other

@brianteeman
Copy link
Contributor

PS this is a perfect example of why hiding field descriptions was such a bad idea

@chmst
Copy link
Contributor Author

chmst commented Jul 3, 2025

I can remove it also from task notification.

The other option is: Make a field where supers users can be selected.
As it is now: I have to check the users and finde isuper users - copy the e-mail addresses. Enter e-mail addresse into the field. Ther can coour copy/paste errors, mising commas and others.
If super users are removed, I have to come back to this list and update. ..
In my opinion this is a perfect example for a bad UX.

@brianteeman
Copy link
Contributor

i agree the ux sucks. never spotted it before myself as I am the only Super User on my sites

if a super user is removed it doesnt matter as the notification is only sent to super users

I would suggest that using the email address in this field was always wrong and it should always have been usernames - you can change your email address - what happens then

@chmst
Copy link
Contributor Author

chmst commented Jul 3, 2025

exactly - but also usernames can change.

@brianteeman
Copy link
Contributor

exactly - but also usernames can change.

true - i forgot that

@muhme
Copy link
Contributor

muhme commented Jul 3, 2025

Thank you @chmst for creating this PR 👍

From my point of view, and after discussions with others and with this PR we prefer not to maintain a separate list of users for Automated Update emails. We already have a user configuration to 'Receive System Emails'. To simplify things, we will use this existing setting and send Automated Update emails to all users who have 'Receive System Emails' enabled.

I haven't heard any arguments for maintaining a separate list, and to me, Automated Update emails are clearly a type of system email. It seems the PR should be adjusted to use users with 'Receive System Emails' enabled, rather than using super users.

@muhme muhme changed the title [5.4.0] Send Auto Update Notiification to all super users. [5.4] Send Auto Update Notiification to all super users. Jul 3, 2025
@brianteeman
Copy link
Contributor

The original pr that introduced this field clearly explained the use case for having rh ability to decide which super users get update notifications.

Now we have two different places with potentially different settings for the same thing.

If you force all super users to receive this notification you are changing the behaviour on existing sites which have restricted who will see the emails.

@brianteeman
Copy link
Contributor

the original pr and the explanation for this option

#6886

By default the plugin sends an update notification email to all Super Users of the site. This may not be desirable, e.g. when you've built a site for your client and you'd rather handle Joomla! updates yourself to prevent potential issues with the site and frantic calls from your client. In this case you can set up one or more email addresses to receive the update notification in the plugin's options

@muhme
Copy link
Contributor

muhme commented Jul 3, 2025

Now we have two different places with potentially different settings for the same thing.

If we remove the input field with this PR we have only one option 'Receive System Emails'.

By default the plugin sends an update notification email to all Super Users of the site. This may not be desirable, e.g. when you've built a site for your client and you'd rather handle Joomla! updates yourself to prevent potential issues with the site and frantic calls from your client. In this case you can set up one or more email addresses to receive the update notification in the plugin's options

For this we have the 'Receive System Emails' Account Details option. If we change the Automated Updates emails to users only with 'Receive System Emails' enabled, this requirement is fullfilled. And it is working the same way as other system emails. Therefore it simplifies the UX.

@brianteeman It feels to me as though we might be talking slightly at cross purposes. I'm not quite sure how else to help, so I’ll try to give an example and make things as clear as possible. Do you agree, or would you suggest a different approach?

  • Imagine you have to reset an administrator to a normal user in your Joomla instance. You have to remove 'Super Users' group assignment, set the user to no more 'Receive System Emails' and without this PR also to remove the email or username from Automated Updates 'Super User Emails'. And this setting for Automated Update emails is unfortunately located in a completely different place from the other user settings.
  • Automated Updates emails are system emails.
  • I welcome this PR to remove the input field. And we should move the overall PR (implementation, description, test instructions) from using all super users emails to all users, who have 'Receive System Emails' enabled. Reusing the existing 'Receive System Emails' setting for Automated Update emails is a logical and consistent choice.

@brianteeman
Copy link
Contributor

@muhme you are completely ignoring the fact that for the last 10 years the joomla update notification has been configurable in addition to the receive system emails for the reason I have quoted above.

Unless you reject the reason given then the same behaviour must be applied here. If you reject the reason given then the behaviour of the joomla update notification needs to be changed to match this proposed change to the automated update notification.

I understand all your comments but you are changing a behaviour that has existed for 10 years. What is worse is that because there are now two different update notifications they are configured separately and behave differently.

What this PR does do is to raise a new issue - why do we have two different update notifications that work in different ways with different configurations. Surely there should just be one now.

@brianteeman
Copy link
Contributor

If we change the Automated Updates emails to users only with 'Receive System Emails' enabled, this requirement is fulfilled.

There is no change needed to do that - that is exactly how the code works today - if it ignores the "receive system emails" then that is a bug

@richard67
Copy link
Member

If we change the Automated Updates emails to users only with 'Receive System Emails' enabled, this requirement is fulfilled.

There is no change needed to do that - that is exactly how the code works today - if it ignores the "receive system emails" then that is a bug

No bug, it works like that, and for the Automated Updates that option had been added in order to be consistent with the same option in the Update Notification Task plugin for the non-automated updates.

And the use case is valid in my opinion. You may have 20 superusers of which 15 shall receive all kinds of system notification mails, and of these 15 only 5 shall receive notifications related to updates.

So for me this PR here is not the right way to go, but that's only my personal opinion.

Another thing is if the UX of that field is really nice or if it couldn't be improved by using a multi-select field where you can select from the users which receive system emails.

Or if it wouldn't be better to select users instead of email addresses.

These are things which might be improved, but that's out of scope of this PR, I think.

But if we decide to go the way of this PR, then I think we should do the same for the (non-automated) update notifications so both are the same.

@brianteeman
Copy link
Contributor

But if we decide to go the way of this PR, then I think we should do the same for the (non-automated) update notifications so both are the same.

absolutely agree 100%

@chmst
Copy link
Contributor Author

chmst commented Jul 3, 2025

My opinion:
A comma separated list of super user e-mails is not acceptable from UX point of view. Neiter a comma separated list of usernames.
I did not see that in the task notification, but making a bad UX here only because we have this in another place is not correct. . This is a new feature, we do not change an existing behaviour.

We can make a field "superUserSelect", this makes it easy for administrators to select only valid recipients.
or
remove the field as I did here and send to all superUsers with "receive system message" enabled. We also can add a note field with a text so everyone knows who are the recipients

Then we can change other usage of notification in another PR which needs deprecations and install scripts. This is not in scope of this PR.

@brianteeman
Copy link
Contributor

i am happy if it works the same in both places however it is done

@muhme
Copy link
Contributor

muhme commented Jul 4, 2025

Updated after I found user group configuration for scheduled task notification emails 44604.

Thank you all for your comments. I tested the existing scheduled tasks update notification emails with different users and played with the existing 'Super User Emails' field and the 'User Groups to Notify' fields in Joomla 5.3.0 administrator backend > System > Scheduled Tasks > Update Notification.

It is working for the 'update available' emails like the field descriptions is:

A comma separated list of the email addresses which will receive the update notification emails. The addresses in the list MUST belong to existing users of your site who have the Super User privilege. If none of the listed emails belongs to Super Users, or if it's left blank, all Super Users of this site will receive the update notification email.

  • And additional you can enable and configure four different notification emails for the task. Here you can configure different groups with 'Super Users' group configured by default. But you cannot configure group(s) for the main purpose, the 'update available' email.
    • Task Success
    • Task Failure
    • Fatal Failures/Crashes
    • Orphaned Tasks

For me, the bad UX is not just the input field - it's the error-prone configuration, which is spread across different places, not standardised and different or misleading labelling.

  1. If I see a switch labelled 'Receive System Emails' in my account settings, I would simply expect to receive those emails when I enable it. However, this actually depends on having Super User privileges, and in addition, email notifications are controlled separately in:
  • System → Scheduled Tasks → Update Notification → Super User Emails
  • Home Dashboard → Automated Updates → Super User Emails
  1. User groups should also be configurable for the main purpose of 'Update Notification'.
  2. In user account details it is named 'System Emails' and in scheduled tasks 'Notification'.

My opinion:

  1. Change this PR to configure user group(s) (like for the standard task operations) instead of having an input field for email addresses.
    • It should be filled with the 'Super Users' group by default.
    • If no group is configured it should default to 'Super Users' group.
  2. Create one more PR for 6.0 (or 6.1) for Update Notification task with
    • Extending notification emails with configuring group(s) to receive the update email
    • Deleting the 'Super User Emails' from Scheduled Tasks > Update Notification. And take it in the release notes as: "Simplify administration and you can configure your own user groups to receive system emails."
  3. Create one more PR for 5.4, improving UX for Account Details > Receive System emails by clarifying that the setting only controls whether the user can receive system emails, e.g. with inline hint (as for the password field), e.g. 'This setting controls whether the user can receive system emails at all.'
  4. Create additional PR for 5.4 to unify naming: Change Systems > Scheduled Tasks > Edit Tasks > Advanced > 'Notifications' field group to 'System Emails'

@muhme muhme marked this pull request as draft July 8, 2025 03:55
@richard67
Copy link
Member

This is a new feature, we do not change an existing behaviour.

@chmst You are right. I forgot about that. As long as not in beta phase we can remove it. So this PR here is indeed a valid option, and other UX fixes for that option for the non-automated update notifications can be done with future PRs, and then the option handled with this PR here could be added or handled with the same kind of fixes.

We will discuss it in the maintainers team. I think @muhme has moved this PR to draft so people won't spend time for testing in the meantime, but I think as soon as we decided it will be moved back to ready for review (non draft).

@muhme
Copy link
Contributor

muhme commented Jul 11, 2025

This PR was discussed in PD CMS Mainenance call. At the moment this PR should be merged as it is (removing the user email field) as Automated Updates is a new feature and the UX of this field is poor. The proposed improvements could then be implemented with new PRs. The PR can therefore now be tested.

@muhme muhme marked this pull request as ready for review July 11, 2025 09:59
@muhme muhme changed the title [5.4] Send Auto Update Notiification to all super users. [5.4] Send Automated Update Notifications to all super users. Jul 11, 2025
@brianteeman
Copy link
Contributor

it is wrong that the two update notifications can be sent to two different sets of users.

@exlemor
Copy link

exlemor commented Jul 11, 2025

I have tested this item ✅ successfully on d5896c3

I have tested this successfully. The field is gone ;)


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

@krishnagandhicode
Copy link
Contributor

I have tested this item ✅ successfully on d5896c3

Tested successfully, Works as expected.


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

@softforge
Copy link
Contributor

I have tested this item ✅ successfully on d5896c3

I have tested successfully and agree that adding another area for comma seperated is not good


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 11, 2025
@richard67
Copy link
Member

it is wrong that the two update notifications can be sent to two different sets of users.

@brianteeman I don't think they have to be equal as there are different use cases. Manual updates can only be started by a user with the necessary privileges, which automated updates don't require such user interaction. That could justify different recipients.

@brianteeman
Copy link
Contributor

we will have to agree to disagree then ;)

@richard67
Copy link
Member

we will have to agree to disagree then ;)

Agreed :-)

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

muhme commented Jul 12, 2025

A big thank you to @chmst, all commentators and testers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Comments