diff --git a/plugins/woocommerce/src/Admin/Marketing/MarketingChannels.php b/plugins/woocommerce/src/Admin/Marketing/MarketingChannels.php index 0fcb6fd1e5e..a5fde99eb32 100644 --- a/plugins/woocommerce/src/Admin/Marketing/MarketingChannels.php +++ b/plugins/woocommerce/src/Admin/Marketing/MarketingChannels.php @@ -5,7 +5,7 @@ namespace Automattic\WooCommerce\Admin\Marketing; -use Automattic\WooCommerce\Internal\Admin\Marketing\MarketingSpecs; +use Exception; /** * MarketingChannels repository class @@ -20,32 +20,6 @@ class MarketingChannels { */ private $registered_channels = []; - /** - * Array of plugin slugs for allowed marketing channels. - * - * @var string[] - */ - private $allowed_channels; - - /** - * MarketingSpecs repository - * - * @var MarketingSpecs - */ - protected $marketing_specs; - - /** - * Class initialization, invoked by the DI container. - * - * @param MarketingSpecs $marketing_specs The MarketingSpecs class. - * - * @internal - */ - final public function init( MarketingSpecs $marketing_specs ) { - $this->marketing_specs = $marketing_specs; - $this->allowed_channels = $this->get_allowed_channels(); - } - /** * Registers a marketing channel. * @@ -55,14 +29,11 @@ class MarketingChannels { * * @return void * - * @see MarketingChannels::is_channel_allowed() Checks if the marketing channel is allowed to be registered or not. + * @throws Exception If the given marketing channel is already registered. */ public function register( MarketingChannelInterface $channel ): void { - if ( ! $this->is_channel_allowed( $channel ) ) { - // Silently log an error and bail. - wc_get_logger()->error( sprintf( 'Marketing channel %s (%s) cannot be registered!', $channel->get_name(), $channel->get_slug() ) ); - - return; + if ( isset( $this->registered_channels[ $channel->get_slug() ] ) ) { + throw new Exception( 'Marketing channel cannot be registered because there is already a channel registered with the same slug!' ); } $this->registered_channels[ $channel->get_slug() ] = $channel; @@ -86,46 +57,6 @@ class MarketingChannels { */ $channels = apply_filters( 'woocommerce_marketing_channels', $this->registered_channels ); - // Only return allowed channels. - $allowed_channels = array_filter( - $channels, - function ( MarketingChannelInterface $channel ) { - if ( ! $this->is_channel_allowed( $channel ) ) { - // Silently log an error and bail. - wc_get_logger()->error( sprintf( 'Marketing channel %s (%s) cannot be registered!', $channel->get_name(), $channel->get_slug() ) ); - - return false; - } - - return true; - } - ); - - return array_values( $allowed_channels ); - } - - /** - * Returns an array of plugin slugs for the marketing channels that are allowed to be registered. - * - * @return array - */ - protected function get_allowed_channels(): array { - $recommended_channels = $this->marketing_specs->get_recommended_plugins(); - if ( empty( $recommended_channels ) ) { - return []; - } - - return array_column( $recommended_channels, 'product', 'product' ); - } - - /** - * Determines whether the given marketing channel is allowed to be registered. - * - * @param MarketingChannelInterface $channel The marketing channel object. - * - * @return bool - */ - protected function is_channel_allowed( MarketingChannelInterface $channel ): bool { - return isset( $this->allowed_channels[ $channel->get_slug() ] ); + return array_values( $channels ); } } diff --git a/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/MarketingServiceProvider.php b/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/MarketingServiceProvider.php index 8e27386ae86..3fd7677c84c 100644 --- a/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/MarketingServiceProvider.php +++ b/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/MarketingServiceProvider.php @@ -38,7 +38,7 @@ class MarketingServiceProvider extends AbstractServiceProvider { */ public function register() { $this->share( MarketingSpecs::class ); - $this->share( MarketingChannels::class )->addArgument( MarketingSpecs::class ); + $this->share( MarketingChannels::class ); $this->share( InstalledExtensions::class )->addArgument( MarketingChannels::class ); } } diff --git a/plugins/woocommerce/tests/php/src/Admin/Marketing/MarketingChannelsTest.php b/plugins/woocommerce/tests/php/src/Admin/Marketing/MarketingChannelsTest.php index cf5885a0557..fdc112ae370 100644 --- a/plugins/woocommerce/tests/php/src/Admin/Marketing/MarketingChannelsTest.php +++ b/plugins/woocommerce/tests/php/src/Admin/Marketing/MarketingChannelsTest.php @@ -4,7 +4,6 @@ namespace Automattic\WooCommerce\Tests\Admin\Marketing; use Automattic\WooCommerce\Admin\Marketing\MarketingChannelInterface; use Automattic\WooCommerce\Admin\Marketing\MarketingChannels; -use Automattic\WooCommerce\Internal\Admin\Marketing\MarketingSpecs; use WC_Unit_Test_Case; /** @@ -16,29 +15,17 @@ class MarketingChannelsTest extends WC_Unit_Test_Case { * Runs before each test. */ public function setUp(): void { - delete_transient( MarketingSpecs::RECOMMENDED_PLUGINS_TRANSIENT ); + remove_all_filters( 'woocommerce_marketing_channels' ); } /** - * @testdox A marketing channel can be registered using the `register` method if it is in the allowed list. + * @testdox A marketing channel can be registered using the `register` method if the same channel slug is NOT previously registered. */ - public function test_registers_allowed_channels() { + public function test_registers_channel() { $test_channel = $this->createMock( MarketingChannelInterface::class ); $test_channel->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); - $marketing_specs = $this->createMock( MarketingSpecs::class ); - $marketing_specs->expects( $this->once() ) - ->method( 'get_recommended_plugins' ) - ->willReturn( - [ - [ - 'product' => 'test-channel-1', - ], - ] - ); - $marketing_channels = new MarketingChannels(); - $marketing_channels->init( $marketing_specs ); $marketing_channels->register( $test_channel ); $this->assertNotEmpty( $marketing_channels->get_registered_channels() ); @@ -46,42 +33,33 @@ class MarketingChannelsTest extends WC_Unit_Test_Case { } /** - * @testdox A marketing channel can NOT be registered using the `register` method if it is NOT in the allowed list. + * @testdox A marketing channel can NOT be registered using the `register` method if it is previously registered. */ - public function test_does_not_register_disallowed_channels() { - $test_channel = $this->createMock( MarketingChannelInterface::class ); - $test_channel->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); + public function test_throws_exception_if_registering_existing_channels() { + $test_channel_1 = $this->createMock( MarketingChannelInterface::class ); + $test_channel_1->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); - $marketing_specs = $this->createMock( MarketingSpecs::class ); - $marketing_specs->expects( $this->once() )->method( 'get_recommended_plugins' )->willReturn( [] ); + $test_channel_1_duplicate = $this->createMock( MarketingChannelInterface::class ); + $test_channel_1_duplicate->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); $marketing_channels = new MarketingChannels(); - $marketing_channels->init( $marketing_specs ); - $marketing_channels->register( $test_channel ); + $marketing_channels->register( $test_channel_1 ); - $this->assertEmpty( $marketing_channels->get_registered_channels() ); + $this->expectException( \Exception::class ); + $marketing_channels->register( $test_channel_1_duplicate ); + + $this->assertCount( 1, $marketing_channels->get_registered_channels() ); + $this->assertEquals( $test_channel_1, $marketing_channels->get_registered_channels()[0] ); } /** - * @testdox A marketing channel can be registered using the `woocommerce_marketing_channels` WordPress filter if it is in the allowed list. + * @testdox A marketing channel can be registered using the `woocommerce_marketing_channels` WordPress filter if the same channel slug is NOT previously registered. */ - public function test_registers_allowed_channels_using_wp_filter() { + public function test_registers_channel_using_wp_filter() { $test_channel = $this->createMock( MarketingChannelInterface::class ); $test_channel->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); - $marketing_specs = $this->createMock( MarketingSpecs::class ); - $marketing_specs->expects( $this->once() ) - ->method( 'get_recommended_plugins' ) - ->willReturn( - [ - [ - 'product' => 'test-channel-1', - ], - ] - ); - $marketing_channels = new MarketingChannels(); - $marketing_channels->init( $marketing_specs ); add_filter( 'woocommerce_marketing_channels', @@ -97,24 +75,29 @@ class MarketingChannelsTest extends WC_Unit_Test_Case { } /** - * @testdox A marketing channel can NOT be registered using the `woocommerce_marketing_channels` WordPress filter if it NOT is in the allowed list. + * @testdox A marketing channel can NOT be registered using the `woocommerce_marketing_channels` WordPress filter if it is previously registered. */ - public function test_does_not_register_disallowed_channels_using_wp_filter() { - $test_channel = $this->createMock( MarketingChannelInterface::class ); - $test_channel->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); + public function test_overrides_existing_channel_if_registered_using_wp_filter() { + $marketing_channels = new MarketingChannels(); - set_transient( MarketingSpecs::RECOMMENDED_PLUGINS_TRANSIENT, [] ); + $test_channel_1 = $this->createMock( MarketingChannelInterface::class ); + $test_channel_1->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); + + $marketing_channels->register( $test_channel_1 ); + + $test_channel_1_duplicate = $this->createMock( MarketingChannelInterface::class ); + $test_channel_1_duplicate->expects( $this->any() )->method( 'get_slug' )->willReturn( 'test-channel-1' ); add_filter( 'woocommerce_marketing_channels', - function ( array $channels ) use ( $test_channel ) { - $channels[ $test_channel->get_slug() ] = $test_channel; + function ( array $channels ) use ( $test_channel_1_duplicate ) { + $channels[ $test_channel_1_duplicate->get_slug() ] = $test_channel_1_duplicate; return $channels; } ); - $marketing_channels = new MarketingChannels(); - $this->assertEmpty( $marketing_channels->get_registered_channels() ); + $this->assertCount( 1, $marketing_channels->get_registered_channels() ); + $this->assertEquals( $test_channel_1_duplicate, $marketing_channels->get_registered_channels()[0] ); } }