diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..1fdc056 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,13 @@ +name: CI + +on: + pull_request: null + +jobs: + Silverstripe: + name: 'Silverstripe (bundle)' + uses: nswdpc/ci-files/.github/workflows/silverstripe_5_81.yml@1.1.0 + PHPStan: + name: 'PHPStan (analyse)' + uses: nswdpc/ci-files/.github/workflows/phpstan.silverstripe.yml@1.1.0 + needs: Silverstripe diff --git a/.gitignore b/.gitignore index 7dba068..62e4929 100644 --- a/.gitignore +++ b/.gitignore @@ -2,5 +2,6 @@ /vendor/ .DS_Store /.php-cs-fixer.cache -/codeception.yml -/tests/codeception/tests/_output/ +/public/ +/composer.lock +node_modules diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php deleted file mode 100644 index f9b7107..0000000 --- a/.php-cs-fixer.dist.php +++ /dev/null @@ -1,21 +0,0 @@ -in(__DIR__); - -$config = new PhpCsFixer\Config(); -return $config->setRules([ - '@PSR2' => true, - 'array_indentation' => true, - 'array_syntax' => ['syntax' => 'short'], - 'blank_line_after_namespace' => true, - 'blank_line_after_opening_tag' => true, - 'full_opening_tag' => true, - 'no_closing_tag' => true, - ]) - ->setIndent(" ") - ->setFinder($finder); diff --git a/_config/routes.yml b/_config/routes.yml index 58ac1dc..6a32c8d 100644 --- a/_config/routes.yml +++ b/_config/routes.yml @@ -3,7 +3,7 @@ Name: silverstripe-chimple-routes After: - '#coreroutes' --- -Silverstripe\Control\Director: +SilverStripe\Control\Director: rules: 'mc-subscribe/v1': 'NSWDPC\Chimple\Controllers\ChimpleController' --- diff --git a/composer.json b/composer.json index d732728..4251bfc 100644 --- a/composer.json +++ b/composer.json @@ -31,21 +31,27 @@ "client/static" ] }, - "support": { - "key" : "value" - }, + "repositories": [ + { + "type": "git", + "url": "https://github.com/nswdpc/ci-files.git" + } + ], "require": { "dnadesign/silverstripe-elemental": "^5", "silverstripe/framework" : "^5", "silverstripe/siteconfig" : "^5", + "silverstripe/asset-admin": "^2", "symbiote/silverstripe-queuedjobs": "^5", "drewm/mailchimp-api" : "^2.5", "symbiote/silverstripe-multivaluefield" : "^6" }, "require-dev": { + "cambis/silverstripe-rector": "^0.5.1", "phpunit/phpunit": "^9.5", - "friendsofphp/php-cs-fixer": "^3", - "vlucas/phpdotenv": "^5" + "syntro/silverstripe-phpstan": "^5", + "vlucas/phpdotenv": "^5", + "nswdpc/ci-files": "dev-main" }, "autoload": { "psr-4": { @@ -60,5 +66,15 @@ "NSWDPC\\Chimple\\Traits\\": "src/Traits/", "NSWDPC\\Chimple\\Tests\\": "tests/" } + }, + "prefer-stable": true, + "minimum-stability": "dev", + "config": { + "allow-plugins": { + "composer/installers": true, + "silverstripe/recipe-plugin": true, + "silverstripe/vendor-plugin": true, + "phpstan/extension-installer": true + } } } diff --git a/src/Controllers/ChimpleController.php b/src/Controllers/ChimpleController.php index ae59087..820fc1e 100644 --- a/src/Controllers/ChimpleController.php +++ b/src/Controllers/ChimpleController.php @@ -31,21 +31,11 @@ */ class ChimpleController extends PageController { + private static string $url_segment = 'mc-subscribe/v1'; - /** - * @var string - */ - private static $url_segment = 'mc-subscribe/v1'; - - /** - * @var bool - */ - private static $hide_generic_form = true; + private static bool $hide_generic_form = true; - /** - * @var array - */ - private static $allowed_actions = [ + private static array $allowed_actions = [ 'SubscribeForm', 'XhrSubscribeForm' ]; @@ -67,17 +57,11 @@ public function index() public function pageTitle($complete = null) { - switch($complete) { - case 'y': - return _t(__CLASS__. '.DEFAULT_TITLE_SUCCESSFUL', 'Thanks for subscribing'); - break; - case 'n': - return _t(__CLASS__. '.DEFAULT_TITLE_NOT_SUCCESSFUL', 'Sorry, there was an error'); - break; - default: - return _t(__CLASS__. '.DEFAULT_TITLE', 'Subscribe'); - break; - } + return match ($complete) { + 'y' => _t(self::class. '.DEFAULT_TITLE_SUCCESSFUL', 'Thanks for subscribing'), + 'n' => _t(self::class. '.DEFAULT_TITLE_NOT_SUCCESSFUL', 'Sorry, there was an error'), + default => _t(self::class. '.DEFAULT_TITLE', 'Subscribe'), + }; } public function Link($action = null) @@ -104,7 +88,7 @@ public function HideGenericChimpleForm() * This returns false to avoid the form being included in generic $Form templates/layouts * Use $ChimpleSubscribeForm('some-code') in templates instead */ - public function Form() + public function Form(): bool { return false; } @@ -112,7 +96,8 @@ public function Form() /** * Set a suffix form the form name */ - public function setFormNameSuffix(string $suffix = '') : self { + public function setFormNameSuffix(string $suffix = ''): self + { $suffix = trim($suffix); $this->formNameSuffix = $suffix; return $this; @@ -121,32 +106,23 @@ public function setFormNameSuffix(string $suffix = '') : self { /** * Return a suffix to use with the form name */ - public function getFormNameSuffix() : string { - if($this->formNameSuffix) { - $suffix = "_{$this->formNameSuffix}"; - } else { - $suffix = ""; - } - return $suffix; + public function getFormNameSuffix(): string + { + return $this->formNameSuffix ? "_{$this->formNameSuffix}" : ""; } /** * Get a subscription form based on parameters */ - public function getSubscriptionForm($useXhr = false) : ?SubscribeForm { - if($useXhr) { - $form = $this->XhrSubscribeForm(); - } else { - $form = $this->SubscribeForm(); - } - return $form; + public function getSubscriptionForm($useXhr = false): SubscribeForm|XhrSubscribeForm|null + { + return $useXhr ? $this->XhrSubscribeForm() : $this->SubscribeForm(); } /** * Return a subscription form if it is enabled - * @link MailchimpConfig::SubscribeForm */ - public function XhrSubscribeForm() : XhrSubscribeForm + public function XhrSubscribeForm(): ?XhrSubscribeForm { $enabled = MailchimpConfig::isEnabled(); @@ -163,7 +139,7 @@ public function XhrSubscribeForm() : XhrSubscribeForm ); $form = $this->configureForm($form); - $form->setFormAction( $this->Link('XhrSubscribeForm') ); + $form->setFormAction($this->Link('XhrSubscribeForm')); // Set a validation response callback handling for XHR form submissions $form->setValidationResponseCallback($this->getCallbackForXhrValidation()); @@ -171,14 +147,14 @@ public function XhrSubscribeForm() : XhrSubscribeForm // this form doesn't need to retain state $form->clearMessage(); - return $form; + // phpstan return type checking requirement + return $form instanceof XhrSubscribeForm ? $form : null; } /** * Return a subscription form if it is enabled - * @link MailchimpConfig::SubscribeForm */ - public function SubscribeForm() + public function SubscribeForm(): ?SubscribeForm { $enabled = MailchimpConfig::isEnabled(); @@ -195,7 +171,7 @@ public function SubscribeForm() ); $form = $this->configureForm($form); - $form->setFormAction( $this->Link('SubscribeForm') ); + $form->setFormAction($this->Link('SubscribeForm')); // Handle error validation with custom callback $form->setValidationResponseCallback($this->getCallbackForValidation($form)); @@ -206,7 +182,8 @@ public function SubscribeForm() /** * Apply common configuration to a subscription form */ - protected function configureForm(SubscribeForm $form) : SubscribeForm { + protected function configureForm(SubscribeForm $form): SubscribeForm + { // Form JS, incl XHR handling Requirements::javascript( 'nswdpc/silverstripe-chimple:client/static/js/chimple.js' @@ -228,39 +205,39 @@ protected function configureForm(SubscribeForm $form) : SubscribeForm { /** * Get fields for the form */ - protected function getFields() : FieldList { - $fields = FieldList::create( - $name = TextField::create('Name', _t(__CLASS__. '.NAME', 'Name')) - ->setAttribute('placeholder', _t(__CLASS__. '.YOUR_NAME', 'Your name')) - ->setAttribute('title', _t(__CLASS__. '.NAME', 'Name')) + protected function getFields(): FieldList + { + return FieldList::create( + $name = TextField::create('Name', _t(self::class. '.NAME', 'Name')) + ->setAttribute('placeholder', _t(self::class. '.YOUR_NAME', 'Your name')) + ->setAttribute('title', _t(self::class. '.NAME', 'Name')) ->setAttribute('required', 'required'), - $email = EmailField::create('Email', _t(__CLASS__. '.EMAIL', 'Email')) - ->setAttribute('placeholder', _t(__CLASS__. '.EMAIL_ADDRESS', 'Email address')) - ->setAttribute('title', _t(__CLASS__. '.EMAIL', 'Email')) + $email = EmailField::create('Email', _t(self::class. '.EMAIL', 'Email')) + ->setAttribute('placeholder', _t(self::class. '.EMAIL_ADDRESS', 'Email address')) + ->setAttribute('title', _t(self::class. '.EMAIL', 'Email')) ->setAttribute('required', 'required') ); - return $fields; } /** * Get actions for the form */ - protected function getActions() : FieldList { - $actions = FieldList::create( + protected function getActions(): FieldList + { + return FieldList::create( FormAction::create( 'subscribe', - _t(__CLASS__ . '.SUBSCRIBE', 'Subscribe') + _t(self::class . '.SUBSCRIBE', 'Subscribe') )->setUseButtonTag(true) ->addExtraClass('signup') ); - return $actions; } /** * Return the default validator for the form. - * @returns Validator|null */ - protected function getValidator() : ?Validator { + protected function getValidator(): ?Validator + { return RequiredFields::create(['Name','Email']); } @@ -268,13 +245,13 @@ protected function getValidator() : ?Validator { * Returns the validation callback upon errors * A response is returned only upon errors in XHR submissions * See FormRequestHandler::getValidationErrorResponse(); - * @return callable */ - protected function getCallbackForXhrValidation() : callable { - return function(ValidationResult $result) { + protected function getCallbackForXhrValidation(): callable + { + return function (ValidationResult $result): \SilverStripe\Control\HTTPResponse { // Fail, using the first message returned from the validation result $messages = $result->getMessages(); - $message = (!empty($messages[0]['message']) ? $messages[0]['message'] : ''); + $message = (empty($messages[0]['message']) ? '' : $messages[0]['message']); return $this->xhrError(400, $message); }; } @@ -282,8 +259,9 @@ protected function getCallbackForXhrValidation() : callable { /** * Callback validator for SubscribeForm, avoid redirectBack() */ - protected function getCallbackForValidation(SubscribeForm $form) : callable { - return function(ValidationResult $result) use ($form) { + protected function getCallbackForValidation(SubscribeForm $form): callable + { + return function (ValidationResult $result) use ($form): \SilverStripe\Control\HTTPResponse { // Prior to redirection, persist this result in session to re-display on redirect $form->setSessionValidationResult($result); $form->setSessionData($form->getData()); @@ -299,28 +277,32 @@ protected function getCallbackForValidation(SubscribeForm $form) : callable { /** * Handle errors, based on the request type */ - private function handleError($code, $error_message, Form $form = null) { + private function handleError($code, $error_message, Form $form = null): ?\SilverStripe\Control\HTTPResponse + { if($this->request->isAjax()) { return $this->xhrError($code, $error_message); - } else if($form) { + } elseif($form instanceof \SilverStripe\Forms\Form) { // set session error on the form $form->sessionError($error_message, ValidationResult::TYPE_ERROR); } - return; + + return null; } /** * Handle successful submissions, based on the request type */ - private function handleSuccess($code, $message, Form $form = null) { + private function handleSuccess(int $code, Form $form = null): ?\SilverStripe\Control\HTTPResponse + { $success_message = Config::inst()->get(MailchimpConfig::class, 'success_message'); if($this->request->isAjax()) { - return $this->xhrSuccess($code, $message, $success_message); - } else if($form) { + return $this->xhrSuccess($code, $success_message); + } elseif($form instanceof \SilverStripe\Forms\Form) { // set session message on the form $form->sessionMessage($success_message, ValidationResult::TYPE_GOOD); } - return; + + return null; } /** @@ -332,55 +314,50 @@ public function subscribe($data = [], Form $form = null) try { $response = null; - $code = "";// MailchimpConfig.Code + $code = strip_tags(trim($data['code'] ?? ''));// MailchimpConfig.Code $list_id = ""; + $mc_config = null; + $error_message = ""; + $email = $data['Email'] ?? ''; - if(!$form) { + if(!$form instanceof \SilverStripe\Forms\Form) { throw new RequestException("Forbidden", 403); } - $mc_config = null; - - if(empty($data['code'])) { + if($code === "") { // fail with error $error_message = _t( - __CLASS__ . '.NO_CODE', + self::class . '.NO_CODE', "No code was provided" ); $error_code = 400;// default to invalid data - } else { - - $code = strip_tags(trim($data['code'] ?: '')); $error_message = ""; $error_code = 400;// default to invalid data - $mc_config = null; - } $enabled = MailchimpConfig::isEnabled(); if(!$enabled) { $error_message = _t( - __CLASS__ . '.SUBSCRIPTIONS_NOT_AVAILABLE', + self::class . '.SUBSCRIPTIONS_NOT_AVAILABLE', "Subscriptions are not available at the moment" ); } // proceed with Email checking... if (!$error_message) { - if (empty($data['Email'])) { + if ($email === '') { // fail with error $error_message = _t( - __CLASS__ . '.NO_EMAIL_ADDRESS', + self::class . '.NO_EMAIL_ADDRESS', "No e-mail address was provided" ); - } - if (!Email::is_valid_address($data['Email'])) { + } elseif (!Email::is_valid_address($email)) { $error_message = _t( - __CLASS__ . '.INVALID_EMAIL_ADDRESS', + self::class . '.INVALID_EMAIL_ADDRESS', "Please provide a valid e-mail address, '{email}' is not valid", [ - 'email' => htmlspecialchars($data['Email']) + 'email' => htmlspecialchars((string) $data['Email']) ] ); } @@ -388,26 +365,27 @@ public function subscribe($data = [], Form $form = null) if (!$error_message) { // check code provided - if (!$code) { + if ($code === '' || $code === '0') { $error_message = _t( - __CLASS__ . ".GENERIC_ERROR_1", + self::class . ".GENERIC_ERROR_1", "Sorry, the sign-up could not be completed" ); } else { $mc_config = MailchimpConfig::getConfig('', '', $code); - if (empty($mc_config->ID)) { + if ($mc_config instanceof \NSWDPC\Chimple\Models\MailchimpConfig) { + $list_id = $mc_config->getMailchimpListId(); + } else { $error_message = _t( - __CLASS__ . ".GENERIC_ERROR_2", + self::class . ".GENERIC_ERROR_2", "Sorry, the sign-up could not be completed" ); } - $list_id = $mc_config->getMailchimpListId(); } } - if (!$list_id) { + if ($list_id === null || $list_id === '' || $list_id === '0') { $error_message = _t( - __CLASS__ . ".GENERIC_ERROR_3", + self::class . ".GENERIC_ERROR_3", "Sorry, the sign-up could not be completed" ); } @@ -426,20 +404,20 @@ public function subscribe($data = [], Form $form = null) ]) // for the Email or the MD5 of it ->filterAny([ - 'Email' => $data['Email'],// match on email address provided - 'SubscribedId' => MailchimpSubscriber::getMailchimpSubscribedId($data['Email'])// OR may not have the email anymore + 'Email' => $email,// match on email address provided + 'SubscribedId' => MailchimpSubscriber::getMailchimpSubscribedId($email)// OR may not have the email anymore ])->first(); if (empty($sub->ID)) { $sub = MailchimpSubscriber::create(); - $sub->Name = $data['Name']; - $sub->Email = $data['Email']; + $sub->Name = $data['Name'] ?? ''; + $sub->Email = $email; $sub->MailchimpListId = $list_id;//list they are subscribing to $sub->Status = MailchimpSubscriber::CHIMPLE_STATUS_NEW; - $sub->Tags = $mc_config->Tags; + $sub->Tags = $mc_config instanceof \NSWDPC\Chimple\Models\MailchimpConfig ? $mc_config->Tags : null; $sub_id = $sub->write(); if (!$sub_id) { - throw new RequestException("502", "Bad Gateway"); + throw new RequestException("Bad Gateway", 502); } } @@ -450,8 +428,8 @@ public function subscribe($data = [], Form $form = null) } // handle a successful subscription - $response = $this->handleSuccess(200, "OK", $form); - if($response && ($response instanceof HTTPResponse)) { + $response = $this->handleSuccess(200, $form); + if($response instanceof \SilverStripe\Control\HTTPResponse && ($response instanceof HTTPResponse)) { // handle responses for e.g XHR return $response; } else { @@ -466,7 +444,7 @@ public function subscribe($data = [], Form $form = null) } catch (RequestException $e) { $error_message = $e->getMessage(); $error_code = $e->getCode(); - } catch (\Exception $e) { + } catch (\Exception) { // general exceptin $error_message = Config::inst()->get(MailchimpConfig::class, 'error_message'); $error_code = 500; @@ -474,7 +452,7 @@ public function subscribe($data = [], Form $form = null) // Handle subscribe attempt failures $response = $this->handleError($error_code, $error_message, $form); - if($response && ($response instanceof HTTPResponse)) { + if($response instanceof \SilverStripe\Control\HTTPResponse && ($response instanceof HTTPResponse)) { // handle XHR error responses return $response; } else { @@ -490,26 +468,26 @@ public function subscribe($data = [], Form $form = null) /** * Return error response for XHR - * @return HTTPResponse */ - private function xhrError($code = 500, $message = "", $description = "") { - $response = new HTTPResponse(); + private function xhrError($code = 500, $message = ""): HTTPResponse + { + $response = \SilverStripe\Control\HTTPResponse::create(); $response->setStatusCode($code); $response->addHeader('Content-Type', 'application/json'); - $response->addHeader('X-Submission-OK', 0); + $response->addHeader('X-Submission-OK', '0'); $response->addHeader('X-Submission-Description', $message); return $response; } /** * Return success response for XHR - * @return HTTPResponse */ - private function xhrSuccess($code = 200, $message = "", $description = "") { - $response = new HTTPResponse(); + private function xhrSuccess(int $code = 200, $description = ""): HTTPResponse + { + $response = \SilverStripe\Control\HTTPResponse::create(); $response->setStatusCode($code); $response->addHeader('Content-Type', 'application/json'); - $response->addHeader('X-Submission-OK', 1); + $response->addHeader('X-Submission-OK', '1'); $response->addHeader('X-Submission-Description', $description); return $response; } diff --git a/src/Controllers/MailchimpAdmin.php b/src/Controllers/MailchimpAdmin.php index 8d40dc6..3f65815 100644 --- a/src/Controllers/MailchimpAdmin.php +++ b/src/Controllers/MailchimpAdmin.php @@ -13,12 +13,12 @@ */ class MailchimpAdmin extends ModelAdmin { - private static $managed_models = [ + private static array $managed_models = [ MailchimpSubscriber::class, MailchimpConfig::class ]; - private static $menu_title = 'Mailchimp'; + private static string $menu_title = 'Mailchimp'; - private static $url_segment = 'mailchimp'; + private static string $url_segment = 'mailchimp'; } diff --git a/src/Exceptions/RequestException.php b/src/Exceptions/RequestException.php index 8c43bff..7df4327 100644 --- a/src/Exceptions/RequestException.php +++ b/src/Exceptions/RequestException.php @@ -6,4 +6,6 @@ * An exception thrown when an incoming subscription request is determined to be * invalid */ -class RequestException extends \Exception {} +class RequestException extends \Exception +{ +} diff --git a/src/Extensions/DisableSecurityTokenExtension.php b/src/Extensions/DisableSecurityTokenExtension.php index 06919c0..eb4bc68 100644 --- a/src/Extensions/DisableSecurityTokenExtension.php +++ b/src/Extensions/DisableSecurityTokenExtension.php @@ -2,7 +2,7 @@ namespace NSWDPC\Chimple\Extensions; -use Silverstripe\Core\Extension; +use SilverStripe\Core\Extension; /** * This extension can be applied at the project level in situations where @@ -12,7 +12,8 @@ */ class DisableSecurityTokenExtension extends Extension { - public function updateChimpleSubscribeForm() { - $this->owner->disableSecurityToken(); + public function updateChimpleSubscribeForm() + { + $this->getOwner()->disableSecurityToken(); } } diff --git a/src/Extensions/PageExtension.php b/src/Extensions/PageExtension.php index 40f5463..2d83c05 100644 --- a/src/Extensions/PageExtension.php +++ b/src/Extensions/PageExtension.php @@ -2,6 +2,7 @@ namespace NSWDPC\Chimple\Extensions; +use NSWDPC\Chimple\Forms\SubscribeForm; use NSWDPC\Chimple\Models\MailchimpConfig; use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\Core\Config\Config; @@ -10,30 +11,30 @@ class PageExtension extends Extension { - /** * Returns a form for the configuration marked 'IsGlobal' - * @return Form|null */ - public function ChimpleGlobalSubscribeForm() { + public function ChimpleGlobalSubscribeForm(): ?SubscribeForm + { $config = MailchimpConfig::getGlobalConfig(); if ($config) { return $config->SubscribeForm(); } + return null; } /** * Returns a form based on a config code - * @return Form|null * @param string $config_code a MailchimpConfig.Code value (not an audience ID) */ - public function ChimpleSubscribeForm($config_code) + public function ChimpleSubscribeForm(string $config_code): ?SubscribeForm { $config = MailchimpConfig::getConfig('', '', $config_code); - if($config) { + if($config instanceof \NSWDPC\Chimple\Models\MailchimpConfig) { return $config->SubscribeForm(); } + return null; } } diff --git a/src/Extensions/SiteConfigExtension.php b/src/Extensions/SiteConfigExtension.php index e9d1459..64b9d3b 100644 --- a/src/Extensions/SiteConfigExtension.php +++ b/src/Extensions/SiteConfigExtension.php @@ -4,21 +4,22 @@ use NSWDPC\Chimple\Models\MailchimpConfig; -use Silverstripe\ORM\DataExtension; +use SilverStripe\ORM\DataExtension; use SilverStripe\Forms\CheckboxField; -use Silverstripe\Forms\FieldList; -use Silverstripe\Forms\DropdownField; +use SilverStripe\Forms\FieldList; +use SilverStripe\Forms\DropdownField; class SiteConfigExtension extends DataExtension { - private static $db = [ + private static array $db = [ 'MailchimpEnabled' => 'Boolean' ]; - private static $has_one = [ + private static array $has_one = [ 'MailchimpConfig' => MailchimpConfig::class // global element for configuration ]; + #[\Override] public function updateCmsFields(FieldList $fields) { $fields->addFieldsToTab( @@ -26,25 +27,24 @@ public function updateCmsFields(FieldList $fields) [ CheckboxField::create( 'MailchimpEnabled', - _t(__CLASS__. '.MAILCHIMP_ENABLED', 'Mailchimp subscriptions enabled') + _t(self::class. '.MAILCHIMP_ENABLED', 'Mailchimp subscriptions enabled') ), DropdownField::create( 'MailchimpConfigID', - _t(__CLASS__. '.DEFAULT_MAILCHIMP_CONFIG', 'Default Mailchimp configuration'), + _t(self::class. '.DEFAULT_MAILCHIMP_CONFIG', 'Default Mailchimp configuration'), MailchimpConfig::get()->map('ID', 'TitleCode') )->setEmptyString('') ] ); } + #[\Override] public function onAfterWrite() { parent::onAfterWrite(); - if($this->owner->MailchimpConfigID) { - if($config = MailchimpConfig::get()->byId($this->owner->MailchimpConfigID)) { - $config->IsGlobal = 1; - $config->write(); - } + if ($this->getOwner()->MailchimpConfigID && ($config = MailchimpConfig::get()->byId($this->getOwner()->MailchimpConfigID))) { + $config->IsGlobal = 1; + $config->write(); } } diff --git a/src/Forms/SubscribeForm.php b/src/Forms/SubscribeForm.php index 48c64b6..777fa21 100644 --- a/src/Forms/SubscribeForm.php +++ b/src/Forms/SubscribeForm.php @@ -10,8 +10,8 @@ * Subscription form subclass of {@link SilverStripe\Forms\Form} * Allows overrides of default form behaviour */ -class SubscribeForm extends Form { - +class SubscribeForm extends Form +{ use SubscriptionForm; } diff --git a/src/Forms/XhrSubscribeForm.php b/src/Forms/XhrSubscribeForm.php index d7e196a..aabfdbe 100644 --- a/src/Forms/XhrSubscribeForm.php +++ b/src/Forms/XhrSubscribeForm.php @@ -13,13 +13,12 @@ * Subscription form subclass to handle submissions via XHR * Allows overrides of default form behaviour */ -class XhrSubscribeForm extends SubscribeForm { - +class XhrSubscribeForm extends SubscribeForm +{ /** * Set to true if forms of this class will appear on a publicly cacheable page - * @var bool */ - private static $disable_security_token = false; + private static bool $disable_security_token = false; public function __construct( RequestHandler $controller = null, @@ -37,6 +36,7 @@ public function __construct( /** * @inheritdoc */ + #[\Override] protected function canBeCached() { $token = $this->getSecurityToken(); @@ -51,6 +51,7 @@ protected function canBeCached() * @inheritdoc * Add attributes */ + #[\Override] protected function getDefaultAttributes(): array { $attributes = parent::getDefaultAttributes(); diff --git a/src/Jobs/MailchimpCleanupJob.php b/src/Jobs/MailchimpCleanupJob.php index 576d3a1..610a606 100644 --- a/src/Jobs/MailchimpCleanupJob.php +++ b/src/Jobs/MailchimpCleanupJob.php @@ -19,46 +19,52 @@ class MailchimpCleanupJob extends AbstractQueuedJob implements QueuedJob { use Configurable; - private static $run_in_minutes = 30; + private static int $run_in_minutes = 30; public function __construct($minutes_ago = 30, $limit = 0, $report_only = 0) { + $minutes_ago = (int)$minutes_ago; + $limit = (int)$limit; + $report_only = (int)$report_only; $this->report_only = $report_only; - $this->minutes_ago = $minutes_ago; - $this->limit = $limit; + + $this->minutes_ago = $minutes_ago > 0 ? $minutes_ago : 30; + $this->limit = max($limit, 0); } public function getTitle() { - return sprintf( - _t( - __CLASS__ . '.TITLE', - "Mailchimp cleanup job - minutes=%s limit=%s report_only=%s" - ), - $this->minutes_ago, - $this->limit, - $this->report_only + return _t( + self::class . '.TITLE', + "Mailchimp cleanup job - minutes={minutes} limit={limit} report_only={report_only}", + [ + 'minutes' => $this->minutes_ago, + 'limit' => $this->limit, + 'report_only' => $this->report_only + ] ); } + #[\Override] public function getJobType() { return QueuedJob::QUEUED; } + #[\Override] public function setup() { $this->totalSteps = 1; } + #[\Override] public function process() { $this->processSubscriptions(); $this->isComplete = true; - return; } - private function processSubscriptions() + private function processSubscriptions(): bool { $prune_datetime = new DateTime(); $minutes = $this->minutes_ago; @@ -87,18 +93,20 @@ private function processSubscriptions() $subscriber->delete(); $success_deletes++; } + $this->addMessage("Deleted {$success_deletes} subscribers with status " . MailchimpSubscriber::CHIMPLE_STATUS_SUCCESS); } - - $this->currentStep = $this->totalSteps = $success_deletes; + $this->currentStep = $success_deletes; + $this->totalSteps = $success_deletes; // remove stale failed subscriptions older than 7 days $fail_datetime = new DateTime(); $fail_datetime->modify('-7 days'); + $failed = MailchimpSubscriber::get()->filter([ - 'Status' => MailchimpSubscriber::CHIMPLE_STATUS_FAIL, - 'Created:LessThan' => $fail_datetime->format('Y-m-d H:i:s') + 'Status' => MailchimpSubscriber::CHIMPLE_STATUS_FAIL, + 'Created:LessThan' => $fail_datetime->format('Y-m-d H:i:s') ]); if ($this->report_only) { @@ -109,11 +117,12 @@ private function processSubscriptions() $failed_subscriber->delete(); $failed_deletes++; } + $this->addMessage("Deleted {$failed_deletes} subscribers with status " . MailchimpSubscriber::CHIMPLE_STATUS_FAIL); } - - $this->currentStep = $this->totalSteps = ($success_deletes + $failed_deletes); + $this->currentStep = $success_deletes + $failed_deletes; + $this->totalSteps = $success_deletes + $failed_deletes; return true; } @@ -121,16 +130,18 @@ private function processSubscriptions() /** * Get next configured run time */ - private function getNextRunMinutes() + private function getNextRunMinutes(): int { $minutes = (int)$this->config()->get('run_in_minutes'); if ($minutes <= 2) { // min every 2 minutes $minutes = 2; } + return $minutes; } + #[\Override] public function afterComplete() { $run_datetime = new DateTime(); diff --git a/src/Jobs/MailchimpSubscribeJob.php b/src/Jobs/MailchimpSubscribeJob.php index 5c2a6a7..ceed02a 100644 --- a/src/Jobs/MailchimpSubscribeJob.php +++ b/src/Jobs/MailchimpSubscribeJob.php @@ -14,46 +14,54 @@ class MailchimpSubscribeJob extends AbstractQueuedJob implements QueuedJob { use Configurable; - private static $run_in_seconds = 60; - private static $default_limit = 100; + private static int $run_in_seconds = 60; + + private static int $default_limit = 100; public function __construct($limit = 100, $report_only = 0) { + $limit = (int)$limit; + $report_only = (int)$report_only; $this->report_only = $report_only; - $this->limit = (int)$limit; + + $this->limit = max($limit, 0); } public function getTitle() { - $title = _t( - __CLASS__ . '.TITLE', - "Batch subscribe emails to Mailchimp" + return _t( + self::class . '.TITLE', + "Batch subscribe emails to Mailchimp (limit: {limit}, report only: {report_only})", + [ + 'limit' => $this->limit, + 'report_only' => $this->report_only + ] ); - $title .= " (limit:{$this->limit})"; - $title .= ($this->report_only ? " - report only" : ""); - return $title; } + #[\Override] public function getJobType() { return QueuedJob::QUEUED; } + #[\Override] public function setup() { $this->totalSteps = 1; } - private function getTotalResults($results) + private function getTotalResults(array $results): int|float { $total = 0; - foreach ($results as $key => $count) { + foreach ($results as $count) { $total += $count; } + return $total; } - private function getTotalNonFailedResults($results) + private function getTotalNonFailedResults(array $results): int|float { $copy = $results; unset($copy[ MailchimpSubscriber::CHIMPLE_STATUS_FAIL]); @@ -63,29 +71,35 @@ private function getTotalNonFailedResults($results) /** * Process the job */ + #[\Override] public function process() { $results = MailchimpSubscriber::batch_subscribe($this->limit, $this->report_only); if ($this->report_only) { $this->addMessage("Report only: " . json_encode($results)); - $this->totalSteps = $this->currentStep = $this->getTotalResults($results); + $this->totalSteps = $this->getTotalResults($results); + $this->currentStep = $this->totalSteps; } elseif (is_array($results)) { foreach ($results as $status => $count) { $message = $status . ":" . $count; $this->addMessage($message); } + $this->totalSteps = $this->getTotalResults($results);// total including failed $this->currentStep = $this->getTotalNonFailedResults($results); } else { $this->addMessage("Failed completely - check logs!"); - $this->totalSteps = $this->currentStep = 0; + $this->totalSteps = 0; + $this->currentStep = 0; } + $this->isComplete = true; } /** * Recreate the MailchimpSubscribeJob job for the next run */ + #[\Override] public function afterComplete() { $run_datetime = new DateTime(); @@ -94,6 +108,7 @@ public function afterComplete() // min every 30s $seconds = 30; } + $run_datetime->modify("+{$seconds} seconds"); singleton(QueuedJobService::class)->queueJob( new MailchimpSubscribeJob($this->limit, $this->report_only), diff --git a/src/Models/Elements/ElementChimpleSubscribe.php b/src/Models/Elements/ElementChimpleSubscribe.php index 0c8c4e3..10744b5 100644 --- a/src/Models/Elements/ElementChimpleSubscribe.php +++ b/src/Models/Elements/ElementChimpleSubscribe.php @@ -3,6 +3,7 @@ namespace NSWDPC\Chimple\Models\Elements; use DNADesign\Elemental\Models\BaseElement; +use NSWDPC\Chimple\Forms\SubscribeForm; use NSWDPC\Chimple\Models\MailchimpConfig; use SilverStripe\Admin\LeftAndMain; use SilverStripe\Assets\Image; @@ -23,46 +24,48 @@ */ class ElementChimpleSubscribe extends BaseElement { + private static string $table_name = 'ElementChimpleSubscribe'; - private static $table_name = 'ElementChimpleSubscribe'; + private static string $singular_name = 'Mailchimp subscribe'; - private static $singular_name = 'Mailchimp subscribe'; - private static $plural_name = 'Mailchimp subscribe'; + private static string $plural_name = 'Mailchimp subscribe'; - private static $icon = 'font-icon-up-circled'; + private static string $icon = 'font-icon-up-circled'; - private static $db = [ + private static array $db = [ 'UseXHR' => 'Boolean',// whether to submit without redirect 'BeforeFormContent' => 'HTMLText', 'AfterFormContent' => 'HTMLText', 'ImageAlignment' => 'Varchar(32)', ]; - private static $defaults = [ + private static array $defaults = [ 'UseXHR' => 1 ]; /** * Has_one relationship - * @var array */ - private static $has_one = [ + private static array $has_one = [ 'MailchimpConfig' => MailchimpConfig::class, 'Image' => Image::class ]; - private static $owns = [ + private static array $owns = [ 'Image' ]; + #[\Override] public function getType() { - return _t(__CLASS__ . '.BlockType', 'Mailchimp Subscribe'); + return _t(self::class . '.BlockType', 'Mailchimp Subscribe'); } - private static $title = 'Mailchimp subscribe'; - private static $description = 'Provide a mailchimp subscription form'; + private static string $title = 'Mailchimp subscribe'; + private static string $description = 'Provide a mailchimp subscription form'; + + #[\Override] public function getCMSFields() { $fields = parent::getCMSFields(); @@ -74,7 +77,7 @@ public function getCMSFields() LiteralField::create( 'NotEnabled', '

' - . _t(__CLASS__ . 'NOT_ENABLED', 'Mailchimp is not enable in site configuration') + . _t(self::class . 'NOT_ENABLED', 'Mailchimp is not enable in site configuration') . '

' ) ); @@ -85,42 +88,43 @@ public function getCMSFields() ]); $fields->addFieldsToTab( - 'Root.Main', [ + 'Root.Main', + [ DropdownField::create( 'MailchimpConfigID', _t( - __CLASS__ . '.SELECT_CONFIGURATION', + self::class . '.SELECT_CONFIGURATION', 'Select the list configuration to use for this subscription form' ), - MailchimpConfig::get()->sort('Title ASC')->map('ID','TitleWithDetails') + MailchimpConfig::get()->sort('Title ASC')->map('ID', 'TitleWithDetails') )->setEmptyString(''), CheckboxField::create( 'UseXHR', _t( - __CLASS__ . '.USE_XHR', + self::class . '.USE_XHR', 'Submit without redirecting' ), ), HTMLEditorField::create( 'BeforeFormContent', _t( - __CLASS__ . '.BEFORE_CONTENT', + self::class . '.BEFORE_CONTENT', 'Content to show before form' ) )->setRows(6), HTMLEditorField::create( 'AfterFormContent', _t( - __CLASS__ . '.AFTER_CONTENT', + self::class . '.AFTER_CONTENT', 'Content to show after form' ) )->setRows(6), UploadField::create( 'Image', - _t(__CLASS__ . '.IMAGE', 'Image') + _t(self::class . '.IMAGE', 'Image') )->setTitle( _t( - __CLASS__ . '.ADD_IMAGE_TO_CONTENT_BLOCK', + self::class . '.ADD_IMAGE_TO_CONTENT_BLOCK', 'Add an image' ) )->setFolderName('blocks/content/' . $this->owner->ID) @@ -128,14 +132,14 @@ public function getCMSFields() ->setIsMultiUpload(false), DropdownField::create( 'ImageAlignment', - _t(__CLASS__ . '.IMAGE_ALIGNMENT', 'Image alignment'), + _t(self::class . '.IMAGE_ALIGNMENT', 'Image alignment'), [ - 'left' => _t(__CLASS__ . '.LEFT', 'Left'), - 'right' => _t(__CLASS__ . '.RIGHT', 'Right') + 'left' => _t(self::class . '.LEFT', 'Left'), + 'right' => _t(self::class . '.RIGHT', 'Right') ] )->setEmptyString('Choose an option') ->setDescription( - _t(__CLASS__ . '.IMAGE_ALIGNMENT_DESCRIPTION', 'Use of this option is dependent on the theme') + _t(self::class . '.IMAGE_ALIGNMENT_DESCRIPTION', 'Use of this option is dependent on the theme') ) ] ); @@ -146,9 +150,9 @@ public function getCMSFields() /** * Provide $SubscribeForm for template * When called in the context of the administration area, return null - * @return Form|null */ - public function getSubscribeForm() { + public function getSubscribeForm(): ?SubscribeForm + { if(Controller::curr() instanceof LeftAndMain) { return null; @@ -156,9 +160,10 @@ public function getSubscribeForm() { if($config = $this->MailchimpConfig()) { // render the form with this element's XHR setting overriding the config being used - $form = $config->SubscribeForm( $this->UseXHR == 1 ); + $form = $config->SubscribeForm($this->UseXHR == 1); return $form; } + return null; } } diff --git a/src/Models/MailchimpConfig.php b/src/Models/MailchimpConfig.php index dbb2e2f..e5f6900 100644 --- a/src/Models/MailchimpConfig.php +++ b/src/Models/MailchimpConfig.php @@ -3,6 +3,7 @@ namespace NSWDPC\Chimple\Models; use NSWDPC\Chimple\Controllers\ChimpleController; +use NSWDPC\Chimple\Forms\SubscribeForm; use NSWDPC\Chimple\Services\Logger; use SilverStripe\Core\Convert; use SilverStripe\Core\Config\Config; @@ -13,7 +14,7 @@ use SilverStripe\Forms\TextField; use SilverStripe\Forms\CheckboxField; use SilverStripe\Forms\HTMLEditor\HTMLEditorField; -use Silverstripe\ORM\DataObject; +use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\Security\PermissionProvider; @@ -30,26 +31,29 @@ */ class MailchimpConfig extends DataObject implements TemplateGlobalProvider, PermissionProvider { + private static string $list_id = ""; - private static $list_id = "";// default list (audience) ID - private static $api_key = "";// API key provided by Mailchimp + // default list (audience) ID + private static string $api_key = "";// API key provided by Mailchimp - private static $success_message = "Thank you for subscribing. You will receive an email to confirm your subscription shortly."; - private static $error_message = "Sorry, we could not subscribe that email address at the current time. Please try again later."; + private static string $success_message = "Thank you for subscribing. You will receive an email to confirm your subscription shortly."; - private static $table_name = 'ChimpleConfig'; + private static string $error_message = "Sorry, we could not subscribe that email address at the current time. Please try again later."; - private static $singular_name = 'Mailchimp Configuration'; - private static $plural_name = 'Mailchimp Configurations'; + private static string $table_name = 'ChimpleConfig'; - private static $title = "Mailchimp Subscriber Form"; - private static $description = "Configuration for a Mailchimp subscribe form"; + private static string $singular_name = 'Mailchimp Configuration'; + + private static string $plural_name = 'Mailchimp Configurations'; + + private static string $title = "Mailchimp Subscriber Form"; + + private static string $description = "Configuration for a Mailchimp subscribe form"; /** * Database fields - * @var array */ - private static $db = [ + private static array $db = [ 'Title' => 'Varchar(255)', 'Code' => 'Varchar(255)',// auto created, used to identify config 'IsGlobal' => 'Boolean', @@ -70,9 +74,8 @@ class MailchimpConfig extends DataObject implements TemplateGlobalProvider, Perm /** * Defines summary fields commonly used in table columns * as a quick overview of the data for this dataobject - * @var array */ - private static $summary_fields = [ + private static array $summary_fields = [ 'Title' => 'Title', 'Code' => 'Code', 'IsGlobal.Nice' => 'Default', @@ -81,16 +84,15 @@ class MailchimpConfig extends DataObject implements TemplateGlobalProvider, Perm 'UseXHR.Nice' => 'Submit w/o redirect' ]; - private static $indexes = [ + private static array $indexes = [ 'MailchimpListId' => true, 'Code' => ['type' => 'unique'] ]; /** * Add default values to database - * @var array */ - private static $defaults = [ + private static array $defaults = [ 'UpdateExisting' => 1,// @deprecated 'SendWelcome' => 0,// @deprecated 'ReplaceInterests' => 0,// @deprecated @@ -99,11 +101,13 @@ class MailchimpConfig extends DataObject implements TemplateGlobalProvider, Perm 'UseXHR' => 1 ]; - public function TitleCode() { + public function TitleCode(): string + { return "{$this->Title} ({$this->Code})"; } - public static function isEnabled() { + public static function isEnabled(): bool + { $site_config = SiteConfig::current_site_config(); return $site_config->MailchimpEnabled == 1; } @@ -120,35 +124,38 @@ public static function getApiKey() /** * Returns the data centre (dc) component based on the API key e.g us2 - * @return string */ - public static function getDataCentre() : string { - $dc = ''; + public static function getDataCentre(): string + { $key = self::getApiKey(); $parts = []; if($key) { - $parts = explode("-", $key); + $parts = explode("-", (string) $key); } - return !empty($parts[1]) ? $parts[1] : ''; + + return empty($parts[1]) ? '' : $parts[1]; } - public function TitleWithCode() { + public function TitleWithCode(): string + { return $this->Title . " - (code {$this->Code})"; } - public function TitleWithDetails() { + public function TitleWithDetails(): string + { $title = $this->Title; $list_id = $this->getMailchimpListId(); - $title .= " (list {$list_id})"; - return $title; + return $title . " (list {$list_id})"; } + #[\Override] public function onBeforeWrite() { parent::onBeforeWrite(); if (!$this->Code) { $this->Code = bin2hex(random_bytes(16)); } + $this->Code = Convert::raw2url($this->Code); if($this->IsGlobal == 1) { @@ -165,44 +172,50 @@ public function onBeforeWrite() /** * Return the current global config */ - public static function getGlobalConfig() { + public static function getGlobalConfig() + { return MailchimpConfig::get()->filter(['IsGlobal' => 1])->first(); } /** * If not configured in the database, return the value in yml */ - public function getMailchimpListId() + public function getMailchimpListId(): ?string { $list_id = $this->getField('MailchimpListId'); if (!$list_id) { $list_id = self::getDefaultMailchimpListId(); } + return $list_id; } - public function HasMailchimpListId() + public function HasMailchimpListId(): bool { return $this->getMailchimpListId() != ''; } - public static function getConfig($id = '', $list_id = '', $code = '') + public static function getConfig($id = '', $list_id = '', $code = ''): ?self { if ($id) { return MailchimpConfig::get()->byId($id); } + if ($list_id) { return MailchimpConfig::get()->filter('MailchimpListId', $list_id)->first(); } + if ($code) { return MailchimpConfig::get()->filter('Code', $code)->first(); } - return false; + + return null; } /** * @inheritdoc */ + #[\Override] public function getCMSFields() { $fields = parent::getCMSFields(); @@ -223,7 +236,7 @@ public function getCMSFields() 'NoApiKey', '

' . _t( - __CLASS__ . '.NO_API_KEY', + self::class . '.NO_API_KEY', 'Warning: no API key was found in the system configuration - subscriptions cannot occur until this is set.' ) . '

' @@ -237,7 +250,7 @@ public function getCMSFields() TextField::create( 'ArchiveLink', _t( - __CLASS__ . '.ARCHIVE_URL', + self::class . '.ARCHIVE_URL', 'Newsletter archive URL' ) ) @@ -247,14 +260,14 @@ public function getCMSFields() $list_id = $this->getField('MailchimpListId'); $fields->dataFieldByName('MailchimpListId') ->setDescription( - !$list_id ? - sprintf( + $list_id ? + "" : sprintf( _t( - __CLASS__ . '.NO_LIST_ID', + self::class . '.NO_LIST_ID', "No list Id is set, the default list id '%s' is being used." ), $default_list_id - ) : "" + ) ); // this is set from SiteConfig @@ -265,9 +278,9 @@ public function getCMSFields() 'IsGlobalBanner', '

' . _t( - __CLASS__. '.CONFIG_IS_GLOBAL', + self::class. '.CONFIG_IS_GLOBAL', 'This configuration is the default for this website' - ) + ) . '

' ), 'Title' @@ -279,7 +292,7 @@ public function getCMSFields() MultiValueTextField::create( 'Tags', _t( - __CLASS__ . '.TAGS_FOR_SUBSCRIPTIONS', + self::class . '.TAGS_FOR_SUBSCRIPTIONS', 'Tags assigned to subscribers' ) ) @@ -290,7 +303,7 @@ public function getCMSFields() CheckboxField::create( 'UseXHR', _t( - __CLASS__ . '.USE_XHR', + self::class . '.USE_XHR', 'Submit without redirecting' ) ), @@ -298,26 +311,28 @@ public function getCMSFields() ); $fields->addFieldsToTab( - 'Root.Main', [ - HTMLEditorField::create( - 'BeforeFormContent', - _t( - __CLASS__ . '.BEFORE_CONTENT', - 'Content to show before form' - ) - )->setRows(6), - HTMLEditorField::create( - 'AfterFormContent', - _t( - __CLASS__ . '.AFTER_CONTENT', - 'Content to show after form' - ) - )->setRows(6) - ]); + 'Root.Main', + [ + HTMLEditorField::create( + 'BeforeFormContent', + _t( + self::class . '.BEFORE_CONTENT', + 'Content to show before form' + ) + )->setRows(6), + HTMLEditorField::create( + 'AfterFormContent', + _t( + self::class . '.AFTER_CONTENT', + 'Content to show after form' + ) + )->setRows(6) + ] + ); if($heading = $fields->dataFieldByName('Heading')) { $heading->setDescription(_t( - __CLASS__ . '.HEADING_DESCRIPTON', + self::class . '.HEADING_DESCRIPTON', 'Displayed above the form' )); } @@ -328,9 +343,8 @@ public function getCMSFields() /** * Return signup link - * @return string */ - public function MailchimpLink() + public function MailchimpLink(): string { return singleton(ChimpleController::class)->Link(); } @@ -338,13 +352,14 @@ public function MailchimpLink() /** * Ensure the subscription for the global footer is added */ + #[\Override] public function requireDefaultRecords() { $config = MailchimpConfig::get()->filter(['IsGlobal' => 1])->first(); if (empty($config->ID)) { $config = MailchimpConfig::create([ - 'Title' => _t(__CLASS__ . '.DEFAULT_CONFIG_TITLE', 'Default Configuration'), - 'Heading' => _t(__CLASS__ . '.DEFAULT_CONFIG_HEADER', 'Subscribe'), + 'Title' => _t(self::class . '.DEFAULT_CONFIG_TITLE', 'Default Configuration'), + 'Heading' => _t(self::class . '.DEFAULT_CONFIG_HEADER', 'Subscribe'), 'IsGlobal' => 1, 'MailchimpListId' => null ]); @@ -353,6 +368,7 @@ public function requireDefaultRecords() } else { $config_id = $config->ID; } + if ($config_id) { $site_config = SiteConfig::current_site_config(); if (!empty($site_config->ID) && empty($site_config->MailchimpConfigID)) { @@ -366,9 +382,8 @@ public function requireDefaultRecords() /** * Use the form provided by the controller * @param bool $force_xhr whether to submit in place via XHR or not, the default is to let the config decide - * @return Form */ - public function SubscribeForm($force_xhr = null) + public function SubscribeForm($force_xhr = null): ?SubscribeForm { // No form available if not enabled $enabled = self::isEnabled(); @@ -396,6 +411,7 @@ public function SubscribeForm($force_xhr = null) if ($this->Heading) { $form->setLegend($this->Heading); } + $form->addExtraClass('form-subscribe'); return $form; } @@ -405,39 +421,43 @@ public function SubscribeForm($force_xhr = null) /** * Return alerts for the form - * @return string */ - public function Alerts() + public function Alerts(): string { return '' . '' . '';// info added by JS } + #[\Override] public function canView($member = null) { return Permission::checkMember($member, 'MAILCHIMP_CONFIG_VIEW'); } + #[\Override] public function canCreate($member = null, $context = []) { return Permission::checkMember($member, 'MAILCHIMP_CONFIG_CREATE'); } + #[\Override] public function canEdit($member = null) { return Permission::checkMember($member, 'MAILCHIMP_CONFIG_EDIT'); } + #[\Override] public function canDelete($member = null) { return Permission::checkMember($member, 'MAILCHIMP_CONFIG_DELETE'); } + #[\Override] public function providePermissions() { return [ @@ -462,14 +482,14 @@ public function providePermissions() /** * Render this record using a template - * @return DBHTMLText|null */ - public function forTemplate($force_xhr = null) + public function forTemplate($force_xhr = null): ?DBHTMLText { $form = $this->SubscribeForm($force_xhr); - if($form) { - return $this->customise(['Form'=>$form])->renderWith( self::class ); + if($form instanceof \NSWDPC\Chimple\Forms\SubscribeForm) { + return $this->customise(['Form' => $form])->renderWith(self::class); } + return null; } @@ -479,45 +499,47 @@ public function forTemplate($force_xhr = null) * The 2nd parameter is a 1 or 0 representing whether to handle the submission via XHR * This is called from a template calling $ChimpleSubscribeForm('code'[,0|1]) * @param array $args - * @return DBHTMLText|null */ - public static function get_chimple_subscribe_form(...$args) + public static function get_chimple_subscribe_form(...$args): ?DBHTMLText { - $code = isset($args[0]) ? $args[0] : ''; + $code = $args[0] ?? ''; if ($code) { $config = self::getConfig('', '', $code); - if ($config) { + if ($config instanceof \NSWDPC\Chimple\Models\MailchimpConfig) { // default to let the config decide $force_xhr = null; if(isset($args[1])) { if($args[1] === '0') { // string '0' passed in as an arg in the template $force_xhr = false; - } else if($args[1] === '1') { + } elseif($args[1] === '1') { // string '1' passed in as an arg in the template $force_xhr = true; } } + return $config->forTemplate($force_xhr); } } + return null; } /** * Get the subscribe form for the current global config * This is called from a template calling $ChimpleSubscribeForm('code') - * @return DBHTMLText|null */ - public static function get_chimple_global_subscribe_form() + public static function get_chimple_global_subscribe_form(): ?DBHTMLText { $config = self::getGlobalConfig(); if ($config) { return $config->forTemplate(); } + return null; } + #[\Override] public static function get_template_global_variables() { return [ diff --git a/src/Models/MailchimpSubscriber.php b/src/Models/MailchimpSubscriber.php index 42cc371..cf58b62 100644 --- a/src/Models/MailchimpSubscriber.php +++ b/src/Models/MailchimpSubscriber.php @@ -18,72 +18,70 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider { - const CHIMPLE_STATUS_UNKNOWN = ''; - const CHIMPLE_STATUS_NEW = 'NEW'; - const CHIMPLE_STATUS_PROCESSING = 'PROCESSING'; - const CHIMPLE_STATUS_BATCHED = 'BATCHED'; - const CHIMPLE_STATUS_SUCCESS = 'SUCCESS'; - const CHIMPLE_STATUS_FAIL = 'FAIL'; + public const CHIMPLE_STATUS_UNKNOWN = ''; - const MAILCHIMP_SUBSCRIBER_PENDING = 'pending'; - const MAILCHIMP_SUBSCRIBER_SUBSCRIBED = 'subscribed'; - const MAILCHIMP_SUBSCRIBER_UNSUBSCRIBED = 'unsubscribed'; - const MAILCHIMP_SUBSCRIBER_CLEANED = 'cleaned'; + public const CHIMPLE_STATUS_NEW = 'NEW'; - const MAILCHIMP_TAG_INACTIVE = 'inactive'; - const MAILCHIMP_TAG_ACTIVE = 'active'; + public const CHIMPLE_STATUS_PROCESSING = 'PROCESSING'; - const MAILCHIMP_EMAIL_TYPE_HTML = 'html'; - const MAILCHIMP_EMAIL_TYPE_TEXT = 'text'; + public const CHIMPLE_STATUS_BATCHED = 'BATCHED'; - /** - * @var string - */ - private static $table_name = 'ChimpleSubscriber'; + public const CHIMPLE_STATUS_SUCCESS = 'SUCCESS'; + + public const CHIMPLE_STATUS_FAIL = 'FAIL'; + + public const MAILCHIMP_SUBSCRIBER_PENDING = 'pending'; + + public const MAILCHIMP_SUBSCRIBER_SUBSCRIBED = 'subscribed'; + + public const MAILCHIMP_SUBSCRIBER_UNSUBSCRIBED = 'unsubscribed'; + + public const MAILCHIMP_SUBSCRIBER_CLEANED = 'cleaned'; + + public const MAILCHIMP_TAG_INACTIVE = 'inactive'; + + public const MAILCHIMP_TAG_ACTIVE = 'active'; + + public const MAILCHIMP_EMAIL_TYPE_HTML = 'html'; + + public const MAILCHIMP_EMAIL_TYPE_TEXT = 'text'; + + private static string $table_name = 'ChimpleSubscriber'; /** * Singular name for CMS - * @var string */ - private static $singular_name = 'Mailchimp Subscriber'; + private static string $singular_name = 'Mailchimp Subscriber'; /** * Plural name for CMS - * @var string */ - private static $plural_name = 'Mailchimp Subscribers'; + private static string $plural_name = 'Mailchimp Subscribers'; /** * Default sort ordering - * @var array */ - private static $default_sort = ['Created' => 'DESC']; + private static array $default_sort = ['Created' => 'DESC']; /** * Default chr for obfuscation - * @var array */ - private static $obfuscation_chr = "•"; + private static string $obfuscation_chr = "•"; /** * Email type, defaults to 'html', other value is 'text' - * @var string */ - private static $email_type = self::MAILCHIMP_EMAIL_TYPE_HTML; + private static string $email_type = self::MAILCHIMP_EMAIL_TYPE_HTML; /** * Remove tags that do not exist in the tags list when a subscriber * attempts to update their subscription * This is a potentially destructive action as it will remove tags added to * a subscriber via other means - * @var string */ - private static $remove_subscriber_tags = false; + private static bool $remove_subscriber_tags = false; - /** - * @var array - */ - private static $db = [ + private static array $db = [ 'Name' => 'Varchar(255)', 'Surname' => 'Varchar(255)', 'Email' => 'Varchar(255)', @@ -107,12 +105,12 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider 'Tags' => 'MultiValueField' ]; - private static $sync_fields = [ + private static array $sync_fields = [ 'Name' => 'FNAME', 'Surname' => 'LNAME' ]; - private static $indexes = [ + private static array $indexes = [ 'Status' => true, 'Email' => true, 'Name' => true, @@ -123,9 +121,8 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider /** * Add default values to database - * @var array */ - private static $defaults = [ + private static array $defaults = [ 'Status' => self::CHIMPLE_STATUS_NEW, 'UpdateExisting' => 1,// @deprecated 'SendWelcome' => 0,// @deprecated @@ -136,9 +133,8 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider /** * Defines summary fields commonly used in table columns * as a quick overview of the data for this dataobject - * @var array */ - private static $summary_fields = [ + private static array $summary_fields = [ 'Created.Nice' => 'Created', 'Name' => 'Name', 'Surname' => 'Surname', @@ -151,9 +147,8 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider /** * Defines a default list of filters for the search context - * @var array */ - private static $searchable_fields = [ + private static array $searchable_fields = [ 'Name' => 'PartialMatchFilter', 'Surname' => 'PartialMatchFilter', 'Email' => 'PartialMatchFilter', @@ -165,25 +160,26 @@ class MailchimpSubscriber extends DataObject implements PermissionProvider * @var MailchimpApiClient * The Mailchimp API client instance */ - protected static $mailchimp = null; + protected static $mailchimp; /** * List of current subscriber tags - * @var array|null */ - private $_cache_tags = null; + private ?array $_cache_tags = null; /** * Return whether subscription was success */ - public function getSuccessful() + public function getSuccessful(): bool { return $this->Status == self::CHIMPLE_STATUS_SUCCESS; } + /** * @inheritdoc */ + #[\Override] public function getCMSFields() { $fields = parent::getCMSFields(); @@ -204,7 +200,7 @@ public function getCMSFields() $fields->dataFieldByName('LastError') ->setDescription( _t( - __CLASS__. '.IF_ERROR_OCCURRED', + self::class. '.IF_ERROR_OCCURRED', "If a subscription error occurred, this will display the last error returned by Mailchimp and can help determine the cause of the error" ) ) @@ -214,13 +210,13 @@ public function getCMSFields() if($this->exists()) { $status_field = DropdownField::create( 'Status', - _t(__CLASS__ . '.STATUS', 'Status'), + _t(self::class . '.STATUS', 'Status'), [ self::CHIMPLE_STATUS_UNKNOWN => '', - self::CHIMPLE_STATUS_NEW => _t(__CLASS__ . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), - self::CHIMPLE_STATUS_PROCESSING => _t(__CLASS__ . '.STATUS_PROCESSING', 'PROCESSING (the subscriber is in the process of being subscribed)'), - self::CHIMPLE_STATUS_SUCCESS => _t(__CLASS__ . '.STATUS_SUCCESS', 'SUCCESS (the subscriber was subscribed)'), - self::CHIMPLE_STATUS_FAIL => _t(__CLASS__ . '.STATUS_FAIL', 'FAIL (the subscriber could not be subscribed - check the \'Last Error\' value)') + self::CHIMPLE_STATUS_NEW => _t(self::class . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), + self::CHIMPLE_STATUS_PROCESSING => _t(self::class . '.STATUS_PROCESSING', 'PROCESSING (the subscriber is in the process of being subscribed)'), + self::CHIMPLE_STATUS_SUCCESS => _t(self::class . '.STATUS_SUCCESS', 'SUCCESS (the subscriber was subscribed)'), + self::CHIMPLE_STATUS_FAIL => _t(self::class . '.STATUS_FAIL', "FAIL (the subscriber could not be subscribed - check the 'Last Error' value)") ] ); @@ -230,31 +226,31 @@ public function getCMSFields() && !empty($this->Status)) { // these status are readonly in CMS fields $status_field = $status_field->performReadonlyTransformation(); - } else if($this->Status == self::CHIMPLE_STATUS_PROCESSING) { + } elseif($this->Status == self::CHIMPLE_STATUS_PROCESSING) { // stuck processing - can reset to new $status_field->setDescription( _t( - __CLASS__ . '.PROCESSING_RESET_NEW_STATUS', + self::class . '.PROCESSING_RESET_NEW_STATUS', "If this attempt is stuck, reset to 'New' for another attempt" ) ); // can retain failed or set to new for retry $status_field->setSource([ - self::CHIMPLE_STATUS_NEW => _t(__CLASS__ . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), - self::CHIMPLE_STATUS_PROCESSING => _t(__CLASS__ . '.STATUS_PROCESSING', 'PROCESSING (the subscriber is in the process of being subscribed)'), + self::CHIMPLE_STATUS_NEW => _t(self::class . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), + self::CHIMPLE_STATUS_PROCESSING => _t(self::class . '.STATUS_PROCESSING', 'PROCESSING (the subscriber is in the process of being subscribed)'), ]); - } else if($this->Status == self::CHIMPLE_STATUS_FAIL) { + } elseif($this->Status == self::CHIMPLE_STATUS_FAIL) { // handling when failed $status_field->setDescription( _t( - __CLASS__ . '.FAIL_RESET_NEW_STATUS', + self::class . '.FAIL_RESET_NEW_STATUS', "Reset this value to 'New' to retry a failed subscription attempt" ) ); // can retain failed or set to new for retry $status_field->setSource([ - self::CHIMPLE_STATUS_NEW => _t(__CLASS__ . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), - self::CHIMPLE_STATUS_FAIL => _t(__CLASS__ . '.STATUS_FAIL', 'FAIL (the subscriber could not be subscribed - check the \'Last Error\' value)') + self::CHIMPLE_STATUS_NEW => _t(self::class . '.STATUS_NEW', 'NEW (the subscriber has not yet been subscribed)'), + self::CHIMPLE_STATUS_FAIL => _t(self::class . '.STATUS_FAIL', "FAIL (the subscriber could not be subscribed - check the 'Last Error' value)") ]); } @@ -275,8 +271,8 @@ public function getCMSFields() 'StatusForNew', '

' . _t( - __CLASS__ . '.STATUS_NEW_MESSAGE', - 'This subscription attempt record will be given status of \'New\' and it will enter the pending subscription queue upon save' + self::class . '.STATUS_NEW_MESSAGE', + "This subscription attempt record will be given status of 'New' and it will enter the pending subscription queue upon save" ) . '

' ), @@ -286,9 +282,9 @@ public function getCMSFields() $tags = $this->getCurrentSubscriberTags(); $tag_field_description = ""; - if(!empty($tags)) { + if($tags !== []) { $tag_field_description = _t( - __CLASS__ . '.TAGS_FIELD_DESCRIPTION', + self::class . '.TAGS_FIELD_DESCRIPTION', 'The current tags for this subscriber are {tags}
Tags not in the tag update list will be removed. New tags will be added.', [ 'tags' => implode(", ", $tags) @@ -302,19 +298,19 @@ public function getCMSFields() KeyValueField::create( 'MergeFields', _t( - __CLASS__ . '.MERGE_FIELDS', + self::class . '.MERGE_FIELDS', 'Merge fields for this subscription attempt' ) )->setDescription( _t( - __CLASS__ . '.MERGE_FIELDS_DESCRIPTION', + self::class . '.MERGE_FIELDS_DESCRIPTION', 'Keys are merge tag names, values are the values for this particular subscriber' ) ), MultiValueTextField::create( 'Tags', _t( - __CLASS__ . '.TAGS_FIELD', + self::class . '.TAGS_FIELD', 'Tag update list' ) )->setDescription( @@ -324,7 +320,7 @@ public function getCMSFields() ); // get profile link - $dc = MailChimpConfig::getDataCentre(); + $dc = MailchimpConfig::getDataCentre(); if($dc && $this->SubscribedWebId) { $subscriber_profile_link = "https://{$dc}.admin.mailchimp.com/lists/members/view?id={$this->SubscribedWebId}"; } else { @@ -332,10 +328,10 @@ public function getCMSFields() } $readonly_fields = [ - 'MailchimpListId' => _t(__CLASS__ . '.SUBSCRIBER_LIST_ID', "The Mailchimp List (audience) Id for this subscription record"), - 'SubscribedUniqueEmailId' => _t(__CLASS__ . '.SUBSCRIBER_UNIQUE_EMAIL_ID', "An identifier for the address across all of Mailchimp."), - 'SubscribedWebId' => _t(__CLASS__ . '.SUBSCRIBER_WEB_ID', "Member profile page: {link}", ['link' => $subscriber_profile_link]), - 'SubscribedId' => _t(__CLASS__ . '.SUBSCRIBER_ID', "The MD5 hash of the lowercase version of the list member's email address.") + 'MailchimpListId' => _t(self::class . '.SUBSCRIBER_LIST_ID', "The Mailchimp List (audience) Id for this subscription record"), + 'SubscribedUniqueEmailId' => _t(self::class . '.SUBSCRIBER_UNIQUE_EMAIL_ID', "An identifier for the address across all of Mailchimp."), + 'SubscribedWebId' => _t(self::class . '.SUBSCRIBER_WEB_ID', "Member profile page: {link}", ['link' => $subscriber_profile_link]), + 'SubscribedId' => _t(self::class . '.SUBSCRIBER_ID', "The MD5 hash of the lowercase version of the list member's email address.") ]; foreach($readonly_fields as $readonly_field => $description) { @@ -345,6 +341,7 @@ public function getCMSFields() if($description) { $field->setDescription($description); } + $fields->addFieldToTab( 'Root.Main', $field @@ -355,6 +352,7 @@ public function getCMSFields() return $fields; } + #[\Override] public function onBeforeWrite() { parent::onBeforeWrite(); @@ -368,7 +366,7 @@ public function onBeforeWrite() $this->Surname = $this->getSurnameFromName(); // reset name $parts = explode(" ", $this->Name, 2); - $this->Name = isset($parts[0]) ? $parts[0] : ''; + $this->Name = $parts[0] ?? ''; } } @@ -377,10 +375,10 @@ public function onBeforeWrite() * Given some meta data, update the MergeFields for this subscriber * The parameter can contain values submitted via a form * By default MergeFields doesn't allow HTML tags as keys or as values - * @param array $meta */ - public function updateMergeFields(array $meta) { - if(empty($meta)) { + public function updateMergeFields(array $meta) + { + if($meta === []) { return; } @@ -390,15 +388,17 @@ public function updateMergeFields(array $meta) { // ignore values that cannot be saved continue; } - $key = strtoupper( trim( strip_tags($k) ) ); - $value = trim( strip_tags($v) ); + + $key = strtoupper(trim(strip_tags($k))); + $value = trim(strip_tags($v)); $data[ $key ] = $value; } + $this->MergeFields = $data; $this->write(); } - public function HasLastError() + public function HasLastError(): string { return trim($this->LastError ?? '') !== '' ? "yes" : "no"; } @@ -406,27 +406,31 @@ public function HasLastError() /** * Attempt to get a surname from the name */ - public function getSurnameFromName() + public function getSurnameFromName(): ?string { $parts = explode(" ", $this->Name, 2); - if (!empty($parts[1])) { + if (isset($parts[1]) && ($parts[1] !== '' && $parts[1] !== '0')) { return $parts[1]; } + + return null; } /** * Get the API client - * @return MailchimpApiClient */ - public static function api() : MailchimpApiClient { + public static function api(): MailchimpApiClient + { // already set up.. if(self::$mailchimp instanceof MailchimpApiClient) { return self::$mailchimp; } + $api_key = MailchimpConfig::getApiKey(); if (!$api_key) { throw new Exception("No Mailchimp API Key configured!"); } + self::$mailchimp = new MailchimpApiClient($api_key); return self::$mailchimp; } @@ -434,7 +438,8 @@ public static function api() : MailchimpApiClient { /** * @deprecated use self::api() instead */ - public function getMailchimp() { + public function getMailchimp(): \DrewM\MailChimp\MailChimp + { return self::api(); } @@ -447,26 +452,28 @@ public function getMailchimpListId() if (!$list_id) { $list_id = MailchimpConfig::getDefaultMailchimpListId(); } + return $list_id; } /** * Applies merge fields prior to subscription attempt - * @return array */ - protected function applyMergeFields() { + protected function applyMergeFields(): array + { $merge_fields = []; // get subscriber meta data $meta = $this->MergeFields->getValue(); - if(is_array($meta) && !empty($meta)) { + if(is_array($meta) && $meta !== []) { foreach($meta as $k => $v) { if($v === '') { // do not set empty values, MailChimp does not like this continue; } + $k = strtoupper(trim($k)); - $merge_fields[$k] = trim($v); + $merge_fields[$k] = trim((string) $v); } } @@ -478,18 +485,17 @@ protected function applyMergeFields() { // ignore non-existent fields continue; } + $value = $this->getField($field); - if(!is_string($value)) { - $value = ''; - } else { - $value = trim($value); - } + $value = is_string($value) ? trim($value) : ''; + if ($value === '') { // do not set empty values, MailChimp does not like this // "The resource submitted could not be validated" continue; } - $tag = strtoupper(trim($tag)); + + $tag = strtoupper(trim((string) $tag)); $merge_fields[$tag] = $value; } @@ -498,9 +504,9 @@ protected function applyMergeFields() { /** * Get the default subscription record data for adding/updating member in list - * @return array */ - public function getSubscribeRecord() : array { + public function getSubscribeRecord(): array + { // merge fields to send $merge_fields = $this->applyMergeFields(); // ensure sane email type either html or text, default html if invalid @@ -508,18 +514,19 @@ public function getSubscribeRecord() : array { if(!$email_type || $email_type != self::MAILCHIMP_EMAIL_TYPE_HTML || $email_type != self::MAILCHIMP_EMAIL_TYPE_TEXT) { $email_type = self::MAILCHIMP_EMAIL_TYPE_HTML; } - $params = [ + + return [ 'email_address' => $this->Email, 'email_type' => $email_type, 'merge_fields' => $merge_fields ]; - return $params; } /** * Return tags for this subscriber */ - public function getSubscriberTags() { + public function getSubscriberTags(): array + { $tags = $this->Tags->getValue(); if(!is_array($tags)) { return []; @@ -530,37 +537,39 @@ public function getSubscriberTags() { /** * Called after a successful subscription, obfuscates email, name and surname - * @return void */ - private function obfuscate() { - $obfuscate = function($in) { + private function obfuscate() + { + $obfuscate = function (string $in): string { $length = strlen($in); if($length == 0) { return ""; } + $chr = $this->config()->get('obfuscation_chr'); if($chr === "") { // if no chr configured, do not obfuscate (e.g not require by project) return $in; } + $sub_length = $length - 2; if($sub_length <= 0) { return str_repeat($chr, $length); } - $replaced = substr_replace($in, str_repeat($chr, $sub_length), 1, $sub_length); - return $replaced; + + return substr_replace($in, str_repeat($chr, $sub_length), 1, $sub_length); }; - $this->Email = $obfuscate($this->Email); - $this->Name = $obfuscate($this->Name); - $this->Surname = $obfuscate($this->Surname); + $this->Email = $obfuscate($this->Email ?? ''); + $this->Name = $obfuscate($this->Name ?? ''); + $this->Surname = $obfuscate($this->Surname ?? ''); } /** * Get the hash that is used as the MC subscribed Id value - * @return string */ - public static function getMailchimpSubscribedId($email) { - if(!is_string($email) || !$email) { + public static function getMailchimpSubscribedId($email): string + { + if(!is_string($email) || ($email === '' || $email === '0')) { return ''; } else { return MailchimpApiClient::subscriberHash($email); @@ -572,31 +581,31 @@ public static function getMailchimpSubscribedId($email) { * @param string $list_id the Audience ID * @param string $email an email address, this is hashed using the MC hashing strategy * @param string $api_key @deprecated - * @return boolean|array */ - public static function checkExistsInList(string $list_id, string $email, string $api_key = '') { + public static function checkExistsInList(string $list_id, string $email, string $api_key = ''): array|false + { // sanity check on input - if(!$email) { + if($email === '' || $email === '0') { throw new \Exception( _t( - __CLASS__ . ".EMAIL_NOT_PROVIDED", + self::class . ".EMAIL_NOT_PROVIDED", "Please provide an email address" ) ); } - if(!$list_id) { + if($list_id === '' || $list_id === '0') { throw new \Exception( _t( - __CLASS__ . ".AUDIENCE_NOT_PROVIDED", + self::class . ".AUDIENCE_NOT_PROVIDED", "Please provide a Mailchimp audience/list ID" ) ); } // attempt to get the subscriber - if( $hash = self::getMailchimpSubscribedId($email) ) { + if(($hash = self::getMailchimpSubscribedId($email)) !== '' && ($hash = self::getMailchimpSubscribedId($email)) !== '0') { $result = self::api()->get( "lists/{$list_id}/members/{$hash}" ); @@ -612,9 +621,8 @@ public static function checkExistsInList(string $list_id, string $email, string /** * Subscribe *this* particular record - * @return bool */ - public function subscribe() + public function subscribe(): bool { try { @@ -635,7 +643,7 @@ public function subscribe() $result = false; - if(!$existing) { + if($existing === [] || $existing === false) { $operation_path = "lists/{$list_id}/members"; $params = $this->getSubscribeRecord(); // add tags @@ -673,16 +681,17 @@ public function subscribe() $this->write(); return true; } elseif (!empty($result['status'])) { - $error_detail = isset($result['detail']) ? $result['detail'] : ''; + $error_detail = $result['detail'] ?? ''; $error_status = $result['status']; $error_title = $result['title']; $errors = "{$error_status}|{$error_title}|{$error_detail}"; } else { $errors = "Unhandled error for email: {$this->Email}"; } + throw new Exception(trim($errors)); - } catch (Exception $e) { - $last_error = $e->getMessage(); + } catch (Exception $exception) { + $last_error = $exception->getMessage(); } // record failure @@ -700,9 +709,9 @@ public function subscribe() * Return all current tags for a subscriber and handle pagination at the time of the API call * @param bool $force whether to get the tags from the remote or use previously retrieved tags * @param int $count the number of records to return per request - * @return array an array of values, each value is a string tag */ - private function getCurrentSubscriberTags(bool $force = false, int $count = 10) : array { + private function getCurrentSubscriberTags(bool $force = false, int $count = 10): array + { // if already retrieved, if(is_array($this->_cache_tags) && !$force) { @@ -712,7 +721,7 @@ private function getCurrentSubscriberTags(bool $force = false, int $count = 10) $list_id = $this->MailchimpListId; $subscriber_hash = self::getMailchimpSubscribedId($this->Email ?? ''); - if(!$list_id || !$subscriber_hash) { + if(!$list_id || ($subscriber_hash === '' || $subscriber_hash === '0')) { $this->_cache_tags = null; return []; } @@ -722,12 +731,12 @@ private function getCurrentSubscriberTags(bool $force = false, int $count = 10) $operation_path = "lists/{$list_id}/members/{$subscriber_hash}/tags/?count={$count}&offset={$offset}"; $result = self::api()->get($operation_path); - $total = isset($result['total_items']) ? $result['total_items'] : 0; + $total = $result['total_items'] ?? 0; // initial set of tags $tags = isset($result['tags']) && is_array($result['tags']) ? $result['tags'] : []; // populate the list of tags - $walker = function($value, $key) use (&$list) { + $walker = function (array $value, $key) use (&$list): void { $list[] = $value['name']; }; array_walk($tags, $walker); @@ -754,7 +763,8 @@ private function getCurrentSubscriberTags(bool $force = false, int $count = 10) /** * Modify this subscriber's tags based on their current tags */ - protected function modifySubscriberTags() : bool { + protected function modifySubscriberTags(): bool + { $current = $this->getCurrentSubscriberTags(); $tags_for_update = $this->getSubscriberTags(); @@ -764,7 +774,7 @@ protected function modifySubscriberTags() : bool { // if enabled: remove tags that do not exist in the modification list if($this->config()->get('remove_subscriber_tags')) { - $inactive = array_diff($current,$tags_for_update); + $inactive = array_diff($current, $tags_for_update); // Mark removed tags as inactive foreach($inactive as $tag) { $params['tags'][] = [ @@ -775,7 +785,7 @@ protected function modifySubscriberTags() : bool { } // Retain active tags that are in both lists - $retained = array_intersect($current,$tags_for_update); + $retained = array_intersect($current, $tags_for_update); foreach($retained as $tag) { $params['tags'][] = [ 'name' => $tag, @@ -799,7 +809,7 @@ protected function modifySubscriberTags() : bool { $operation_path = "/lists/{$list_id}/members/{$subscriber_hash}/tags"; // submit payload to API - $result = self::api()->post( + self::api()->post( $operation_path, $params ); @@ -815,9 +825,8 @@ protected function modifySubscriberTags() : bool { /** * Batch subscribe via API - hit from MailchimpSubscribeJob * Retrieve all subscribers marked new and attempt to subscribe them - * @return array */ - public static function batch_subscribe($limit = 100, $report_only = false) + public static function batch_subscribe($limit = 100, $report_only = false): array { $results = []; try { @@ -827,11 +836,13 @@ public static function batch_subscribe($limit = 100, $report_only = false) if ($limit) { $subscribers = $subscribers->limit($limit); } + if ($report_only) { $results[ self::CHIMPLE_STATUS_PROCESSING ] = $subscribers->count(); foreach ($subscribers as $subscriber) { Logger::log("REPORT_ONLY: would subscribe #{$subscriber->ID} to list {$subscriber->MailchimpListId}", 'DEBUG'); } + return $results; } @@ -847,22 +858,26 @@ public static function batch_subscribe($limit = 100, $report_only = false) // set status to 0 $results[ $subscriber->Status ] = 0; } + // increment this status $results[ $subscriber->Status ]++; } + return $results; - } catch (Exception $e) { - Logger::log("FAIL: could not batch_subscribe, error=" . $e->getMessage(), 'NOTICE'); + } catch (Exception $exception) { + Logger::log("FAIL: could not batch_subscribe, error=" . $exception->getMessage(), 'NOTICE'); } - return false; + return []; } + #[\Override] public function canView($member = null) { return Permission::checkMember($member, 'MAILCHIMP_SUBSCRIBER_VIEW'); } + #[\Override] public function canEdit($member = null) { return Permission::checkMember($member, 'MAILCHIMP_SUBSCRIBER_EDIT'); @@ -871,29 +886,32 @@ public function canEdit($member = null) /** * Only admin can delete subscribers */ + #[\Override] public function canDelete($member = null) { return Permission::checkMember($member, 'ADMIN'); } + #[\Override] public function canCreate($member = null, $context = []) { return Permission::checkMember($member, 'MAILCHIMP_SUBSCRIBER_CREATE'); } + #[\Override] public function providePermissions() { return [ 'MAILCHIMP_SUBSCRIBER_VIEW' => [ - 'name' => _t(__CLASS__ . '.MAILCHIMP_SUBSCRIBER_VIEW', 'View Mailchimp Subscribers'), + 'name' => _t(self::class . '.MAILCHIMP_SUBSCRIBER_VIEW', 'View Mailchimp Subscribers'), 'category' => 'Mailchimp', ], 'MAILCHIMP_SUBSCRIBER_EDIT' => [ - 'name' => _t(__CLASS__ . '.MAILCHIMP_SUBSCRIBER_EDIT', 'Edit Mailchimp Subscribers'), + 'name' => _t(self::class . '.MAILCHIMP_SUBSCRIBER_EDIT', 'Edit Mailchimp Subscribers'), 'category' => 'Mailchimp', ], 'MAILCHIMP_SUBSCRIBER_CREATE' => [ - 'name' => _t(__CLASS__ . '.MAILCHIMP_SUBSCRIBER_CREATE', 'Create Mailchimp Subscribers'), + 'name' => _t(self::class . '.MAILCHIMP_SUBSCRIBER_CREATE', 'Create Mailchimp Subscribers'), 'category' => 'Mailchimp', ] ]; diff --git a/src/Traits/SubscriptionForm.php b/src/Traits/SubscriptionForm.php index 99aa0cd..91f5fcc 100644 --- a/src/Traits/SubscriptionForm.php +++ b/src/Traits/SubscriptionForm.php @@ -7,12 +7,13 @@ /** * Trait for use by subscription forms */ -trait SubscriptionForm { - +trait SubscriptionForm +{ /** * This method can be used to check the cache-able status of the form */ - public function checkCanBeCached() : bool { + public function checkCanBeCached(): bool + { return $this->canBeCached(); } diff --git a/tests/ChimpleConfigTest.php b/tests/ChimpleConfigTest.php index 25a9305..911535f 100644 --- a/tests/ChimpleConfigTest.php +++ b/tests/ChimpleConfigTest.php @@ -22,7 +22,6 @@ */ class ChimpleConfigTest extends SapphireTest { - /** * @inheritdoc */ @@ -46,7 +45,9 @@ class ChimpleConfigTest extends SapphireTest /** * @inheritdoc */ - public function setUp() : void { + #[\Override] + public function setUp(): void + { parent::setUp(); // Create default configuration @@ -71,7 +72,8 @@ public function setUp() : void { $config->write(); } - protected function getMailchimpConfig() { + protected function getMailchimpConfig(): ?\NSWDPC\Chimple\Models\MailchimpConfig + { // get config for the test list $config = MailchimpConfig::get()->filter(['MailchimpListId' => $this->test_list_id])->first(); @@ -84,17 +86,17 @@ protected function getMailchimpConfig() { // list check $list_id = $config->getMailchimpListId(); $this->assertNotEquals($list_id, $this->default_list_id, "List Id should not be the same as default list id"); - $this->assertTrue( $config->HasMailchimpListId(), "Config should have a list id"); + $this->assertTrue($config->HasMailchimpListId(), "Config should have a list id"); // test config retrieval via Code - $retrieved_config = MailchimpConfig::getConfig('','', $config->Code); + $retrieved_config = MailchimpConfig::getConfig('', '', $config->Code); $this->assertEquals($retrieved_config->ID, $config->ID, "Configs should be the same"); return $retrieved_config; } - public function testConfiguration() + public function testConfiguration(): void { $forceXhr = true; @@ -105,40 +107,41 @@ public function testConfiguration() // test configuration form retrieval $form = $config->SubscribeForm($forceXhr); - $this->assertTrue( $form instanceof Form, "SubscribeForm is not an instance of Form"); + $this->assertTrue($form instanceof Form, "SubscribeForm is not an instance of Form"); - $this->assertEquals( 1, $form->getAttribute('data-xhr'), "Form should have XHR attribute enabled" ); + $this->assertEquals(1, $form->getAttribute('data-xhr'), "Form should have XHR attribute enabled"); $fields = $form->Fields(); $email = $fields->dataFieldByName('Email'); - $this->assertTrue( $email instanceof EmailField, "Email field is not an email field"); + $this->assertTrue($email instanceof EmailField, "Email field is not an email field"); - $name = $fields->dataFieldByName('Name'); - $this->assertTrue( $email instanceof TextField, "Name field is not an text field"); + $fields->dataFieldByName('Name'); + $this->assertTrue($email instanceof TextField, "Name field is not an text field"); $token_name = SecurityToken::get_default_name(); $token = $fields->dataFieldByName($token_name); - $this->assertTrue( $token instanceof HiddenField, "{$token_name} field is not present"); + $this->assertTrue($token instanceof HiddenField, "{$token_name} field is not present"); $code_field = $fields->dataFieldByName('code'); - $this->assertTrue( $code_field instanceof HiddenField, "'code' field is not present"); + $this->assertTrue($code_field instanceof HiddenField, "'code' field is not present"); $code_value = $code_field->dataValue(); $this->assertEquals($code_value, $config->Code, "Code value in form is not the same as config value"); $static_form = MailchimpConfig::get_chimple_subscribe_form($code_value); - $this->assertTrue( $static_form instanceof DBHTMLText, "Static form for code {$code_value} was not returned"); + $this->assertTrue($static_form instanceof DBHTMLText, "Static form for code {$code_value} was not returned"); $needle = " value=\"{$code_value}\" "; - $this->assertTrue( strpos($static_form->forTemplate(), $needle) !== false, "Missing {$code_value} input from form HTML"); + $this->assertTrue(str_contains($static_form->forTemplate(), $needle), "Missing {$code_value} input from form HTML"); } - public function testCanBeCached() { + public function testCanBeCached(): void + { Config::modify()->set(XhrSubscribeForm::class, 'disable_security_token', true); @@ -147,59 +150,61 @@ public function testCanBeCached() { // test forcing XHR $forceXhr = true; $form = $config->SubscribeForm($forceXhr); - $this->assertTrue( $form->checkCanBeCached() ); + $this->assertTrue($form->checkCanBeCached()); // test not forcing XHR $forceXhr = false; $form = $config->SubscribeForm($forceXhr); - $this->assertFalse( $form->checkCanBeCached() ); + $this->assertFalse($form->checkCanBeCached()); // test using config value // config is turned off $config->UseXHR = 0; $form = $config->SubscribeForm();// default null value - $this->assertFalse( $form->checkCanBeCached() ); + $this->assertFalse($form->checkCanBeCached()); // config turned on $config->UseXHR = 1; $form = $config->SubscribeForm();// default null value - $this->assertTrue( $form->checkCanBeCached() ); + $this->assertTrue($form->checkCanBeCached()); } - public function testSubscribeFormTemplateVariable() { + public function testSubscribeFormTemplateVariable(): void + { $config = $this->getMailchimpConfig(); $config->UseXHR = 0; $config->write(); // Use config value $template = MailchimpConfig::get_chimple_subscribe_form($config->Code, null); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") === false, "Attribute is not in template"); + $this->assertTrue(!str_contains($template, 'data-xhr="1"'), "Attribute is not in template"); $config->UseXHR = 1; $config->write(); $template = MailchimpConfig::get_chimple_subscribe_form($config->Code, null); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") !== false, "Attribute is in template"); + $this->assertTrue(str_contains($template, 'data-xhr="1"'), "Attribute is in template"); // template override $config->UseXHR = 0; $config->write(); $template = MailchimpConfig::get_chimple_subscribe_form($config->Code, '1'); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") !== false, "Attribute is in template"); + $this->assertTrue(str_contains($template, 'data-xhr="1"'), "Attribute is in template"); $config->UseXHR = 0; $config->write(); $template = MailchimpConfig::get_chimple_subscribe_form($config->Code, '0'); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") === false, "Attribute is not in template"); + $this->assertTrue(!str_contains($template, 'data-xhr="1"'), "Attribute is not in template"); } - public function testGlobalSubscribeFormTemplateVariable() { + public function testGlobalSubscribeFormTemplateVariable(): void + { $config = $this->getMailchimpConfig(); $config->UseXHR = 0; $config->write(); // Use config value $template = MailchimpConfig::get_chimple_global_subscribe_form(); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") === false, "Attribute is not in template"); + $this->assertTrue(!str_contains($template, 'data-xhr="1"'), "Attribute is not in template"); $config->UseXHR = 1; $config->write(); $template = MailchimpConfig::get_chimple_global_subscribe_form(); - $this->assertTrue( strpos($template, "data-xhr=\"1\"") !== false, "Attribute is in template"); + $this->assertTrue(str_contains($template, 'data-xhr="1"'), "Attribute is in template"); } } diff --git a/tests/ChimpleFunctionalTest.php b/tests/ChimpleFunctionalTest.php index 11a95ad..75691f9 100644 --- a/tests/ChimpleFunctionalTest.php +++ b/tests/ChimpleFunctionalTest.php @@ -24,7 +24,6 @@ */ class ChimpleFunctionalTest extends FunctionalTest { - /** * @inheritdoc */ @@ -51,19 +50,22 @@ class ChimpleFunctionalTest extends FunctionalTest protected $test_form_code = 'functionalformcode'; - public function setUp() : void { + #[\Override] + public function setUp(): void + { parent::setUp(); // Inject test form Injector::inst()->registerService( - new TestSubscribeForm(), SubscribeForm::class + \NSWDPC\Chimple\Tests\TestSubscribeForm::create(), + SubscribeForm::class ); // Suppress themes SSViewer::set_themes( [ - SSViewer::DEFAULT_THEME + SSViewer::DEFAULT_THEME ] ); @@ -90,10 +92,10 @@ public function setUp() : void { $config->write(); } - public function testFormSubmission() + public function testFormSubmission(): void { - $this->useTestTheme(__DIR__, 'chimpletest', function () { + $this->useTestTheme(__DIR__, 'chimpletest', function (): void { // request default route $url = "/mc-subscribe/v1/"; @@ -115,7 +117,7 @@ public function testFormSubmission() 'Status' => MailchimpSubscriber::CHIMPLE_STATUS_NEW ])->first(); - $this->assertTrue( $subscriber && $subscriber->exists() ); + $this->assertTrue($subscriber && $subscriber->exists()); }); diff --git a/tests/ChimpleSubscriberTest.php b/tests/ChimpleSubscriberTest.php index 84dee4c..d7b57b9 100644 --- a/tests/ChimpleSubscriberTest.php +++ b/tests/ChimpleSubscriberTest.php @@ -16,30 +16,26 @@ */ class ChimpleSubscriberTest extends SapphireTest { - use Configurable; protected $usesDatabase = true; /** * e.g example.com - * @var string */ - private static $test_email_domain = ""; + private static string $test_email_domain = ""; /** * e.g bob.smith - * @var string */ - private static $test_email_user = ""; + private static string $test_email_user = ""; /** * use plus notation in email address - * @var bool */ - private static $test_use_plus = true; + private static bool $test_use_plus = true; - public function testSubscriber() + public function testSubscriber(): void { $fname = "Test"; $lname = "Tester"; @@ -68,6 +64,7 @@ public function testSubscriber() if($test_use_plus) { $email_address_for_test .= "+unittest" . bin2hex(random_bytes(2)); } + $email_address_for_test .= "@{$test_email_domain}"; $tags = ['TestOne','TestTwo']; @@ -98,21 +95,21 @@ public function testSubscriber() $subscribe_record = $subscriber->getSubscribeRecord(); - $this->assertTrue( is_array($subscribe_record), "Record is not an array of values"); + $this->assertTrue(is_array($subscribe_record), "Record is not an array of values"); - $this->assertTrue( !empty($subscribe_record), "Record is empty"); + $this->assertTrue($subscribe_record !== [], "Record is empty"); - $this->assertTrue( isset($subscribe_record['merge_fields']), "Record merge_fields is not set"); + $this->assertTrue(isset($subscribe_record['merge_fields']), "Record merge_fields is not set"); - $this->assertTrue( !empty($subscribe_record['email_type']), "Record email_type is empty"); + $this->assertTrue(!empty($subscribe_record['email_type']), "Record email_type is empty"); - $this->assertEquals( $subscriber->Email, $subscribe_record['email_address'], "Subscribed email_address value is not the same as subsciber record Email field value"); + $this->assertEquals($subscriber->Email, $subscribe_record['email_address'], "Subscribed email_address value is not the same as subsciber record Email field value"); // check merge fields $sync_fields = $subscriber->config()->get('sync_fields'); $merge_fields = $subscribe_record['merge_fields']; - foreach($sync_fields as $field => $tag ) { - $this->assertTrue( isset($merge_fields[ $tag ]) && $merge_fields[ $tag ] = $subscriber->getField($field), "Merge field tag {$tag} value does not match subscriber {$field} value"); + foreach($sync_fields as $field => $tag) { + $this->assertTrue(isset($merge_fields[ $tag ]) && $merge_fields[ $tag ] = $subscriber->getField($field), "Merge field tag {$tag} value does not match subscriber {$field} value"); } $email = $subscriber->Email; @@ -127,11 +124,11 @@ public function testSubscriber() $this->assertNotEmpty($subscriber->SubscribedUniqueEmailId, "SubscribedUniqueEmailId should not be empty"); - $this->assertTrue(substr_count($subscriber->Email, $obfuscation_chr) > 0, "Email is not obfuscated, it should be"); + $this->assertTrue(substr_count($subscriber->Email, (string) $obfuscation_chr) > 0, "Email is not obfuscated, it should be"); - $this->assertTrue(substr_count($subscriber->Name, $obfuscation_chr) > 0, "Name is not obfuscated, it should be"); + $this->assertTrue(substr_count($subscriber->Name, (string) $obfuscation_chr) > 0, "Name is not obfuscated, it should be"); - $this->assertTrue(substr_count($subscriber->Surname, $obfuscation_chr) > 0, "Surname is not obfuscated, it should be"); + $this->assertTrue(substr_count($subscriber->Surname, (string) $obfuscation_chr) > 0, "Surname is not obfuscated, it should be"); $mc_record = MailchimpSubscriber::checkExistsInList($list_id, $email); @@ -142,7 +139,7 @@ public function testSubscriber() $this->assertEquals(count($tags), count($mc_record['tags']), "Tag count mismatch"); $mc_tags_list = []; - array_walk($mc_record['tags'], function($value, $key) use (&$mc_tags_list) { + array_walk($mc_record['tags'], function (array $value, $key) use (&$mc_tags_list): void { $mc_tags_list[] = $value['name']; }); diff --git a/tests/TestSubscribeForm.php b/tests/TestSubscribeForm.php index 8bacb3d..7b2271b 100644 --- a/tests/TestSubscribeForm.php +++ b/tests/TestSubscribeForm.php @@ -3,17 +3,18 @@ namespace NSWDPC\Chimple\Tests; use NSWDPC\Chimple\Forms\SubscribeForm; -use SilverStripe\Dev\Testonly; +use SilverStripe\Dev\TestOnly; /** * Test subscribe form handling */ -class TestSubscribeForm extends SubscribeForm implements TestOnly { - +class TestSubscribeForm extends SubscribeForm implements TestOnly +{ /** * No need to spam protection on tests */ - public function enableSpamProtection() { + public function enableSpamProtection(): mixed + { return null; } }