-
Notifications
You must be signed in to change notification settings - Fork 10.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multichannel Marketing - Changes to the marketing classes #36012
Multichannel Marketing - Changes to the marketing classes #36012
Conversation
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.
The InstalledExtensions class will be used by the previous generation of Marketing dashboard (if the user has not enabled the new "Marketing" feature); therefore, it's best to restore it to the original code.
New hook, template, or database changes in this PRTemplate changes:
|
Test Results SummaryCommit SHA: 4e192e7
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
…ure/35956-mcm-library-changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻 LGTM, I left a few notes about the changes made to (or missing from) MarketingChannels.php
.
One of the tests was failing, but I re-ran them and everything came up green.
|
||
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!' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could/should this be included for translation? e.g.,
woocommerce/plugins/woocommerce/src/Admin/Notes/DataStore.php
Lines 126 to 128 in 527249f
} else { | |
throw new \Exception( __( 'Invalid admin note', 'woocommerce' ) ); | |
} |
$this->marketing_specs = $marketing_specs; | ||
$this->allowed_channels = $this->get_allowed_channels(); | ||
} | ||
|
||
/** | ||
* Registers a marketing channel. | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a few references to the "predetermined list of third party extension" in the method doc blocks that could be cleaned up.
* Multichannel Marketing - Core Library (#35099) * Create channel interface and campaign value class * Create MarketingChannels class * Register MarketingChannels class in DI container * Use the new MarketingChannels class to get the installed marketing extensions' data * Use DI container to access InstalledExtensions class * Add InstalledExtensions to the $provides array * Hint that campaign cost should also indicate the currency * Initialize the channels array * Add unit tests for MarketingCampaign * Add unit tests for MarketingChannels * Add Price class to represent a price with currency * Use Price class for marketing campaign's cost * Define a constant to indicate the MCM classes exist This constant will be checked by third-party extensions before utilizing any of the classes/interfaces defined for this feature. * Create MarketingSpecs class to include WC.com API calls * Remove WC.com API calls from Marketing class And replace them with calls from MarketingSpecs class. * Use the const from MarketingSpecs * Fix MarketingChannels unit tests * Add missing settings URL to the channel data Co-authored-by: Nima <nima.karimi@automattic.com> * Multichannel Marketing - Changes to the marketing classes (#36012) * Rename `get_errors_no` to `get_errors_count` * 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 allows any third-party extension to register as a marketing channel. * Revert InstalledExtensions The InstalledExtensions class will be used by the previous generation of the Marketing dashboard (if the user has not enabled the new "Marketing" feature); therefore, it's best to restore it to the original code. * Fix code style * Translate Exception message * Remove doc references to a predetermined list of marketing channels Co-authored-by: Nima <nima.karimi@automattic.com> * Multichannel Marketing - API (#36222) * Rename `get_errors_no` to `get_errors_count` * 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. * Revert InstalledExtensions The InstalledExtensions class will be used by the previous generation of Marketing dashboard (if the user has not enabled the new "Marketing" feature); therefore, it's best to restore it to the original code. * Fix code style * Add channel property to MarketingCampaign * Add methods to filter the recommended marketing channels and extensions * Add `marketing/recommendations` API * Add unit tests for `marketing/recommendations` API * Add `marketing/channels` API * Add unit tests for `marketing/channels` API * Add `marketing/campaigns` API * Add unit tests for `marketing/campaigns` API * Translate Exception message * Remove doc references to predetermined list of marketing channels * Add `unregister_all` method To allow unregistering all marketing channels. * Unregister all channels on test tear down * Change API access denied authorization code * Change API access permission * Add MarketingCampaignType class This allows defining campaign types for each marketing channel. * Add campaign type property to campaign class * Add `marketing/campaign-types` API This API returns the aggregated list of supported marketing campaign types for all registered marketing channels. * Add unit tests for `marketing/campaign-types` API * Remove unused jsonSerialize method * Fix unit tests Co-authored-by: Nima <nima.karimi@automattic.com> * Add changelog * Add product listing status sync failed Co-authored-by: Nima <nima.karimi@automattic.com>
* Multichannel Marketing - Core Library (#35099) * Create channel interface and campaign value class * Create MarketingChannels class * Register MarketingChannels class in DI container * Use the new MarketingChannels class to get the installed marketing extensions' data * Use DI container to access InstalledExtensions class * Add InstalledExtensions to the $provides array * Hint that campaign cost should also indicate the currency * Initialize the channels array * Add unit tests for MarketingCampaign * Add unit tests for MarketingChannels * Add Price class to represent a price with currency * Use Price class for marketing campaign's cost * Define a constant to indicate the MCM classes exist This constant will be checked by third-party extensions before utilizing any of the classes/interfaces defined for this feature. * Create MarketingSpecs class to include WC.com API calls * Remove WC.com API calls from Marketing class And replace them with calls from MarketingSpecs class. * Use the const from MarketingSpecs * Fix MarketingChannels unit tests * Add missing settings URL to the channel data Co-authored-by: Nima <nima.karimi@automattic.com> * Multichannel Marketing - Changes to the marketing classes (#36012) * Rename `get_errors_no` to `get_errors_count` * 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 allows any third-party extension to register as a marketing channel. * Revert InstalledExtensions The InstalledExtensions class will be used by the previous generation of the Marketing dashboard (if the user has not enabled the new "Marketing" feature); therefore, it's best to restore it to the original code. * Fix code style * Translate Exception message * Remove doc references to a predetermined list of marketing channels Co-authored-by: Nima <nima.karimi@automattic.com> * Multichannel Marketing - API (#36222) * Rename `get_errors_no` to `get_errors_count` * 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. * Revert InstalledExtensions The InstalledExtensions class will be used by the previous generation of Marketing dashboard (if the user has not enabled the new "Marketing" feature); therefore, it's best to restore it to the original code. * Fix code style * Add channel property to MarketingCampaign * Add methods to filter the recommended marketing channels and extensions * Add `marketing/recommendations` API * Add unit tests for `marketing/recommendations` API * Add `marketing/channels` API * Add unit tests for `marketing/channels` API * Add `marketing/campaigns` API * Add unit tests for `marketing/campaigns` API * Translate Exception message * Remove doc references to predetermined list of marketing channels * Add `unregister_all` method To allow unregistering all marketing channels. * Unregister all channels on test tear down * Change API access denied authorization code * Change API access permission * Add MarketingCampaignType class This allows defining campaign types for each marketing channel. * Add campaign type property to campaign class * Add `marketing/campaign-types` API This API returns the aggregated list of supported marketing campaign types for all registered marketing channels. * Add unit tests for `marketing/campaign-types` API * Remove unused jsonSerialize method * Fix unit tests Co-authored-by: Nima <nima.karimi@automattic.com> * Add changelog * Add product listing status sync failed Co-authored-by: Nima <nima.karimi@automattic.com>
All Submissions:
Changes proposed in this Pull Request:
This PR implements the following changes to the core library (initially introduced in #35099):
GET https://woocommerce.com/wp-json/wccom/marketing-tab/1.2/recommendations.json
API before allowing them to register as a marketing channel.MarketingChannelInterface::get_error_no
toget_error_count
. The reasoning for this change was thatget_error_count
describes more clearly what the method returns. (reference: pe2C5g-zb-p2)InstalledExtensions.php
class and its usage. This is a legacy class, and it's best to keep it intact since we are introducing the new marketing dashboard as an opt-in feature. The new feature will be implemented as a new API in Multichannel Marketing Backend MVP: API #34556Closes #35956.
How to test the changes in this Pull Request:
Some of the changes in this PR can not be tested yet because they still require an API to be implemented in #34556 and a front-end as described in #34903.
We can test if the changes to
InstalledExtensions
have reverted the Marketing page to its original state (before #35099):GET https://woocommerce.com/wp-json/wccom/marketing-tab/1.2/recommendations.json
API. For example, AutomateWoo, Facebook for WooCommerce, or Pinterest for WooCommerce.Other information:
pnpm --filter=<project> changelog add
?FOR PR REVIEWER ONLY: