-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat(ia): audience checkout/payment and donations wizard #3564
Conversation
|
||
/** | ||
* Prevent rendering of Donate block if Reader Revenue platform is set to 'other. | ||
* | ||
* @param string $block_content The block content about to be rendered. | ||
* @param array $block The data of the block about to be rendered. | ||
*/ | ||
public static function prevent_rendering_donate_block( $block_content, $block ) { | ||
if ( | ||
isset( $block['blockName'] ) | ||
&& 'newspack-blocks/donate' === $block['blockName'] | ||
&& self::is_platform_other() | ||
) { | ||
return ''; | ||
} | ||
return $block_content; | ||
} |
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.
I moved this from the wizard class because it's not relevant for the wizard, it makes more sense to live in the more general Donations class.
// Clean up the notification when unmounting. | ||
return () => window.localStorage.removeItem( HANDOFF_KEY ); |
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.
This was removed because it instantly removed the handoff message when redirecting from a different tab in the same wizard. That happens because it unmounts the component from the origin tab, given it's a rerender on the same react app.
export const useWizardData = ( wizardName, defaultValue = {} ) => | ||
useSelect( select => | ||
select( WIZARD_STORE_NAMESPACE ).getWizardAPIData( `newspack-${ wizardName }-wizard` ) | ||
export const useWizardData = ( wizardName, defaultValue = {} ) => { | ||
return useSelect( select => | ||
select( WIZARD_STORE_NAMESPACE ).getWizardAPIData( wizardName ) | ||
) || defaultValue; | ||
}; |
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.
I refactored this util so wizards can have different slugs outside newspack-{slug}-wizard
, which is the case across the IA project.
It should now receive the same argument as updateWizardSettings()
and saveWizardSettings()
, standardizing it when using the store inside components.
{ hasPaymentGateway && ( | ||
<AdditionalSettings | ||
settings={ settings } | ||
/> | ||
) } |
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.
Moved to the Audience Donations wizard
@@ -42,43 +43,8 @@ const FREQUENCIES: { | |||
}; | |||
const FREQUENCY_SLUGS: FrequencySlug[] = Object.keys( FREQUENCIES ) as FrequencySlug[]; | |||
|
|||
type WizardData = { |
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.
Moved to src/wizards/types/hooks.d.ts
@@ -10,7 +10,7 @@ import { useDispatch } from '@wordpress/data'; | |||
import { PluginToggle, ActionCard, Wizard } from '../../../../components/src'; | |||
|
|||
const Intro = () => { | |||
const settingsData = Wizard.useWizardData( 'settings' ); | |||
const settingsData = Wizard.useWizardData( 'newspack-settings-wizard' ); |
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.
This was the only wizard, other than "Reader Revenue", that used the wizard store and had to update to the new useWizardData()
protected $api_route = '/newspack/v1/wizards/reader-revenue'; | ||
protected $api_route = '/newspack/v1/wizards/site-design'; |
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.
Had to change to a different one because reader-revenue
no longer exists
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.
@miguelpeixe thank you for this, a ton of effort went into it.
Functionally it works as described in "How to test the changes in this Pull Request:". I tested+reviewed as thoroughly as I could. Made a few suggestions throughout, nothing too scary.
$wc_configuration_manager = Configuration_Managers::configuration_manager_class_for_plugin_slug( 'woocommerce' ); | ||
$wc_installed = 'active' === Plugin_Manager::get_managed_plugin_status( 'woocommerce' ); | ||
$stripe_data = Stripe_Connection::get_stripe_data(); |
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.
Last 3 declarations are unused, can we remove?
$wc_configuration_manager = Configuration_Managers::configuration_manager_class_for_plugin_slug( 'woocommerce' ); | |
$wc_installed = 'active' === Plugin_Manager::get_managed_plugin_status( 'woocommerce' ); | |
$stripe_data = Stripe_Connection::get_stripe_data(); |
* @package Newspack | ||
*/ | ||
|
||
namespace Newspack; |
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.
namespace Newspack; | |
namespace Newspack; | |
use WP_Error; |
public function get_payment_data() { | ||
$platform = Donations::get_platform_slug(); | ||
$wc_configuration_manager = Configuration_Managers::configuration_manager_class_for_plugin_slug( 'woocommerce' ); | ||
$wc_installed = 'active' === Plugin_Manager::get_managed_plugin_status( 'woocommerce' ); |
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.
Is unused.
$wc_installed = 'active' === Plugin_Manager::get_managed_plugin_status( 'woocommerce' ); |
import { AUDIENCE_DONATIONS_WIZARD_SLUG, NEWSPACK, OTHER } from '../../constants'; | ||
|
||
const AudienceDonations = () => { | ||
const { platform_data, donation_data } = Wizard.useWizardData( 'newspack-audience-donations' ); |
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.
const { platform_data, donation_data } = Wizard.useWizardData( 'newspack-audience-donations' ); | |
const { platform_data, donation_data } = Wizard.useWizardData( AUDIENCE_DONATIONS_WIZARD_SLUG ); |
|
||
const Salesforce = () => { | ||
const { salesforce_redirect_url: redirectUrl } = window?.newspack_reader_revenue || {}; | ||
const { salesforce_redirect_url: redirectUrl } = window?.newspackAudienceDonations || {}; |
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.
const { salesforce_redirect_url: redirectUrl } = window?.newspackAudienceDonations || {}; | |
const { salesforce_redirect_url: redirectUrl } = window?.newspackAudience || {}; |
Thank you for the review, @jaredrethman! Quite the diff you went through 🙇 Applied all your suggestions in 8b92d15 |
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.
Nice work here @miguelpeixe 🎉 , approved!
All Submissions:
Changes proposed in this Pull Request:
Implements:
Most of the effort around this PR is decoupling logic from the existing "Reader Revenue" wizard and placing in different sections of the Audience wizard:
How to test the changes in this Pull Request:
wp option delete newspack_reader_activation_enabled
)wp option update newspack_salesforce_client_id 123
), refresh and confirm the Salesforce settings renderOther information: