From 4b0beecdfbcde9dffd0be31aea66a7da805c20b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Jakes=CC=8C?= Date: Wed, 8 May 2019 11:21:09 +0200 Subject: [PATCH] Convert 'display_revenues' setting to feature flag [MAILPOET-2008] --- lib/API/JSON/v1/Newsletters.php | 14 ++++++++++---- lib/Cron/Workers/StatsNotifications/Worker.php | 8 +++++++- lib/Cron/Workers/WorkersFactory.php | 8 +++++++- lib/Features/FeaturesController.php | 2 ++ lib/Models/Newsletter.php | 13 ++++++++----- tests/integration/API/JSON/v1/NewslettersTest.php | 10 +++++++--- .../Cron/Workers/StatsNotifications/WorkerTest.php | 3 ++- tests/integration/Models/NewsletterTest.php | 3 ++- 8 files changed, 45 insertions(+), 16 deletions(-) diff --git a/lib/API/JSON/v1/Newsletters.php b/lib/API/JSON/v1/Newsletters.php index 8631a95266..aa8356d1f7 100644 --- a/lib/API/JSON/v1/Newsletters.php +++ b/lib/API/JSON/v1/Newsletters.php @@ -8,6 +8,7 @@ use MailPoet\API\JSON\Error as APIError; use MailPoet\Config\AccessControl; use MailPoet\Cron\CronHelper; use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterQueueTask; +use MailPoet\Features\FeaturesController; use MailPoet\Listing; use MailPoet\Models\Newsletter; use MailPoet\Models\NewsletterOption; @@ -43,6 +44,9 @@ class Newsletters extends APIEndpoint { /** @var SettingsController */ private $settings; + /** @var FeaturesController */ + private $features_controller; + public $permissions = array( 'global' => AccessControl::PERMISSION_MANAGE_EMAILS ); @@ -52,13 +56,15 @@ class Newsletters extends APIEndpoint { Listing\Handler $listing_handler, WPFunctions $wp, WCHelper $woocommerce_helper, - SettingsController $settings + SettingsController $settings, + FeaturesController $features_controller ) { $this->bulk_action = $bulk_action; $this->listing_handler = $listing_handler; $this->wp = $wp; $this->woocommerce_helper = $woocommerce_helper; $this->settings = $settings; + $this->features_controller = $features_controller; } function get($data = array()) { @@ -443,13 +449,13 @@ class Newsletters extends APIEndpoint { $newsletter ->withSegments(true) ->withSendingQueue() - ->withStatistics($this->woocommerce_helper); + ->withStatistics($this->woocommerce_helper, $this->features_controller); } else if ($newsletter->type === Newsletter::TYPE_WELCOME || $newsletter->type === Newsletter::TYPE_AUTOMATIC) { $newsletter ->withOptions() ->withTotalSent() ->withScheduledToBeSent() - ->withStatistics($this->woocommerce_helper); + ->withStatistics($this->woocommerce_helper, $this->features_controller); } else if ($newsletter->type === Newsletter::TYPE_NOTIFICATION) { $newsletter ->withOptions() @@ -459,7 +465,7 @@ class Newsletters extends APIEndpoint { $newsletter ->withSegments(true) ->withSendingQueue() - ->withStatistics($this->woocommerce_helper); + ->withStatistics($this->woocommerce_helper, $this->features_controller); } if ($newsletter->status === Newsletter::STATUS_SENT || diff --git a/lib/Cron/Workers/StatsNotifications/Worker.php b/lib/Cron/Workers/StatsNotifications/Worker.php index 3cde923a58..021913905b 100644 --- a/lib/Cron/Workers/StatsNotifications/Worker.php +++ b/lib/Cron/Workers/StatsNotifications/Worker.php @@ -5,6 +5,7 @@ namespace MailPoet\Cron\Workers\StatsNotifications; use Carbon\Carbon; use MailPoet\Config\Renderer; use MailPoet\Cron\CronHelper; +use MailPoet\Features\FeaturesController; use MailPoet\Mailer\Mailer; use MailPoet\Models\Newsletter; use MailPoet\Models\NewsletterLink; @@ -32,6 +33,9 @@ class Worker { /** @var SettingsController */ private $settings; + /** @var FeaturesController */ + private $features_controller; + /** @var WCHelper */ private $woocommerce_helper; @@ -39,6 +43,7 @@ class Worker { Mailer $mailer, Renderer $renderer, SettingsController $settings, + FeaturesController $features_controller, WCHelper $woocommerce_helper, $timer = false ) { @@ -46,6 +51,7 @@ class Worker { $this->renderer = $renderer; $this->mailer = $mailer; $this->settings = $settings; + $this->features_controller = $features_controller; $this->woocommerce_helper = $woocommerce_helper; } @@ -103,7 +109,7 @@ class Worker { return $newsletter ->withSendingQueue() ->withTotalSent() - ->withStatistics($this->woocommerce_helper); + ->withStatistics($this->woocommerce_helper, $this->features_controller); } /** diff --git a/lib/Cron/Workers/WorkersFactory.php b/lib/Cron/Workers/WorkersFactory.php index 570762d119..8ef0c04ab7 100644 --- a/lib/Cron/Workers/WorkersFactory.php +++ b/lib/Cron/Workers/WorkersFactory.php @@ -13,6 +13,7 @@ use MailPoet\Cron\Workers\KeyCheck\PremiumKeyCheck as PremiumKeyCheckWorker; use MailPoet\Cron\Workers\KeyCheck\SendingServiceKeyCheck as SendingServiceKeyCheckWorker; use MailPoet\Cron\Workers\WooCommerceSync as WooCommerceSyncWorker; use MailPoet\Cron\Workers\SendingQueue\SendingErrorHandler; +use MailPoet\Features\FeaturesController; use MailPoet\Segments\WooCommerce as WooCommerceSegment; use MailPoet\WooCommerce\Helper as WooCommerceHelper; use MailPoet\Mailer\Mailer; @@ -33,6 +34,9 @@ class WorkersFactory { /** @var SettingsController */ private $settings; + /** @var FeaturesController */ + private $features_controller; + /** @var WooCommerceSegment */ private $woocommerce_segment; @@ -53,6 +57,7 @@ class WorkersFactory { Mailer $mailer, Renderer $renderer, SettingsController $settings, + FeaturesController $features_controller, WooCommerceSegment $woocommerce_segment, InactiveSubscribersController $inactive_subscribers_controller, WooCommerceHelper $woocommerce_helper @@ -62,6 +67,7 @@ class WorkersFactory { $this->mailer = $mailer; $this->renderer = $renderer; $this->settings = $settings; + $this->features_controller = $features_controller; $this->woocommerce_segment = $woocommerce_segment; $this->inactive_subscribers_controller = $inactive_subscribers_controller; $this->woocommerce_helper = $woocommerce_helper; @@ -78,7 +84,7 @@ class WorkersFactory { } function createStatsNotificationsWorker($timer) { - return new StatsNotificationsWorker($this->mailer, $this->renderer, $this->settings, $this->woocommerce_helper, $timer); + return new StatsNotificationsWorker($this->mailer, $this->renderer, $this->settings, $this->features_controller, $this->woocommerce_helper, $timer); } /** @return SendingServiceKeyCheckWorker */ diff --git a/lib/Features/FeaturesController.php b/lib/Features/FeaturesController.php index 716ef87356..e4a02e324a 100644 --- a/lib/Features/FeaturesController.php +++ b/lib/Features/FeaturesController.php @@ -8,10 +8,12 @@ class FeaturesController { // Define features below in the following form: // const FEATURE_NAME_OF_FEATURE = 'name-of-feature'; + const FEATURE_DISPLAY_WOOCOMMERCE_REVENUES = 'display-woocommerce-revenues'; // may also have 'display_revenues' setting // Define feature defaults in the array below in the following form: // self::FEATURE_NAME_OF_FEATURE => true, public static $defaults = [ + self::FEATURE_DISPLAY_WOOCOMMERCE_REVENUES => true, ]; /** @var array */ diff --git a/lib/Models/Newsletter.php b/lib/Models/Newsletter.php index 4190299b0e..0f37f6307e 100644 --- a/lib/Models/Newsletter.php +++ b/lib/Models/Newsletter.php @@ -1,6 +1,7 @@ getStatistics($woocommerce_helper); + function withStatistics(WCHelper $woocommerce_helper, FeaturesController $features_controller) { + $statistics = $this->getStatistics($woocommerce_helper, $features_controller); $this->statistics = $statistics; return $this; } @@ -560,7 +561,7 @@ class Newsletter extends Model { return $renderer->render(); } - function getStatistics(WCHelper $woocommerce_helper) { + function getStatistics(WCHelper $woocommerce_helper, FeaturesController $features_controller) { if (($this->type !== self::TYPE_WELCOME) && ($this->queue === false)) { return false; } @@ -589,8 +590,10 @@ class Newsletter extends Model { } // WooCommerce revenues - $settings = new SettingsController(); - if ($woocommerce_helper->isWooCommerceActive() && $settings->get('display_revenues')) { + if ( + $features_controller->isSupported(FeaturesController::FEATURE_DISPLAY_WOOCOMMERCE_REVENUES) + && $woocommerce_helper->isWooCommerceActive() + ) { $currency = $woocommerce_helper->getWoocommerceCurrency(); $row = StatisticsWooCommercePurchases::selectExpr('SUM(order_price_total) AS total') ->where([ diff --git a/tests/integration/API/JSON/v1/NewslettersTest.php b/tests/integration/API/JSON/v1/NewslettersTest.php index 12b14c54a8..e34fa3c871 100644 --- a/tests/integration/API/JSON/v1/NewslettersTest.php +++ b/tests/integration/API/JSON/v1/NewslettersTest.php @@ -7,6 +7,7 @@ use Codeception\Util\Fixtures; use Codeception\Util\Stub; use Helper\WordPressHooks as WPHooksHelper; use MailPoet\API\JSON\Response as APIResponse; +use MailPoet\Features\FeaturesController; use MailPoet\Listing\BulkActionController; use MailPoet\Listing\Handler; use MailPoet\API\JSON\v1\Newsletters; @@ -112,7 +113,8 @@ class NewslettersTest extends \MailPoetTest { ContainerWrapper::getInstance()->get(Handler::class), $wp, $this->makeEmpty(WCHelper::class), - new SettingsController() + new SettingsController(), + new FeaturesController() ); $response = $this->endpoint->get(array('id' => $this->newsletter->id)); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -151,7 +153,8 @@ class NewslettersTest extends \MailPoetTest { ContainerWrapper::getInstance()->get(Handler::class), $wp, $this->makeEmpty(WCHelper::class), - new SettingsController() + new SettingsController(), + new FeaturesController() ); $response = $this->endpoint->save($valid_data); @@ -517,7 +520,8 @@ class NewslettersTest extends \MailPoetTest { ContainerWrapper::getInstance()->get(Handler::class), $wp, $this->makeEmpty(WCHelper::class), - new SettingsController() + new SettingsController(), + new FeaturesController() ); $response = $this->endpoint->duplicate(array('id' => $this->newsletter->id)); diff --git a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php index 34858daff3..578641bace 100644 --- a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php +++ b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php @@ -3,6 +3,7 @@ namespace MailPoet\Cron\Workers\StatsNotifications; use MailPoet\Config\Renderer; +use MailPoet\Features\FeaturesController; use MailPoet\Mailer\Mailer; use MailPoet\Models\Newsletter; use MailPoet\Models\NewsletterLink; @@ -39,7 +40,7 @@ class WorkerTest extends \MailPoetTest { $this->mailer = $this->createMock(Mailer::class); $this->renderer = $this->createMock(Renderer::class); $this->settings = new SettingsController(); - $this->stats_notifications = new Worker($this->mailer, $this->renderer, $this->settings, $this->makeEmpty(WCHelper::class)); + $this->stats_notifications = new Worker($this->mailer, $this->renderer, $this->settings, new FeaturesController(), $this->makeEmpty(WCHelper::class)); $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => true, 'address' => 'email@example.com' diff --git a/tests/integration/Models/NewsletterTest.php b/tests/integration/Models/NewsletterTest.php index 678ac18866..e47a47476a 100644 --- a/tests/integration/Models/NewsletterTest.php +++ b/tests/integration/Models/NewsletterTest.php @@ -2,6 +2,7 @@ namespace MailPoet\Test\Models; use Carbon\Carbon; +use MailPoet\Features\FeaturesController; use MailPoet\Models\Newsletter; use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; @@ -170,7 +171,7 @@ class NewsletterTest extends \MailPoetTest { $unsubscribes->save(); $newsletter->queue = $newsletter->getQueue()->asArray(); - $statistics = $newsletter->getStatistics($this->makeEmpty(WCHelper::class)); + $statistics = $newsletter->getStatistics($this->makeEmpty(WCHelper::class), new FeaturesController()); expect($statistics['opened'])->equals(1); expect($statistics['clicked'])->equals(1); expect($statistics['unsubscribed'])->equals(1);