Fix more PR remarks [MAILPOET-2015]

Use fast termination to remove some nesting
Use DI for CaptchaRenderer
Remove unused vars in router subscription endpoint
This commit is contained in:
wxa
2019-07-15 13:35:28 +03:00
committed by M. Shull
parent f24aec847f
commit 4a309e7317
6 changed files with 68 additions and 57 deletions

View File

@@ -246,8 +246,12 @@ class Subscribers extends APIEndpoint {
} }
private function validateCaptcha($captcha_settings, $data) { private function validateCaptcha($captcha_settings, $data) {
if (empty($captcha_settings['type'])) {
return true;
}
$is_builtin_captcha_required = false; $is_builtin_captcha_required = false;
if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_BUILTIN) { if ($captcha_settings['type'] === Captcha::TYPE_BUILTIN) {
$is_builtin_captcha_required = $this->subscription_captcha->isRequired(isset($data['email']) ? $data['email'] : ''); $is_builtin_captcha_required = $this->subscription_captcha->isRequired(isset($data['email']) ? $data['email'] : '');
if ($is_builtin_captcha_required && empty($data['captcha'])) { if ($is_builtin_captcha_required && empty($data['captcha'])) {
$meta = []; $meta = [];
@@ -258,13 +262,12 @@ class Subscribers extends APIEndpoint {
} }
} }
if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_RECAPTCHA && empty($data['recaptcha'])) { if ($captcha_settings['type'] === Captcha::TYPE_RECAPTCHA && empty($data['recaptcha'])) {
return $this->badRequest([ return $this->badRequest([
APIError::BAD_REQUEST => WPFunctions::get()->__('Please check the CAPTCHA.', 'mailpoet'), APIError::BAD_REQUEST => WPFunctions::get()->__('Please check the CAPTCHA.', 'mailpoet'),
]); ]);
} }
if (!empty($captcha_settings['type'])) {
if ($captcha_settings['type'] === Captcha::TYPE_RECAPTCHA) { if ($captcha_settings['type'] === Captcha::TYPE_RECAPTCHA) {
$res = empty($data['recaptcha']) ? $data['recaptcha-no-js'] : $data['recaptcha']; $res = empty($data['recaptcha']) ? $data['recaptcha-no-js'] : $data['recaptcha'];
$res = WPFunctions::get()->wpRemotePost('https://www.google.com/recaptcha/api/siteverify', [ $res = WPFunctions::get()->wpRemotePost('https://www.google.com/recaptcha/api/siteverify', [
@@ -302,7 +305,6 @@ class Subscribers extends APIEndpoint {
$_SESSION[Captcha::SESSION_FORM_KEY] = null; $_SESSION[Captcha::SESSION_FORM_KEY] = null;
} }
} }
}
return true; return true;
} }

View File

@@ -146,6 +146,7 @@ class ContainerConfigurator implements IContainerConfigurator {
$container->autowire(\MailPoet\Settings\UserFlagsController::class); $container->autowire(\MailPoet\Settings\UserFlagsController::class);
// Subscription // Subscription
$container->autowire(\MailPoet\Subscription\Captcha::class)->setPublic(true); $container->autowire(\MailPoet\Subscription\Captcha::class)->setPublic(true);
$container->autowire(\MailPoet\Subscription\CaptchaRenderer::class)->setPublic(true);
$container->autowire(\MailPoet\Subscription\Comment::class)->setPublic(true); $container->autowire(\MailPoet\Subscription\Comment::class)->setPublic(true);
$container->autowire(\MailPoet\Subscription\Form::class)->setPublic(true); $container->autowire(\MailPoet\Subscription\Form::class)->setPublic(true);
$container->autowire(\MailPoet\Subscription\Manage::class)->setPublic(true); $container->autowire(\MailPoet\Subscription\Manage::class)->setPublic(true);

View File

@@ -33,7 +33,7 @@ class Subscription {
} }
function captcha($data) { function captcha($data) {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_CAPTCHA, $data); $this->initSubscriptionPage(UserSubscription\Pages::ACTION_CAPTCHA, $data);
} }
function captchaImage($data) { function captchaImage($data) {
@@ -49,7 +49,7 @@ class Subscription {
} }
function manage($data) { function manage($data) {
$subscription = $this->initSubscriptionPage(UserSubscription\Pages::ACTION_MANAGE, $data); $this->initSubscriptionPage(UserSubscription\Pages::ACTION_MANAGE, $data);
} }
function unsubscribe($data) { function unsubscribe($data) {

View File

@@ -51,7 +51,10 @@ class Captcha {
$subscriber_ip = Helpers::getIP(); $subscriber_ip = Helpers::getIP();
if (!empty($subscriber_ip)) { if (empty($subscriber_ip)) {
return false;
}
$subscription_count = SubscriberIP::where('ip', $subscriber_ip) $subscription_count = SubscriberIP::where('ip', $subscriber_ip)
->whereRaw( ->whereRaw(
'(`created_at` >= NOW() - INTERVAL ? SECOND)', '(`created_at` >= NOW() - INTERVAL ? SECOND)',
@@ -61,7 +64,6 @@ class Captcha {
if ($subscription_count > 0) { if ($subscription_count > 0) {
return true; return true;
} }
}
return false; return false;
} }

View File

@@ -11,12 +11,16 @@ class CaptchaRenderer {
/** @var UrlHelper */ /** @var UrlHelper */
private $url_helper; private $url_helper;
function __construct() { /** @var WPFunctions */
$this->url_helper = new UrlHelper(new WPFunctions()); private $wp;
function __construct(UrlHelper $url_helper, WPFunctions $wp) {
$this->url_helper = $url_helper;
$this->wp = $wp;
} }
public function getCaptchaPageTitle() { public function getCaptchaPageTitle() {
return WPFunctions::get()->__("Confirm youre not a robot", 'mailpoet'); return $this->wp->__("Confirm youre not a robot", 'mailpoet');
} }
public function getCaptchaPageContent() { public function getCaptchaPageContent() {
@@ -25,7 +29,7 @@ class CaptchaRenderer {
'id' => 'captcha', 'id' => 'captcha',
'type' => 'text', 'type' => 'text',
'params' => [ 'params' => [
'label' => WPFunctions::get()->__('Type in the input the characters you see in the picture above:', 'mailpoet'), 'label' => $this->wp->__('Type in the input the characters you see in the picture above:', 'mailpoet'),
'value' => '', 'value' => '',
'obfuscate' => false, 'obfuscate' => false,
], ],
@@ -39,7 +43,7 @@ class CaptchaRenderer {
'id' => 'submit', 'id' => 'submit',
'type' => 'submit', 'type' => 'submit',
'params' => [ 'params' => [
'label' => WPFunctions::get()->__('Subscribe', 'mailpoet'), 'label' => $this->wp->__('Subscribe', 'mailpoet'),
], ],
], ],
] ]
@@ -69,7 +73,7 @@ class CaptchaRenderer {
$form_html .= '<div class="mailpoet_form_hide_on_success">'; $form_html .= '<div class="mailpoet_form_hide_on_success">';
$form_html .= '<p class="mailpoet_paragraph">'; $form_html .= '<p class="mailpoet_paragraph">';
$form_html .= '<img class="mailpoet_captcha mailpoet_captcha_update" src="' . $captcha_url . '" width="' . $width . '" height="' . $height . '" title="' . WPFunctions::get()->__('Click to refresh the captcha', 'mailpoet') . '" />'; $form_html .= '<img class="mailpoet_captcha mailpoet_captcha_update" src="' . $captcha_url . '" width="' . $width . '" height="' . $height . '" title="' . $this->wp->__('Click to refresh the captcha', 'mailpoet') . '" />';
$form_html .= '</p>'; $form_html .= '</p>';
// subscription form // subscription form

View File

@@ -46,18 +46,20 @@ class Pages {
NewSubscriberNotificationMailer $new_subscriber_notification_sender, NewSubscriberNotificationMailer $new_subscriber_notification_sender,
WPFunctions $wp, WPFunctions $wp,
SettingsController $settings, SettingsController $settings,
UrlHelper $url_helper UrlHelper $url_helper,
CaptchaRenderer $captcha_renderer
) { ) {
$this->wp = $wp; $this->wp = $wp;
$this->new_subscriber_notification_sender = $new_subscriber_notification_sender; $this->new_subscriber_notification_sender = $new_subscriber_notification_sender;
$this->settings = $settings; $this->settings = $settings;
$this->url_helper = $url_helper; $this->url_helper = $url_helper;
$this->captcha_renderer = new CaptchaRenderer; $this->captcha_renderer = $captcha_renderer;
} }
function init($action = false, $data = [], $init_shortcodes = false, $init_page_filters = false) { function init($action = false, $data = [], $init_shortcodes = false, $init_page_filters = false) {
$this->action = $action; $this->action = $action;
$this->data = $data; $this->data = $data;
$this->wp = new WPFunctions();
$this->subscriber = $this->getSubscriber(); $this->subscriber = $this->getSubscriber();
if ($init_page_filters) $this->initPageFilters(); if ($init_page_filters) $this->initPageFilters();
if ($init_shortcodes) $this->initShortcodes(); if ($init_shortcodes) $this->initShortcodes();