refactored mailer module to 2.0 style #2849
Conversation
| * | ||
| * Please see the NOTICE file distributed with this source code for further | ||
| * information regarding copyright and licensing. | ||
| */ |
There was a problem hiding this comment.
please use the new header from now on. https://github.com/zikula/core/wiki/Coding-Standards#php-files
| * | ||
| * @return bool true if successful | ||
| */ | ||
| public function sendMessage($args) |
There was a problem hiding this comment.
public api should no longer user $args. please delineate each param.
There was a problem hiding this comment.
one option here would be to create a Mail object (and companion interface) that can be type hinted into this method as an argument. Then the properties can be set and expected.
There was a problem hiding this comment.
maybe Message instead of Mail - and maybe simply type hint Switmailer's Message? and force the creation of the message before utilizing the method? just thinking out loud.
|
just did a quick scan and added a few comments. will want to look more carefully later. good work so far. 👍 |
| @@ -24,10 +24,12 @@ class LinkContainer implements LinkContainerInterface | |||
| * @var Translator | |||
| */ | |||
| private $translator; | |||
|
|
|||
There was a problem hiding this comment.
any way to make styleCI check this?
|
apparently |
|
@cmfcmf any suggestions for that problem? |
| @@ -87,8 +89,7 @@ private function getAdmin() | |||
| 'text' => $this->translator->__('Create new block'), | |||
| 'icon' => 'plus' | |||
| ]; | |||
| } | |||
| if ($this->permissionApi->hasPermission('ZikulaBlocksModule::', '::', ACCESS_ADD)) { | |||
There was a problem hiding this comment.
This line is removed because it was checked twice.
if ($this->permissionApi->hasPermission('ZikulaBlocksModule::', '::', ACCESS_ADD)) {
// A
}
if ($this->permissionApi->hasPermission('ZikulaBlocksModule::', '::', ACCESS_ADD)) {
// B
}
became
if ($this->permissionApi->hasPermission('ZikulaBlocksModule::', '::', ACCESS_ADD)) {
// A
// B
}
|
about |
|
It is supposed to happen here: https://github.com/zikula/core/blob/1.4/src/app/ZikulaKernel.php#L70 @cmfcmf we could really use your help here. |
| * @param $translator | ||
| * @param RouterInterface $router | ||
| * @param PermissionApi $permissionApi | ||
| * @param Translator $translator Translator service instance. |
|
Closed in favor of #2866 |
Description
This PR updates the mailer module to new controller, Symfony forms, Twig, etc.