Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Hide saved payment methods if their gateway is disabled #2975

Merged
merged 7 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,17 @@ const SavedPaymentMethodOptions = ( { onSelect } ) => {
);
}
} );
currentOptions.current = options;
currentOptions.current.push( {
value: '0',
label: __(
'Use a new payment method',
'woo-gutenberg-product-blocks'
),
name: `wc-saved-payment-method-token-new`,
} );
if ( options.length > 0 ) {
currentOptions.current = options;
currentOptions.current.push( {
value: '0',
label: __(
'Use a new payment method',
'woo-gutenberg-product-blocks'
),
name: `wc-saved-payment-method-token-new`,
} );
}
}
}, [
customerPaymentMethods,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,33 @@ export const usePaymentMethodDataContext = () => {
return useContext( PaymentMethodDataContext );
};

/**
* Gets the payment methods saved for the current user after filtering out
* disabled ones.
*
* @param {Object[]} availablePaymentMethods List of available payment methods.
* @return {Object} Object containing the payment methods saved for a specific
* user which are available.
*/
const getCustomerPaymentMethods = ( availablePaymentMethods = [] ) => {
const customerPaymentMethods = getSetting( 'customerPaymentMethods', {} );
const paymentMethodKeys = Object.keys( customerPaymentMethods );
if ( paymentMethodKeys.length === 0 ) {
return {};
}
const enabledCustomerPaymentMethods = {};
paymentMethodKeys.forEach( ( type ) => {
enabledCustomerPaymentMethods[ type ] = customerPaymentMethods[
type
].filter( ( paymentMethod ) => {
return Object.keys( availablePaymentMethods ).includes(
paymentMethod.method.gateway
);
} );
} );
return enabledCustomerPaymentMethods;
};

/**
* PaymentMethodDataProvider is automatically included in the
* CheckoutDataProvider.
Expand Down Expand Up @@ -107,10 +134,6 @@ export const PaymentMethodDataProvider = ( { children } ) => {
const currentObservers = useRef( observers );

const { isEditor, previewData } = useEditorContext();
const customerPaymentMethods =
isEditor && previewData?.previewSavedPaymentMethods
? previewData?.previewSavedPaymentMethods
: getSetting( 'customerPaymentMethods', {} );
const [ paymentData, dispatch ] = useReducer(
reducer,
DEFAULT_PAYMENT_DATA
Expand Down Expand Up @@ -150,6 +173,24 @@ export const PaymentMethodDataProvider = ( { children } ) => {
[ dispatch ]
);

const customerPaymentMethods = useMemo( () => {
if ( isEditor && previewData.previewSavedPaymentMethods ) {
return previewData.previewSavedPaymentMethods;
}
if (
! paymentMethodsInitialized ||
paymentData.paymentMethods.length === 0
) {
return {};
}
return getCustomerPaymentMethods( paymentData.paymentMethods );
}, [
isEditor,
previewData.previewSavedPaymentMethods,
paymentMethodsInitialized,
paymentData.paymentMethods,
] );

const setExpressPaymentError = useCallback(
( message ) => {
if ( message ) {
Expand Down
6 changes: 3 additions & 3 deletions assets/js/type-defs/contexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,9 @@
/**
* @typedef {Object} EditorDataContext
*
* @property {number} isEditor Indicates whether in the editor context.
* @property {number} currentPostId The post ID being edited.
* @property {Object} previewData Object containing preview data for the editor.
* @property {boolean} isEditor Indicates whether in the editor context.
* @property {number} currentPostId The post ID being edited.
* @property {Object} previewData Object containing preview data for the editor.
*/

/**
Expand Down
15 changes: 13 additions & 2 deletions src/Payments/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,25 @@ public function add_payment_method_script_dependencies( $dependencies, $handle )
return array_merge( $dependencies, $this->payment_method_registry->get_all_registered_script_handles() );
}

/**
* Returns true if the payment gateway is enabled.
*
* @param object $gateway Payment gateway.
* @return boolean
*/
private function is_payment_gateway_enabled( $gateway ) {
return filter_var( $gateway->enabled, FILTER_VALIDATE_BOOLEAN );
}

/**
* Add payment method data to Asset Registry.
*/
public function add_payment_method_script_data() {
// Enqueue the order of enabled gateways as `paymentGatewaySortOrder`.
if ( ! $this->asset_registry->exists( 'paymentGatewaySortOrder' ) ) {
$available_gateways = WC()->payment_gateways->payment_gateways();
$this->asset_registry->add( 'paymentGatewaySortOrder', array_keys( $available_gateways ) );
$payment_gateways = WC()->payment_gateways->payment_gateways();
$enabled_gateways = array_filter( $payment_gateways, array( $this, 'is_payment_gateway_enabled' ) );
$this->asset_registry->add( 'paymentGatewaySortOrder', array_keys( $enabled_gateways ) );
}
Comment on lines +79 to 98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this break the logic that was implemented as a result of this comment?

If you implement the changes I suggested in an earlier comment, there won't be need for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really: enabled and available are different concepts. For example, COD might be enabled but not available depending on the shipping method.

While this code is no longer needed for this PR, what do you think about keeping it? While I see you discussed that was not an information leak, at the same time I don't see any benefit on exposing disabled payment methods in the frontend, so I think we could keep the filter. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, COD might be enabled but not available depending on the shipping method.

Ahh, right that makes sense. I'm fine with keeping this but you should make sure testing steps for the work done in #2157 are done to cover any unexpected regressions (don't expect any but still...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider testing steps in #2934 already cover any potential regression introduced in this PR. I updated the testing steps in this PR to also point to testing steps in #2934. Do you think we need to add anything else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, if you're happy with that, it works for me!


// Enqueue all registered gateway data (settings/config etc).
Expand Down