Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WooPayments Compatibility #142

Merged
merged 11 commits into from
Aug 2, 2024
Merged

WooPayments Compatibility #142

merged 11 commits into from
Aug 2, 2024

Conversation

iamdharmesh
Copy link
Member

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

PR adds compatibility with the WooPayments extension. it contains the following changes.

  • Prevent the express pay buttons from displaying on Square gift card product pages.
  • Prevent the express pay buttons from displaying on the cart and checkout page if the card contains a Square gift card product.

Apart from these compatibility changes PR also fixes one issue around split payments to ensure in case of split payment with the gift card only supported payment methods should be displayed.

Closes #121

Steps to test the changes in this Pull Request:

  1. Set up WooPayments extension.
  2. Enable Square extension.
  3. Create and publish a simple product
    a. Visit the product page: ensure all the express pay buttons appear
    b. Place the item in the cart: visit the cart and ensure all the express pay buttons appear
    c. Place the item in the cart: visit the checkout page and ensure all the express pay buttons appear
  4. Create and publish a Gift card product
    a. Visit the product page: ensure no express pay buttons appear from the WooPayments
    b. Place the item in the cart: visit the cart and no express pay buttons appear from the WooPayments
    c. Place the item in the cart: visit the checkout page and no express pay buttons and no payment methods appear from the WooPayments
  5. Add a simple product to cart, and make sure it has a price more than $1 (Gift card balance)
  6. Go to the checkout page.
  7. Add a Gift card on the checkout page and verify that only split payment-supported payment methods are displayed there.

For overall compatibility do the smoke testing on the square extension.

Changelog entry

Fix - Ensure compatibility with WooPayments extension.

@iamdharmesh iamdharmesh self-assigned this May 31, 2024
@iamdharmesh iamdharmesh added this to the Future Release milestone May 31, 2024
@iamdharmesh iamdharmesh mentioned this pull request May 31, 2024
Closed
@iamdharmesh iamdharmesh changed the title [WIP] WooPayments Compatibility WooPayments Compatibility Jun 12, 2024
@iamdharmesh iamdharmesh requested a review from dkotter June 12, 2024 06:04
@qasumitbagthariya
Copy link
Contributor

qasumitbagthariya commented Jun 14, 2024 •
edited
Loading

QA Update ✅

I have checked this WooPayment compatibility in the enhancement/121 branch and it's working as expected.

Express pay buttons on the product page ✅

image

Express pay button on the block cart page ✅

image

Express pay button on the shortcode cart page ✅

image

Giftcard product ✅

image

Giftcard product on cart page - Express pay buttons do not appear ✅

image
image

Giftcard product on checkout page - Express pay buttons do not appear ✅

image

Split Payment Credit card + Giftcard ✅

Testing Environment

  • WordPress: 6.5.4
  • Theme: Twenty Twenty-Four 1.1
  • WooCommerce - 8.9.3
  • WooPayments - 7.7.0
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for code review (Woo)
Testing Document status:
Cases related to this Issue/PR are added to the Critical Flow Wiki pages:

  • Yes
  • Not Required/Applicable for this PR

@qasumitbagthariya qasumitbagthariya requested review from a team and james-allan and removed request for a team June 14, 2024 07:42
@vikrampm1
Copy link
Contributor

@james-allan this PR is ready for your review.

@jeffpaul
Copy link
Contributor

jeffpaul commented Jul 1, 2024

@diegocurbelo @james-allan checking in on Woo code review, let us know if we should assign elsewhere?

@diegocurbelo diegocurbelo self-requested a review July 2, 2024 00:13
james-allan
james-allan approved these changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@james-allan james-allan left a comment

Choose a reason for hiding this comment

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

Changes look good an tested well. I left some non-blocking feedback.

includes/Gateway/Templates/payment.php
@@ -32,7 +32,7 @@
if ( ! empty( $available_gateways ) ) {
$plugin_gateways = wc_square()->get_gateway_ids();
foreach ( $available_gateways as $gateway ) {
if ( ! $has_balance && ! in_array( Plugin::GATEWAY_ID, $plugin_gateways, true ) ) {
if ( ! $has_balance && ! in_array( $gateway->id, $plugin_gateways, true ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not something we need to look into right now but this logic seems a little odd. $has_balance doesn't change between each looped element and so this whole loop could be avoided if ! $has_balance.

Copy link
Member Author

Choose a reason for hiding this comment

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

I quickly checked on this, and it doesn't look quite straightforward. @vikrampm1, could you please help by creating an issue for this? This way, the fix can go through proper testing. Thank you.

cc: @Sidsector9

includes/WC_Payments_Compatibility.php Show resolved Hide resolved
includes/WC_Payments_Compatibility.php Outdated Show resolved Hide resolved
includes/WC_Payments_Compatibility.php Outdated Show resolved Hide resolved
@iamdharmesh
Copy link
Member Author

Thanks for the review @james-allan. I have made suggested changes and responded to the feedback comments.

Thank you.

@vikrampm1 vikrampm1 mentioned this pull request Jul 3, 2024
Open
diegocurbelo
diegocurbelo approved these changes Jul 19, 2024
View reviewed changes
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

Code changes look good, and it tests well 🚢

@qasumitbagthariya
Copy link
Contributor

Regression / Smoke Test Report ✅

Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.5.5, npm version 10.7.0, node version 20.15.1)

Status- Working expected with Plugin Archive/Zip file same as fix specific branch.

Testing Environment

  • WordPress: 6.6.1
  • Theme: Twenty Twenty-Four 1.2
  • WooCommerce - 9.1.4
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3

Next Step- Ready to Merge 🚀

@vikrampm1 vikrampm1 modified the milestones: Future Release, 4.7.3 Aug 2, 2024
@vikrampm1 vikrampm1 marked this pull request as ready for review August 2, 2024 14:42
@vikrampm1 vikrampm1 merged commit fea17e1 into trunk Aug 2, 2024
5 of 6 checks passed
@vikrampm1 vikrampm1 deleted the enhancement/121 branch August 2, 2024 14:44
@vikrampm1 vikrampm1 mentioned this pull request Aug 2, 2024
Open
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegocurbelo diegocurbelo diegocurbelo approved these changes

@dkotter dkotter dkotter approved these changes

@jeffpaul jeffpaul jeffpaul approved these changes

@james-allan james-allan james-allan approved these changes

@qasumitbagthariya qasumitbagthariya qasumitbagthariya approved these changes

Assignees

@iamdharmesh iamdharmesh

Labels
needs: e2e testing
Projects
None yet
Milestone
4.7.3
Development

Successfully merging this pull request may close these issues.

Check compatibility with WooPayments
7 participants
@iamdharmesh @qasumitbagthariya @vikrampm1 @jeffpaul @diegocurbelo @dkotter @james-allan