Skip to content
This repository was archived by the owner on Oct 4, 2019. It is now read-only.

Add hotfix that prevents the cheque payment method from being auto-enabled. #33

Merged
merged 2 commits into from
Jul 31, 2017

Conversation

emdashcodes
Copy link

See Automattic/wp-calypso#16658 and Automattic/wp-calypso#16663 for some history.

By default WC auto-enables the cheque payment method. We don't want this auto-enabled for Store on WP.com. Luckily, we are provided a filter where we can pass defaults, which are used if settings are not stored for the site (i.e. user hasn't configured the payment method). It also cleans up the default text.

It also adds a constant for easily disabling all these WP.com specific hotfixes.

To Test:

  • Install this branch/plugin over your existing wc-api-dev copy.
  • Unless you just set-up a fresh store, you will need to remove your settings for the cheque payment method. What I did for my AT site, was edit wc-api-dev.php's init using the wp-admin plugin editor and added a delete_option( 'woocommerce_cheque_settings' );. I did a page load, and then removed this line.
  • View the checkout page and make sure the cheque payment method is not displayed.
  • Go to http://calypso.localhost:3000/store/settings/:site and enable the cheque payment method. Under setup, you should see the defaults.
  • View the checkout page and make sure the cheque payment method is displayed.
  • Run phpunit and make sure all tests pass (just to be safe).

Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Looks good, worked in testing. Just one comment about the textdomain - if using a different textdomain for hotfixes is intentional, it should be documented somewhere.


function wc_api_dev_cheque_defaults( $settings ) {
$settings['enabled']['default'] = 'no';
$settings['description']['default'] = __( 'Pay for this order by check.', 'wc-api-dev' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean for the textdomain to be wc-api-dev, rather than woocommerce?

Copy link
Author

Choose a reason for hiding this comment

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

@ryelle So that's a good question..

For the other strings in the plugin, the strings are in WooCommerce Core already. These strings are new so they would have to be translated separately. They also won't ever be merged back to WooCommerce core like the API strings.

I think we will be breaking these types of changes out of the API plugin very shortly (see the latest thread on our P2: p90Yrv-1K).. so those would have a separate text-domain anyway, and a separate sub-project here, so they could be translated prior to launching to more users on .com. So in that case, I think it makes sense to use a different text-domain for these changes. I can document that in the README. Does that sound good?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK — I thought it would be something like that, but without a note I wasn't sure. Documenting in the readme sounds good 👍

@emdashcodes emdashcodes merged commit c796c3d into master Jul 31, 2017
@emdashcodes emdashcodes deleted the update/cheque-defaults branch July 31, 2017 22:16
@emdashcodes emdashcodes restored the update/cheque-defaults branch August 1, 2017 18:15
emdashcodes pushed a commit that referenced this pull request Aug 1, 2017
emdashcodes pushed a commit that referenced this pull request Aug 1, 2017
@emdashcodes emdashcodes mentioned this pull request Aug 1, 2017
@emdashcodes emdashcodes deleted the update/cheque-defaults branch August 1, 2017 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants