-
Notifications
You must be signed in to change notification settings - Fork 74
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
🎯 fix: POS discount coupons were accessible from single order page on admin dashboard #159
Conversation
WalkthroughThe changes involve enhancements to the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 0
🧹 Outside diff range and nitpick comments (2)
includes/Admin/Discounts.php (1)
148-156
: LGTM: New method effectively prevents access to POS discount coupons.The
remove_discount_coupon_url
method successfully addresses the PR objective by replacing the coupon URL with '#' for POS orders. This prevents access to the coupon editing screen for automatically generated coupons used in POS transactions.Consider a minor refactoring for improved readability:
public function remove_discount_coupon_url( $url, $item, $order ) { - if ( ! $order->get_meta( '_wepos_is_pos_order' ) ) { - return $url; - } - - $url = '#'; - - return $url; + return $order->get_meta( '_wepos_is_pos_order' ) ? '#' : $url; }This change maintains the same functionality while making the code more concise.
assets/src/frontend/components/Home.vue (1)
909-910
: Remove Unnecessary Empty LinesThere are unnecessary empty lines at the end of the
processPayment()
method. Removing them can help keep the code clean and maintain consistent formatting.Apply this diff to remove the extra lines:
- 909~ - 910~
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- assets/src/frontend/components/Home.vue (2 hunks)
- includes/Admin/Discounts.php (2 hunks)
🔇 Additional comments (3)
includes/Admin/Discounts.php (2)
37-37
: LGTM: Constructor change aligns with PR objective.The addition of the
woocommerce_admin_order_item_coupon_url
filter in the constructor is appropriate. It hooks theremove_discount_coupon_url
method, which will help prevent access to automatically generated coupons in the admin dashboard, aligning with the PR's objective.
Line range hint
1-156
: Summary: Changes effectively address the PR objective.The modifications to the
Discounts
class successfully implement the required functionality to prevent access to automatically generated coupons for POS orders in the admin dashboard. The newremove_discount_coupon_url
method, coupled with the added filter in the constructor, provides a clean and effective solution to the issue described in the PR.The implementation is well-structured, follows good coding practices, and integrates seamlessly with the existing codebase. These changes should resolve the problem of POS discount coupons being accessible from the single order page on the admin dashboard.
assets/src/frontend/components/Home.vue (1)
892-892
: 🛠️ Refactor suggestionRefactor Suggestion: Consolidate
$contentWrap.unblock();
to Reduce DuplicationThe
$contentWrap.unblock();
function is called in both theif
andelse
blocks of the.done()
handler, leading to duplicate code. Consider moving$contentWrap.unblock();
outside theif-else
block to ensure it executes regardless of the condition. This enhances readability and maintainability.Apply this diff to refactor the code:
wepos.api.post( wepos.rest.root + wepos.rest.posversion + '/payment/process', response ) .done( data => { if ( data.result == 'success' ) { this.$router.push({ name: 'Home', query: { order_key: response.order_key, payment: 'success' } }); } else { // Handle non-success case if necessary } + $contentWrap.unblock(); }) .fail( data => { $contentWrap.unblock(); alert( data.responseJSON.message ); });
Likely invalid or redundant comment.
Summary by CodeRabbit
New Features
Bug Fixes
Chores