Skip to content

[6.2] MailTemplate should work with interface#47677

Merged
MacJoom merged 15 commits into
joomla:6.2-devfrom
Digital-Peak:mailtemplate/interface
Jun 16, 2026
Merged

[6.2] MailTemplate should work with interface#47677
MacJoom merged 15 commits into
joomla:6.2-devfrom
Digital-Peak:mailtemplate/interface

Conversation

@laoneo

@laoneo laoneo commented Apr 24, 2026

Copy link
Copy Markdown
Member
  • I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.

Summary of Changes

The MailTemplate class should work with an injected MailerInterface and not concrete implementation to be more flexible. Additionally this pr deprecates the optional parameter to be able to remove the deprecated Factory usage.

Testing Instructions 1

  • Log in on the back end
  • Got to the global configuration server tab
  • Send a test mail

Actual result BEFORE applying this Pull Request

Mail is sent.

Expected result AFTER applying this Pull Request

Mail is sent.

Testing Instructions 2

  • Log in on the back end
  • Set HTML as mail format on /administrator/index.php?option=com_config&view=component&component=com_mails
  • Create a new user and enable Receive System Emails
  • Create a GET request task with the url https://downloads.joomla.org/api/v1/latest/cms and Manual execution as Execution Rule
  • In the advanced tab of the task form enable success notification and the respective user group from the previously created user image
  • Save the task
  • Click on the "Run Task"

Actual result BEFORE applying this Pull Request

Mail is sent with attachment in HTML format.

Expected result AFTER applying this Pull Request

Mail is sent with attachment in HTML format.

Link to documentations

Please select:

@CSGoat0

CSGoat0 commented Apr 24, 2026

Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on a59506a

I followed the instructions, everything works fine.


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

@exlemor

exlemor commented Apr 24, 2026

Copy link
Copy Markdown

I have tested this item ✅ successfully on a59506a

I have tested this successfully! Thanks @laoneo!


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

@richard67

Copy link
Copy Markdown
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 24, 2026

@MacJoom MacJoom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check

Comment thread libraries/src/Mail/MailTemplate.php Outdated
@joomdonation

Copy link
Copy Markdown
Contributor

I would question about the valid of this PR. Maybe someone with stronger OPP background should look at this more carefully

  • You modified code to allow using MailerInterface but in the code of MailTemplate class, you have code to check for a concrete implement. Is MailerInterface incomplete ?
  • In terms of features, although the intent is support MailerInterface, any implementation which is not Mail class will loose the functions provided by MailTemplate (because you hard code the check with a concrete implement before calling certain code):
  • Loose the feature to use custom SMTP parameters for each mail template
  • Lose the feature to handle attachments as string

@laoneo

laoneo commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

All it does is to support the interface and doesn't restrict to the Mail class as we should work with interfaces. If you need these parameters like isHTML, then extend from Mail class. But there is no point to not work with interfaces. If I have a plugin which redirects everything to a custom Mailer service or whatever, there is no need to configure SMTP, etc. This brings a lot of flexibility while not loosing any settings from before.

@joomdonation

Copy link
Copy Markdown
Contributor

I think it just a fake support to interface because if you pass any implement of the interface which is not a Mail instance, you will loose the features of MailTemplate which I mentioned earlier. Plus the support interface while having to check for concrete implement seems not right to me.

That's what I see from looking at the change in this PR. I will leave it to other maintainers and release managers to look at it further and make final decision.

@laoneo

laoneo commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Due all respect, but your comment doesn't make sense at all. This is how support for the interface should be implemented in a backwards compatible way. Mailer instances which come from the factory doesn't have to be always a Mail class. If a plugin would implement it's own mailer (which is not a Mail class) and set it in the global container, then this code would badly break. When this gets merged, then we can convert the rest of the code away from Factory::getMailer, otherwise we will have to life forever with it.

@joomdonation

joomdonation commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Sorry, I do not see the point of supporting interface if it causes loosing features of MailTemplate. I have expressed my concerns in my earlier comments, it's normal that you do not agree or I am wrong. Let's wait for feedbacks from others.

@laoneo

laoneo commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

No problem, don't expect it either.

@MacJoom

MacJoom commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

I would like to have two new tests @exlemor , @CSGoat0 - with Html tested.

@richard67

Copy link
Copy Markdown
Member

Back to pending. See previous comment.


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

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 29, 2026
@MacJoom

MacJoom commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Please test attachments too.

@laoneo

laoneo commented Apr 30, 2026

Copy link
Copy Markdown
Member Author

I'v updated the testing instructions with HTML and attachment test.

@MacJoom

MacJoom commented May 13, 2026

Copy link
Copy Markdown
Contributor

We had some changes - @CSGoat0 , @exlemor could you test again - Thank you very much.

@CSGoat0

CSGoat0 commented May 20, 2026

Copy link
Copy Markdown
Member

Hi @laoneo, Thanks for the PR.
This is the testing I've done but I am not sure about the conclusion.


Testing Results

Environment

  • Localhost using Laragon
  • Emails are received in folder: C:\laragon\bin\sendmail\output

Testing Instructions Performed

Test 1: Basic Mail Functionality

  1. Logged in to the Joomla back end
  2. Navigated to System → Global Configuration → Server tab
  3. Sent a test mail

Result BEFORE PR: Mail is sent from only the main email (first user) to the sender who sent the test email (first or second user). The sender is constant (first user). Either it's first or second user who sent the mail, the sender is always the first user's email.

Result AFTER PR: Same behavior - mail is sent successfully.

Test 2: HTML Mail with Task Scheduler

  1. Set HTML as mail format: System → Templates → Mail Templates → Options (top right)
  2. Created a new user and enabled Receive System Emails
  3. Created a GET request task:
    • URL: https://downloads.joomla.org/api/v1/latest/cms
    • Execution Rule: Manual execution
  4. In the Advanced tab of the task form:
    • Notification → Task Success → Enable
    • Selected the respective user group from the previously created user
  5. Saved the task
  6. Clicked Run Task

Result BEFORE PR:

  • The email is always sent FROM the main email (first user)
  • The email is always sent TO the main email (first user)
  • The configured user group (second user) receives NO email regardless who sent/create the task.

Result AFTER PR:

  • Exactly the same behavior as BEFORE the PR
  • No change observed

Additional Testing

  • Triggered the task from the second user's backend (after recreating the task as the second user)
  • Result: Still FROM the first user's email TO the first user's email
  • The second user never receives any notification email regardless of who creates or triggers the task

Summary

Both the normal branch and this PR have the same functionalities. The task success notification always uses the first user who created the task as both sender and receiver, ignoring the configured user group in the notification settings.
I get this attachment in the email:

{"branches":[{"branch":"Joomla! 1.0","version":"1.0.15"},{"branch":"Joomla! 1.5","version":"1.5.26"},{"branch":"Joomla! 2.5","version":"2.5.28"},{"branch":"Joomla! 3","version":"3.10.12"},{"branch":"Joomla! 4","version":"4.4.14"},{"branch":"Joomla! 5","version":"5.4.5"},{"branch":"Joomla! 6","version":"6.1.0"}]}

Questions

Is this the expected output?

If this is the intended behavior for task success notifications (always going to the main user from the main user regardless of who sent/create it, not to configured user groups), then I will mark this test as successful as there are no regressions introduced by this PR.

Note: Receiver changes in the Test 1: Basic Mail Functionality

Could you please clarify?

@laoneo

laoneo commented May 20, 2026

Copy link
Copy Markdown
Member Author

Is the behavior the same as without the patch? If yes, then it looks like you probably found a bug.

@CSGoat0

CSGoat0 commented May 20, 2026

Copy link
Copy Markdown
Member

Is the behavior the same as without the patch? If yes, then it looks like you probably found a bug.

It's the same behavior before and after the patch.
Mail system might be not accurate due to testing on local host. Maybe It needs a real host.

@laoneo

laoneo commented May 20, 2026

Copy link
Copy Markdown
Member Author

If it is the same as before the patch then it would be good if you can open an issue and mark your test as successful.

@CSGoat0

CSGoat0 commented Jun 2, 2026

Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 6d52d72

Sorry for late reply, didn't notice the message.


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

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@brianteeman

Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on b180180

tested plain, html and html with attachments


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

@MacJoom

MacJoom commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

RTC

@MacJoom MacJoom added the RTC This Pull Request is Ready To Commit label Jun 15, 2026
@MacJoom MacJoom added this to the Joomla! 6.2.0 milestone Jun 16, 2026
@MacJoom MacJoom merged commit 10efd79 into joomla:6.2-dev Jun 16, 2026
57 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 16, 2026
@MacJoom

MacJoom commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Thank you!

@laoneo laoneo deleted the mailtemplate/interface branch June 17, 2026 04:20
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.

9 participants