[CIAB: POS] Hide collect payment button for refunded bookings#15402
[CIAB: POS] Hide collect payment button for refunded bookings#15402samiuelson wants to merge 1 commit intotrunkfrom
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #15402 +/- ##
=========================================
Coverage 39.29% 39.29%
- Complexity 10977 10981 +4
=========================================
Files 2238 2238
Lines 127540 127545 +5
Branches 17802 17803 +1
=========================================
+ Hits 50113 50120 +7
+ Misses 72318 72316 -2
Partials 5109 5109 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| else -> false | ||
| } | ||
|
|
||
| private fun isBookingRefunded( |
There was a problem hiding this comment.
Small naming suggestion: isBookingPaid / isBookingRefunded sound like they only check the booking status, but they actually check a combination of booking status + payment status. This is a bit misleading.
Proposal:
// Methods
isBookingPaid(...) → hasPaymentBeenCollected(...)
isBookingRefunded(...) → hasPaymentBeenRefunded(...)
// Local variables
val isPaid = ... → val isPaymentCollected = ...
val isRefunded = ... → val isPaymentRefunded = ...
There was a problem hiding this comment.
@samiuelson thinking further -
Suggestion: instead of two boolean methods (isBookingPaid + isBookingRefunded), we could use a single method that returns an enum. The two booleans are really three mutually exclusive states: paid, refunded, or unpaid.
private enum class PaymentState { Paid, Refunded, Unpaid }
private fun resolvePaymentState(
bookingStatus: BookingEntity.Status,
paymentStatus: PaymentStatus,
): PaymentState = when (bookingStatus) {
BookingEntity.Status.Paid, BookingEntity.Status.Complete -> PaymentState.Paid
BookingEntity.Status.Cancelled -> when (paymentStatus) {
PaymentStatus.PAID -> PaymentState.Paid
PaymentStatus.REFUNDED -> PaymentState.Refunded
else -> PaymentState.Unpaid
}
else -> PaymentState.Unpaid
}
Then usage becomes:
// actions list
val paymentState = resolvePaymentState(booking.status, paymentStatus)
if (paymentState == PaymentState.Paid) {
add(WooPosBookingsState.BookingAction.IssueRefund(booking.orderId))
}
// payment section
val paymentState = resolvePaymentState(booking.status, paymentStatus)
paidWithLabel = if (paymentState == PaymentState.Paid) paymentInfo?.paymentMethodTitle else null,
collectPaymentLabel = if (paymentState == PaymentState.Unpaid) totalAmount else null,This removes the two boolean methods, makes the states explicit, and avoids the !isPaid && !isRefunded check.
kidinov
left a comment
There was a problem hiding this comment.
LGTM - I left a code improvement suggestion

Description
WOOMOB-2285
When a booking is in "Cancelled" status with a "Refunded" payment status, the "Collect Payment" button was still being shown in the POS booking detail screen. This PR hides the collect payment button for refunded bookings, since collecting payment on an already-refunded booking doesn't make sense.
Adds an
isBookingRefundedcheck alongside the existingisBookingPaidcheck so thatcollectPaymentLabelis set tonullwhen the booking is both cancelled and refunded.Test Steps
Images/gif
Cancelled && Refunded
Cancelled
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.