Skip to content

[Order Details] Allow failed orders to show receipt if eligible #15558

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f6c670d
Update eligibility based on order status
iamgabrielma Apr 28, 2025
c0b5a70
Handle receipt eligibility when failed order
iamgabrielma Apr 28, 2025
7711057
move funcs to private extension
iamgabrielma Apr 28, 2025
c26e0a8
typo: remove internal comment
iamgabrielma Apr 28, 2025
e8a3690
clean up dev version
iamgabrielma Apr 28, 2025
f353f87
Rename and lint
iamgabrielma Apr 28, 2025
1459a25
Merge branch 'trunk' into issue/woomob-104-allow-failed-orders-to-ope…
iamgabrielma Apr 29, 2025
c3dc2dd
dev and beta version checks no longer needed
iamgabrielma Apr 29, 2025
701421c
Move gateway check to use case
iamgabrielma Apr 29, 2025
26ca6d3
refactor isEligibleForFailedPaymentEmailReceipts to closure-based
iamgabrielma Apr 29, 2025
c055fe5
update test
iamgabrielma Apr 29, 2025
0aa4a1c
Merge branch 'trunk' into issue/woomob-104-allow-failed-orders-to-ope…
iamgabrielma May 6, 2025
66b212a
restore original check for payment eligibility
iamgabrielma May 6, 2025
8f20e0f
lint
iamgabrielma May 6, 2025
c231384
encapsulate meetsOrderStatusRequirement in receipt eligibility use case
iamgabrielma May 6, 2025
af4010d
update protocol
iamgabrielma May 6, 2025
843a4fa
Merge branch 'trunk' into issue/woomob-104-allow-failed-orders-to-ope…
iamgabrielma May 9, 2025
bff46d4
check for order status requirements on eligibility
iamgabrielma May 9, 2025
a1bbfac
restore previous implementation
iamgabrielma May 9, 2025
b7e4771
restore enum name. unnecessary change
iamgabrielma May 9, 2025
56ad1d0
clean up
iamgabrielma May 9, 2025
bb54851
release notes
iamgabrielma May 9, 2025
d2fda6c
remove unused function
iamgabrielma May 14, 2025
9eb0773
rename for consistency
iamgabrielma May 14, 2025
fcbbcb0
structure tests
iamgabrielma May 14, 2025
a04d1d7
more explicit on test name
iamgabrielma May 14, 2025
2b185a6
remove unnecessary check
iamgabrielma May 14, 2025
3862c54
Merge branch 'trunk' into issue/woomob-104-allow-failed-orders-to-ope…
iamgabrielma May 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

22.4
-----
- [*] Payments: Receipts for failed transactions are now available in Order details [https://github.com/woocommerce/woocommerce-ios/pull/15558]
- [*] Order Details: Improvements to textfield capitalization rules [https://github.com/woocommerce/woocommerce-ios/pull/15611]
- [*] POS: Scrolling search results now dismisses the keyboard [https://github.com/woocommerce/woocommerce-ios/pull/15606]
- [*] POS: Product search shows popular products before a search is entered. [https://github.com/woocommerce/woocommerce-ios/pull/15625]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1525,11 +1525,10 @@ extension OrderDetailsDataSource {

@MainActor
private func isEligibleForBackendReceipt() async -> Bool {
guard !isEligibleForPayment else { return false }
return await withCheckedContinuation { continuation in
receiptEligibilityUseCase.isEligibleForBackendReceipts { isEligible in
receiptEligibilityUseCase.isEligibleForReceipt(order.status, onCompletion: { isEligible in
continuation.resume(returning: isEligible)
}
})
}
}

Expand All @@ -1545,15 +1544,6 @@ extension OrderDetailsDataSource {
return refundFound
}

private func isEligibleForBackendReceipt(completion: @escaping (Bool) -> Void) {
guard !isEligibleForPayment else {
return completion(false)
}
receiptEligibilityUseCase.isEligibleForBackendReceipts { isEligibleForReceipt in
completion(isEligibleForReceipt)
}
}

private func updateOrderNoteAsyncDictionary(orderNotes: [OrderNote]) {
orderNoteAsyncDictionary.clear()
for orderNote in orderNotes {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ protocol ReceiptEligibilityUseCaseProtocol {
func isEligibleForBackendReceipts(onCompletion: @escaping (Bool) -> Void)
func isEligibleForSuccessfulPaymentEmailReceipts(onCompletion: @escaping (Bool) -> Void)
func isEligibleForFailedPaymentEmailReceipts(paymentGatewayID: String, onCompletion: @escaping (Bool) -> Void)
func isEligibleForReceipt(_ orderStatus: OrderStatusEnum, onCompletion: @escaping (Bool) -> Void)
}

final class ReceiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol {
Expand All @@ -27,15 +28,10 @@ final class ReceiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol {
guard let wcPlugin = wcPlugin, wcPlugin.active else {
return onCompletion(false)
}
// 2. If WooCommerce version is any of the specific API development branches, mark as eligible
if Constants.BackendReceipt.wcPluginDevVersion.contains(wcPlugin.version) {
onCompletion(true)
} else {
// 3. Else, if WooCommerce version is higher than minimum required version, mark as eligible
let isSupported = VersionHelpers.isVersionSupported(version: wcPlugin.version,
minimumRequired: Constants.BackendReceipt.wcPluginMinimumVersion)
onCompletion(isSupported)
}
// 2. If WooCommerce version is higher than minimum required version, mark as eligible
let isSupported = VersionHelpers.isVersionSupported(version: wcPlugin.version,
minimumRequired: Constants.BackendReceipt.wcPluginMinimumVersion)
onCompletion(isSupported)
}
stores.dispatch(action)
}
Expand Down Expand Up @@ -90,6 +86,26 @@ final class ReceiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol {
onCompletion(isWooCommerceSupported && isGatewaySupported)
}
}

func isEligibleForReceipt(_ orderStatus: OrderStatusEnum, onCompletion: @escaping (Bool) -> Void) {
switch orderStatus {
case .completed, .processing, .refunded:
isEligibleForBackendReceipts { isEligibleForReceipt in
onCompletion(isEligibleForReceipt)
}
case .failed:
selectedPaymentGatewayID { [weak self] gatewayID in
guard let gatewayID else {
return onCompletion(false)
}
self?.isEligibleForFailedPaymentEmailReceipts(paymentGatewayID: gatewayID) { isEligibleForReceipt in
onCompletion(isEligibleForReceipt)
}
}
default:
return onCompletion(false)
}
}
}

private extension ReceiptEligibilityUseCase {
Expand All @@ -102,11 +118,6 @@ private extension ReceiptEligibilityUseCase {
return continuation.resume(returning: false)
}

// Checking for concrete versions to cover dev and beta versions
if plugin.version.contains(minimumVersion) {
return continuation.resume(returning: true)
}

Comment on lines -105 to -109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ What are the reasons for removing the check that supports dev and beta versions?
Saw this mentioned in the PR description:

Removed the logic to handle dev woocommerce versions

When testing with Woo Beta Tester plugin or for core development, I thought it's common to have the WC version as the dev/beta versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was very specific to this project: The internals of VersionHelpers used in the same function already checks for beta and dev version that might come with Woo Beta Tester, so it will pass the check as expected. This additional check was put in place just for testing, due to how the changes were provided by core folks with specific development branches to be installed in the test site, which sometimes were behind the latest versions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually having a similar plugin version check in a separate PR and removing this check blocks -beta/-dev/-alpha/-rc. From the test cases for the VersionHelpers, this blocking behavior also seems intentional:

VersionTestCase(foundVersion: "1.0.0-dev", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-alpha", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-a", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-beta", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-b", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-RC1", requiredMinimumVersion: "1.0.0", meetsMinimum: false),
VersionTestCase(foundVersion: "1.0.0-rc1", requiredMinimumVersion: "1.0.0", meetsMinimum: false),

If we want to support alpha/beta/dev/rc builds of the same version, I think we do need to keep this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to support alpha/beta/dev/rc builds of the same version

Interesting, I see what you mean: Anything tagged as -dev or other for the same version we're checking, is always below the non-tagged version:

po VersionHelpers.isVersionSupported(version: "9.0-dev", minimumRequired: "9.0.0")
false

po VersionHelpers.isVersionSupported(version: "9.0.1-dev", minimumRequired: "9.0.0")
true

// If plugin version is higher than minimum required version, mark as eligible
let isSupported = VersionHelpers.isVersionSupported(version: plugin.version,
minimumRequired: minimumVersion)
Expand All @@ -115,6 +126,15 @@ private extension ReceiptEligibilityUseCase {
stores.dispatch(action)
}
}

// Returns the current payment gateway ID, needed when checking if a transaction is eligible for receipts
// based on which plugin and versions are active
private func selectedPaymentGatewayID(onCompletion: @escaping (String?) -> Void) {
let action = CardPresentPaymentAction.selectedPaymentGatewayAccount { paymentGatewayAccount in
onCompletion(paymentGatewayAccount?.gatewayID)
}
stores.dispatch(action)
}
}

private extension ReceiptEligibilityUseCase {
Expand All @@ -123,7 +143,6 @@ private extension ReceiptEligibilityUseCase {

enum BackendReceipt {
static let wcPluginMinimumVersion = "8.7.0"
static let wcPluginDevVersion: [String] = ["8.7.0-dev", "8.6.0-dev"]
}

enum ReceiptAfterPayment {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import enum Yosemite.OrderStatusEnum
@testable import WooCommerce

final class MockReceiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol {
var isEligibleForBackendReceipts: Bool = true
var isEligibleForSuccessfulPaymentEmailReceipts: Bool = false
var isEligibleForFailedPaymentEmailReceipts: Bool = false
var isEligibleForReceipt: Bool = true

func isEligibleForBackendReceipts(onCompletion: @escaping (Bool) -> Void) {
onCompletion(isEligibleForBackendReceipts)
Expand All @@ -16,4 +18,8 @@ final class MockReceiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol {
func isEligibleForFailedPaymentEmailReceipts(paymentGatewayID: String, onCompletion: @escaping (Bool) -> Void) {
onCompletion(isEligibleForFailedPaymentEmailReceipts)
}

func isEligibleForReceipt(_ orderStatus: OrderStatusEnum, onCompletion: @escaping (Bool) -> Void) {
onCompletion(isEligibleForReceipt)
}
}
Loading