-
Notifications
You must be signed in to change notification settings - Fork 131
[Shipping labels] Refund request UI #14170
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
Conversation
This commit introduces the `WooShippingLabelRefundViewModel` to manage the logic and state for the shipping label refund screen.
This commit introduces the `WooShippingLabelRefundFragment` which will be used to display the refund screen for shipping labels.
This commit updates the `LoadingScreen` in the split shipment feature. It replaces the Material `CircularProgressIndicator` with the Material3 version and wraps the `CircularProgressIndicator` with a `Surface` to ensure the background is white.
This commit modifies the `StartRefundRequest` event to include the `orderId` and the selected `shipment` data. This information is necessary for initiating the refund process for a specific shipping label.
This commit adds the navigation flow from the `WooShippingLabelCreationFragment` to the `WooShippingLabelRefundRequestFragment`. When a refund is requested for a shipment, the app will now navigate to the refund request screen, passing the `orderId` and `shipment` details.
This commit updates the `GetShipments` class to include `refundableAmount` and `purchaseDate` when mapping a `ShippingLabelDTO` to a `Shipment`.
Instead of just getting its status, fetch the full `ShippingLabelModel`.
This change modifies the `ObserveShippingLabelStatus` use case to include the `refundableAmount` in its emitted result.
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
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.
I intentionally haven’t added any tests for this view model, as it doesn’t contain any meaningful logic. Let me know if you think we should still add them.
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.
I think that's fine for now, but once we handle the networking of the refund, I think having some unit tests for the success and failures won't hurt, WDYT?
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.
I've added in network PR: #14171
if (_viewState.value is ViewState.Loading) { | ||
triggerEvent(ShowSnackbar(R.string.order_refunds_refund_in_progress)) | ||
} else { | ||
triggerEvent(Exit) | ||
} |
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.
This logic was copied from ShippingLabelRefundFragment
.
Box( | ||
modifier = Modifier.fillMaxSize(), | ||
contentAlignment = Alignment.Center | ||
) { | ||
CircularProgressIndicator() | ||
} |
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.
I also updated split screen to fix the background color of the loading composable.
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
...com/woocommerce/android/ui/orders/wooshippinglabels/refund/WooShippingLabelRefundFragment.kt
Dismissed
Show dismissed
Hide dismissed
...n/com/woocommerce/android/ui/orders/wooshippinglabels/refund/WooShippingLabelRefundScreen.kt
Dismissed
Show dismissed
Hide dismissed
...n/com/woocommerce/android/ui/orders/wooshippinglabels/refund/WooShippingLabelRefundScreen.kt
Dismissed
Show dismissed
Hide dismissed
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14170 +/- ##
=========================================
Coverage 37.73% 37.74%
- Complexity 8949 8952 +3
=========================================
Files 1954 1955 +1
Lines 109474 109520 +46
Branches 14357 14364 +7
=========================================
+ Hits 41315 41334 +19
- Misses 64400 64427 +27
Partials 3759 3759 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Nice work @irfano, code looks good
@@ -166,6 +171,13 @@ class WooShippingLabelCreationFragment : BaseFragment(), BackPressListener { | |||
) | |||
} | |||
|
|||
private fun startRefundRequest(orderId: Long, shipment: ShipmentUIModel) { |
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.
np, I would use a verb like show
or open
here, calling it start
makes it seem like this will start the refund right away.
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.
I agree, navigateToRefundRequest
would be a better name. All navigation events were previously prefixed with Start...
in WooShippingLabelCreationViewModel
. I've also renamed them all to start with NavigateTo...
.
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.
I think that's fine for now, but once we handle the networking of the refund, I think having some unit tests for the success and failures won't hurt, WDYT?
Part of WOOMOB-370
Description
This adds the UI for the Refund request screen. The design slightly differs from the Figma file, as I aimed to maintain consistency with other screens, such as the Hazmat selection screen.
Warning
Network integration has not been implemented yet. Tapping the Refund label will result in infinite loading, which is expected at this stage.
Steps to reproduce
The tests that have been performed
Steps above
Images/gif
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.