-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat: Add refund support to Cash payment gateway and refactor initialization #169
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes enhance the payment gateway system in three areas. In the Cash gateway, a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant Cash as Cash Gateway
participant Order as WooCommerce Order System
C->>Cash: Request refund (order_id, amount, reason)
Cash->>Order: Retrieve order (wc_get_order)
Order-->>Cash: Return order details
Cash->>Cash: Check refund eligibility (can_refund_order)
alt Refund eligible
Cash->>Order: Add refund note & update status (REFUNDED)
Cash->>Order: Save order
Cash-->>C: Return refund confirmation
else Refund ineligible
Cash-->>C: Return WP_Error
end
sequenceDiagram
participant WePOS as WePOS Plugin
participant Hook as wepos_loaded Event
participant Manager as Payment Gateway Manager
WePOS->>WePOS: Run init_hooks()
WePOS->>Hook: Register load_payment_gateways callback
WePOS->>WePOS: Run init_plugin() (triggers do_action('wepos_loaded'))
Hook->>WePOS: Invoke load_payment_gateways()
WePOS->>Manager: Instantiate Payment Gateway Manager
WePOS->>WePOS: Set container['gateways'] to Manager instance
Poem
✨ Finishing Touches
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
🧹 Nitpick comments (2)
includes/Gateways/Cash.php (1)
126-153
: Well-implemented refund method following WooCommerce standardsThe implementation follows WooCommerce's standard pattern for payment gateway refunds with proper error handling, order note recording, and status updates.
However, I noticed that line 146 has a PHPCS ignore comment. Consider addressing the underlying issue rather than ignoring it:
- sprintf( __( 'Refunded %1$s - Reason: %2$s', 'woocommerce' ), $amount, $reason ) // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + sprintf( __( 'Refunded %1$s - Reason: %2$s', 'woocommerce' ), $amount, $reason )Also, you might want to improve the refund message by formatting the amount as currency:
- sprintf( __( 'Refunded %1$s - Reason: %2$s', 'woocommerce' ), $amount, $reason ) // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + sprintf( __( 'Refunded %1$s - Reason: %2$s', 'woocommerce' ), wc_price($amount), $reason )wepos.php (1)
355-365
: Well-structured payment gateway loading methodThe new method encapsulates the gateway manager initialization, which improves code organization and allows for better control of when payment gateways are loaded.
There's a placeholder
WEPOS_SINCE
in the docblock at line 358 instead of an actual version number:- * @since WEPOS_SINCE + * @since 1.3.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
includes/Gateways/Cash.php
(3 hunks)includes/Gateways/Manager.php
(1 hunks)wepos.php
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e_api tests
🔇 Additional comments (4)
includes/Gateways/Cash.php (2)
4-5
: Good addition of OrderInternalStatus importNice use of the WooCommerce enum for order status management, which helps maintain consistency and avoid hardcoded status strings.
41-43
: Properly declared refund supportThe addition of the 'refunds' support property correctly indicates to WooCommerce that this gateway can handle refund operations, which is essential for the new refund functionality to work.
includes/Gateways/Manager.php (1)
17-17
: Correctly changed action to filter for payment gateways registrationExcellent change from
add_action
toadd_filter
since thepayment_gateways
method modifies and returns the array of gateways. This properly aligns with WooCommerce's expectation that this hook will filter the gateway list.wepos.php (1)
316-316
: Good addition of payment gateway loading hookAdding this hook improves the plugin structure by making the initialization of payment gateways event-driven.
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/Gateways/Cash.php (1)
126-153
: Successfully implemented WooCommerce-compatible refund processing.The implementation follows WooCommerce's standard pattern for processing refunds including proper order validation, note addition, and status updates.
Consider adding validation for the
$amount
parameter to ensure it's a positive number and doesn't exceed the original order amount, although WooCommerce likely validates this before calling this method.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Gateways/Cash.php
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e_api tests
🔇 Additional comments (3)
includes/Gateways/Cash.php (3)
4-5
: Good use of WooCommerce standard enum for order status.Using the
OrderInternalStatus
enum from WooCommerce's library is the correct approach for consistent order status management.
41-43
: Properly added refund support to the gateway.The addition of refund support to the
$this->supports
property follows WooCommerce's standard practice for enabling refund functionality in a payment gateway.
90-90
: Improved availability check with frontend context.The updated check ensures the gateway is only available in the appropriate context by combining the parent check with
wepos_is_frontend()
.
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/Gateways/Cash.php (1)
130-157
: Implementation of refund functionality with some suggestions.The refund method follows the WooCommerce standard pattern, but consider these improvements:
- The code doesn't validate the order after retrieving it
- There's no validation for the refund amount (should be positive and not exceed order total)
- Translation domain inconsistency: 'woocommerce' is used here while 'wepos' is used elsewhere
- The purpose of the phpcs:ignore comment on line 150 isn't clear
Consider implementing these improvements:
public function process_refund( $order_id, $amount = null, $reason = '' ) { $order = wc_get_order( $order_id ); - if ( ! $this->can_refund_order( $order ) ) { + if ( ! $order || ! $this->can_refund_order( $order ) ) { return new \WP_Error( 'error', __( 'Refund failed.', 'woocommerce' ) ); } + if ( ! is_numeric( $amount ) || $amount <= 0 ) { + return new \WP_Error( 'error', __( 'Refund amount must be greater than zero.', 'wepos' ) ); + } $order->add_order_note( /* translators: 1: Refund amount, 2: Refund reason */ - sprintf( __( 'Refunded %1$s - Reason: %2$s', 'woocommerce' ), $amount, $reason ) // phpcs:ignore WordPress.NamingConventions.UsedPropertyNotSnakeCase + sprintf( __( 'Refunded %1$s - Reason: %2$s', 'wepos' ), $amount, $reason ) ); $order->update_status(OrderInternalStatus::REFUNDED ); $order->save(); return true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Gateways/Cash.php
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e_api tests
🔇 Additional comments (4)
includes/Gateways/Cash.php (4)
4-5
: Good addition of WooCommerce's OrderInternalStatus enum.This import allows the proper use of WooCommerce's standard order status enums, which is a best practice for status management.
11-14
: LGTM: Protected properties declaration.Good practice to explicitly declare class properties. These properties were already used in the constructor, so making them explicit improves code clarity.
45-47
: Appropriate addition of refund support.Adding 'refunds' to the $supports array correctly declares that this payment gateway supports the refund functionality, which is necessary for WooCommerce to enable the refund UI for this gateway.
94-94
: Good context check for gateway availability.Adding the
wepos_is_frontend()
check ensures the gateway is only available in the appropriate WePOS frontend context.
This PR adds refund support to the Cash payment gateway in the wePOS plugin and improves the gateway initialization process.
Changes made:
process_refund()
method for Cash gateway with proper order status updatesadd_action
toadd_filter
for proper WooCommerce integrationRelated Issues
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor