User tests: Successful: Unsuccessful:
Introduces a new MailerInterface
which does contain the most common functions which are needed to send a mail. A factory which is served through the container and the corresponding aware interface and trait are also shipped with this pr.
It also fixes an inconsistency, getting a mailer with the global configuration settings. Mail::getInstance()
delivers the same object, but with a different configuration, either if Factory::getMailer()
is called before or not. Here is an example:
Factory::getConfig()->set('mailfrom', 'test@example.com');
echo Mail::getInstance()->From; // Prints an empty string
echo Factory::getMailer()->From; // Prints test@example.com
echo Mail::getInstance()->From; // Prints now test@example.com
With this pr all calls do return as From value the EMail address from the global configuration.
Send a test mail while editing the global Joomla configuration.
Test mail is sent.
Test mail is sent.
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org: joomla/Manual#116
No documentation changes for manual.joomla.org needed
Category | ⇒ | Administration com_config Libraries Unit Tests |
Status | New | ⇒ | Pending |
Labels |
Added:
?
PR-4.4-dev
|
More or less I was there at the beginning. Then I though that mostly you want to have the settings in the mailer instance from the global config.
Note that factory doesn't actually set the mailer into it's list of statics - all it's doing is populating the data from JApplication
This is not correct, it calls Mail::getInstance()
which is cached, so it basically sets that config on the cached instance. A subsequent call on Mail::getInstance()
delivers a mailer with the global settings. So actually this is a pretty inconsistent behaviour as you can get two different mailer, depending if Factory::getMailer()
is called already or not. I'v updated the pr description to illustrate the current problem.
a new method in the application
Please no more functions which do make the application instance an even bigger god object.
This is not correct, it calls Mail::getInstance() which is cached, so it basically sets that config on the cached instance. A subsequent call on Mail::getInstance() delivers a mailer with the global settings. So actually this is a pretty inconsistent behaviour as you can get two different mailer, depending if Factory::getMailer() is called already or not. I'v updated the pr description to illustrate the current problem.
This was what I was trying to say - just badly!
More or less I was there at the beginning. Then I though that mostly you want to have the settings in the mailer instance from the global config.
OK but what is the value of having that central object stored in the container? Like the container is there to do dependency management not configuration of objects
Please no more functions which do make the application instance an even bigger god object
OK Then put it in JConfiguration as the only alternative - which is where all the variables come from. I don't see any benefit in the abstraction your providing...
OK but what is the value of having that central object stored in the container? Like the container is there to do dependency management not configuration of objects
Only the factory is in the container, not the mailer instance.
Does it work without the patch? Just want to make sure that the settings itself do work.
Does it work without the patch? Just want to make sure that the settings itself do work.
Yeah, I tried without the patch first, that worked, then with patch applied and finally with patch removed. Only with applied patch no luck. This is the latest nighly build on PHP 8.2.1.
Did you try with the send test mail button?
Does it work without the patch? Just want to make sure that the settings itself do work.
It says 'Send a test mail while editing the global Joomla configuration.'
I assume that means press the send Test Mail button - that's what I did. Without the patch it says 'Test mail sent'; applying the patch, there's an error message and no mail gets sent.
@ChrisHoefliger can you delete the file /administrator/cache/autoload_psr4.php? Does it then still not work?
@ChrisHoefliger can you delete the file /administrator/cache/autoload_psr4.php? Does it then still not work?
After deleting autoload_psr4.php and then re-applying this patch, it still does not work. Without the patch, it works.
I have now re-tested this successfully on a test site, using SMTP mail on PHP 8.2
I have tested this item
Tested successfully with SMTP
I have tested this item
Tested successfully in Joomla 4.4.0-dev of 28 May in Wampserver 3.3.1 64-bit on Windows 11 using SMTP and PHP 8.1.6.
Status | Pending | ⇒ | Ready to Commit |
RTC
ParamSignatureMismatch of MailerInterface
to Joomla\CMS\Mail\Mail
libraries/src/Mail/Mail.php:134 PhanParamSignatureMismatch Declaration of function Send() : bool should be compatible with function send() : void defined in libraries/src/Mail/MailerInterface.php:34
libraries/src/Mail/Mail.php:197 PhanParamSignatureMismatch Declaration of function setSender(mixed $from, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function setSender(string $fromEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:48
libraries/src/Mail/Mail.php:233 PhanParamSignatureMismatch Declaration of function setSubject(string $subject) : \Joomla\CMS\Mail\Mail should be compatible with function setSubject(string $subject) : void defined in libraries/src/Mail/MailerInterface.php:59
libraries/src/Mail/Mail.php:249 PhanParamSignatureMismatch Declaration of function setBody(string $content) : \Joomla\CMS\Mail\Mail should be compatible with function setBody(string $content) : void defined in libraries/src/Mail/MailerInterface.php:70
libraries/src/Mail/Mail.php:332 PhanParamSignatureMismatch Declaration of function addRecipient(mixed $recipient, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addRecipient(string $recipientEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:84
libraries/src/Mail/Mail.php:349 PhanParamSignatureMismatch Declaration of function addCc(mixed $cc, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addCc(string $ccEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:98
libraries/src/Mail/Mail.php:371 PhanParamSignatureMismatch Declaration of function addBcc(mixed $bcc, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addBcc(string $bccEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:112
libraries/src/Mail/Mail.php:397 PhanParamSignatureMismatch Declaration of function addAttachment(mixed $path, mixed|string $name = '', mixed|string $encoding = 'base64', mixed|string $type = 'application/octet-stream', string $disposition = 'attachment') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addAttachment(string $data, string $name = '', string $encoding = 'base64', string $type = 'application/octet-stream') : void defined in libraries/src/Mail/MailerInterface.php:126
libraries/src/Mail/Mail.php:481 PhanParamSignatureMismatch Declaration of function addReplyTo(mixed $replyto, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addReplyTo(string $replyToEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:140
Labels |
Added:
?
|
Then phan messages can be ignored for now as the interface should be decoupled from the actual Mail implementation. Till the interface gets wider acceptance and we can change the methods in mailer we have to leave with the mismatch. PHP itself is accepting that and this is what counts for me at the moment. The target is to have an independent mailer interface without any reference to an implementation so we can exchange it on some point when there is a need for it.
Title |
|
Labels |
Added:
Feature
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-06-14 17:51:06 |
Closed_By | ⇒ | MacJoom |
I don't think this is the correct implementation for this.
Note that factory doesn't actually set the mailer into it's list of statics - all it's doing is populating the data from JApplication. Funnily enough as things stand if all applications only called
Mail::getInstance
instead ofFactory::getMailer
then they wouldn't have this data populated despite the doc blocks. Now all instances of the mailer will have this information injected if they are using the factory which is a big behaviour change.This is a much simpler case than the mvc classes. I think you just need an interface on the mail class and on top of that a new method in the application that bootstraps a mail instance that can be injected. This will also make the application dependent code much easier to unit test and from the models perspective it makes no diference anyhow whether that bootstrap of the from sender etc has actually taken place or not when the mail object is being injected.