-
Notifications
You must be signed in to change notification settings - Fork 69
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
Fixes undefined constant WC_Install::STORE_ID_OPTION error #10432
Fixes undefined constant WC_Install::STORE_ID_OPTION error #10432
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: 0 B Total Size: 1.29 MB ℹ️ View Unchanged
|
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.
One thing unrelated to your changes is that I'd like to understand in general is why we're hooking onto order processed add_action( 'woocommerce_checkout_order_processed', [ $this, 'checkout_order_processed' ], 10, 2 );
and tracking checkout_order_placed
event from within WooPay tracker class for WooPayments checkout. It doesn't hurt, I just started wondering why the error happens on the checkout even with WooPay disabled.
I will get back with the testing outcome soon.
Yeah, this surprised me a little bit, but it seems that I'd suspect that this is the case because this tracking, in general, was only added with the advent of WooPay for the purposes of specifically tracking WooPay events and consequently this misleading class name has stuck. I think there could be value in giving this class a more meaningful name in the future, but I'd also like to learn a bit more context about why it is named so deceptively in the first place. |
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.
The change makes sense. I've tested locally with WC 7.7.0
develop
- saving settings fails
- checkouts fail
This branch
- saving settings works
- checkouts work
Thanks @FangedParakeet !
Changes proposed in this Pull Request
Fixes undefined constant WC_Install::STORE_ID_OPTION error
This PR fixes the below error due to incorrect usage of the
defined
function in a conditional, prior to including this constant in the body of aWooPay_Tracker
event.\WC_Install::STORE_ID_OPTION
was introduced in WC 8.4, so we must check that this constant is defined for earlier versions of WC, before using this property in the tracker request.This PR also removes some not particularly helpful unit tests and avoids mocking the
WooPay_Tracker
system under test.Testing instructions
store_id
value should now benull
.npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge