Remove the validation for marketing channel slugs
Do not check if the marketing channel's slug exists in the list returned by WooCommerce.com Recommendation API. This essentially allows any third-party extension to register as a marketing channel.
This commit is contained in:
Nima
parent
8d8f416fce
commit
7734d41887
|
|
@ -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 );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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] );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue