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.

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

Set the payment form context in the session #696

Merged
merged 22 commits into from
Aug 21, 2024

Conversation

agibson-godaddy
Copy link
Contributor

@agibson-godaddy agibson-godaddy commented Aug 16, 2024

Summary

Related: https://github.com/gdcorp-partners/woocommerce-gateway-elavon/pull/103

This sets the page context in the session when rendering the payment forms.

Release: #1234 (release PR)

Details

  • Also copied the EnumTrait from mwc-common
  • Set up an enum for the contexts

QA

If applicable, add specific steps for the reviewer to perform as part of their QA process prior to approving this pull request. Steps should be in a step -> success? format, like below

Setup

Before merge

  • I have confirmed these changes in each supported minor WooCommerce version

@agibson-godaddy agibson-godaddy self-assigned this Aug 16, 2024
@agibson-godaddy agibson-godaddy changed the title Mwc 17264/t c checkout page Set the payment form context in the session Aug 16, 2024
@agibson-godaddy agibson-godaddy marked this pull request as ready for review August 19, 2024 12:19
@nmolham-godaddy
Copy link
Contributor

nmolham-godaddy commented Aug 20, 2024

@agibson-godaddy So, I got carried away a bit here 😅

At first, I wanted to add unit test for the new PaymentFormContextChecker component, so I ended up adding mwc-tests, setup PHPUnit and converting existing codeception unit test to PHPUnit tests along side adding the new class tests

Ref: 34b27ab...dc8f67c

I am not sure why Github doesn't see the 6792-mwcmanaged runner and the job fails straight away instead of the usual hold/pending status until the repo gets added to the pool's repo list. Maybe because this repo is on a different organization level godaddy-wordpress instead of gdcorp-partners ? 🤔

@agibson-godaddy
Copy link
Contributor Author

@agibson-godaddy So, I got carried away a bit here 😅

At first, I wanted to add unit test for the new PaymentFormContextChecker component, so I ended up adding mwc-tests, setup PHPUnit and converting existing codeception unit test to PHPUnit tests along side adding the new class tests

Ref: 34b27ab...dc8f67c

I am not sure why Github doesn't see the 6792-mwcmanaged runner and the job fails straight away instead of the usual hold/pending status until the repo gets added to the pool's repo list. Maybe because this repo is on a different organization level godaddy-wordpress instead of gdcorp-partners ? 🤔

That could definitely be the problem. Runner pools are per-organization, and every repo added is assumed to be part of that org. So we can't even add this repo to our existing runner pool because of that. We'd have to create a separate runner pool.

I don't know for certain that's what's causing the instant error, but it's definitely a problem anyway that we have to fix!

I can take a look at this today to see if I can get it running. Nice work on the tests!!! 🙌

@agibson-godaddy
Copy link
Contributor Author

@nmolham-godaddy Reworked to drop mwc-tests dependency and use hosted runners.

Copy link
Contributor

@nmolham-godaddy nmolham-godaddy left a comment

Choose a reason for hiding this comment

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

LVGTM

@nmolham-godaddy nmolham-godaddy merged commit 54e2799 into release/5.13.0 Aug 21, 2024
5 checks passed
@nmolham-godaddy nmolham-godaddy deleted the mwc-17264/t-c-checkout-page branch August 21, 2024 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants