-
Notifications
You must be signed in to change notification settings - Fork 8
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
Prevent password protected notice being triggered by woocommerse checkout page scan #842
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces enhancements to the plugin's WooCommerce integration by adding two new helper functions in the Changes
Sequence DiagramsequenceDiagram
participant Validate as edac_validate()
participant Helper as Helper Functions
participant WooCommerce as WooCommerce System
Validate->>Helper: Check if WooCommerce enabled
Helper->>WooCommerce: Verify WooCommerce status
Helper-->>Validate: Return WooCommerce status
Validate->>Helper: Check if post is checkout page
Helper->>WooCommerce: Confirm checkout page
Helper-->>Validate: Return checkout page status
alt Is Checkout Page
Validate->>Validate: Skip password protection
else Not Checkout Page
Validate->>Validate: Continue normal validation
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 phpstan (2.0.3)includes/helper-functions.php1065-1065: Function wc_get_page_id not found. (function.notFound) 🔇 Additional comments (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
includes/validate.php (1)
149-156
: Correct the WooCommerce references and ensure future enhancement tasks are tracked.
- The inline comment text repeatedly spells "WooCommerce" as "woocommerse."
- This block provides a temporary fix ("TEMPORARY FIX") to return early when content is empty for the checkout page. Ensure that you've opened a follow-up issue or have plans to implement the final approach (automating a test cart item insertion before scanning, etc.)
As a small improvement, here’s a suggested diff to fix the spelling and prepare for a future rename of the helper function if needed:
-// The woocommerse checkout page will always be a redirect when it has no items. ... +// The WooCommerce checkout page will always be a redirect when it has no items. ... // TEMPORARY FIX: ... -if ( edac_check_if_post_id_is_woocommerse_checkout_page( $post_ID ) ) { +if ( edac_check_if_post_id_is_woocommerce_checkout_page( $post_ID ) ) { return; }I can open a new issue to track the permanent fix for scanning the checkout page.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/helper-functions.php
(1 hunks)includes/validate.php
(1 hunks)
🧰 Additional context used
🪛 phpstan (2.0.3)
includes/helper-functions.php
1061-1061: Function edacp_is_woocommerse_enabled not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
1065-1065: Function wc_get_page_id not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check code style
includes/helper-functions.php
Outdated
/** | ||
* Check if WooCommerce is enabled. | ||
* | ||
* This just checks for existance of the main WooCommerce function and class. | ||
* | ||
* @return bool | ||
*/ | ||
function edac_is_woocommerse_enabled() { | ||
return function_exists( 'WC' ) && class_exists( 'WooCommerce' ); | ||
} |
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.
🛠️ Refactor suggestion
Rename function to avoid confusion and align with standard WooCommerce spelling.
"woocommerse" is misspelled, and using consistent naming helps with readability. Here’s a suggested diff to rename the function:
-/**
- * Check if WooCommerce is enabled.
- *
- * This just checks for existance of the main WooCommerce function and class.
- *
- * @return bool
- */
-function edac_is_woocommerse_enabled() {
+/**
+ * Check if WooCommerce is enabled.
+ *
+ * This just checks for existence of WooCommerce’s main function and class.
+ *
+ * @return bool
+ */
+function edac_is_woocommerce_enabled() {
return function_exists( 'WC' ) && class_exists( 'WooCommerce' );
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/** | |
* Check if WooCommerce is enabled. | |
* | |
* This just checks for existance of the main WooCommerce function and class. | |
* | |
* @return bool | |
*/ | |
function edac_is_woocommerse_enabled() { | |
return function_exists( 'WC' ) && class_exists( 'WooCommerce' ); | |
} | |
/** | |
* Check if WooCommerce is enabled. | |
* | |
* This just checks for existence of WooCommerce's main function and class. | |
* | |
* @return bool | |
*/ | |
function edac_is_woocommerce_enabled() { | |
return function_exists( 'WC' ) && class_exists( 'WooCommerce' ); | |
} |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/helper-functions.php (1)
1043-1052
: Fix typo in function documentation.There's a typo in the docblock: "existance" should be "existence".
/** * Check if WooCommerce is enabled. * - * This just checks for existance of the main WooCommerce function and class. + * This just checks for existence of the main WooCommerce function and class. * * @return bool */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/helper-functions.php
(1 hunks)includes/validate.php
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/validate.php
🧰 Additional context used
🪛 phpstan (2.0.3)
includes/helper-functions.php
1065-1065: Function wc_get_page_id not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
🔇 Additional comments (1)
includes/helper-functions.php (1)
1054-1066
: LGTM! Implementation is correct and safe.The function properly checks for WooCommerce availability before attempting to use WooCommerce-specific functions. The static analysis warning about
wc_get_page_id
can be safely ignored as the function call is properly guarded byedac_is_woocommerce_enabled()
.🧰 Tools
🪛 phpstan (2.0.3)
1065-1065: Function wc_get_page_id not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols(function.notFound)
This PR detects when a scan fail occurs on the woocommerse checkout page (due to a redirect when cart is empty) and returns early before the
edac_password_protected
option is set.The page still will not scan after this change - but it will not show an inaccurate notice when it occurs.
We will need to cycle back in future to resolve actually being able to scan the checkout page in different situations.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes