Skip to content

Conversation

@atorresveiga
Copy link
Contributor

Closes: #12437

Description

This PR adds the logic to connect the origin addresses request response with the UI. To achieve this goal, I:

  • Created the address data store to cache origin addresses in a separate file and prevent retriggering requests when the store options get refreshed
  • Refactored the store options data store so that the name reflects that the class is in charge of storing the shipping label configuration
  • Created the new package address to group all the classes related to addresses
  • Update the rest client and repository classes to include the request for fetching origin addresses
  • Added the fetch origin addresses use case
  • Added the observe origin addresses use case

Testing information

TC1

  1. Open the orders section
  2. Open an order eligible for shipping label creation
  3. Tap on create shipping label
  4. Check that the origin addresses are fetched from the API
  5. Navigate back
  6. Tap again on create shipping label
  7. Check that the origin addresses are presented from the preferences
  8. Check that the origin addresses are refreshed on background

TC2

  1. Open the orders section
  2. Open an order eligible for shipping label creation
  3. Tap on create shipping label
  4. Check that the origin addresses are fetched from the API
  5. Check that when the API request fails then the error screen is displayed
  • You can apply the below path to force the fetch settings to fail
Patch
Index: WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt
--- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt	(revision 2783940e973d9c96722e7897e87c08da8e9654ae)
+++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt	(date 1735306410793)
@@ -10,20 +10,6 @@
     private val selectedSite: SelectedSite
 ) {
     suspend operator fun invoke(): Result<List<OriginShippingAddress>> {
-        return selectedSite.getOrNull()?.let {
-            val response = shippingRepository.fetchOriginAddresses(it)
-            val result = response.model
-            when {
-                response.isError.not() && !result.isNullOrEmpty() -> {
-                    Result.success(result)
-                }
-
-                else -> {
-                    val message =
-                        response.error?.message ?: if (result.isNullOrEmpty()) "Empty result" else "Unknown error"
-                    Result.failure(Exception(message))
-                }
-            }
-        } ?: Result.failure(Exception("No site selected"))
+        return  Result.failure(Exception("No site selected"))
     }
 }

The tests that have been performed

  • Checked that the UI works as expected when there is no cached data
  • Checked that the UI works as expected when there is cached data
  • Checked that the UI works as expected when refreshing the data fails

Images/gif

Screen_recording_20241227_104206.mp4
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@atorresveiga atorresveiga added type: task An internally driven task. feature: shipping labels Related to creating, ordering, or printing shipping labels. labels Dec 27, 2024
@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commitd668110
Direct Downloadwoocommerce-wear-prototype-build-pr13211-d668110.apk

@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commitd668110
Direct Downloadwoocommerce-prototype-build-pr13211-d668110.apk

@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2024

Codecov Report

Attention: Patch coverage is 25.92593% with 80 lines in your changes missing coverage. Please review.

Project coverage is 40.62%. Comparing base (a7fd6a3) to head (d668110).
Report is 19 commits behind head on trunk.

Files with missing lines Patch % Lines
...oid/ui/orders/wooshippinglabels/networking/DTOs.kt 0.00% 17 Missing ⚠️
...nglabels/networking/WooShippingNetworkingMapper.kt 0.00% 17 Missing ⚠️
...inglabels/networking/WooShippingLabelRepository.kt 0.00% 15 Missing ⚠️
...nglabels/datasource/WooShippingAddressDataStore.kt 0.00% 11 Missing ⚠️
...ooshippinglabels/address/ObserveOriginAddresses.kt 58.82% 0 Missing and 7 partials ⚠️
...inglabels/networking/WooShippingLabelRestClient.kt 0.00% 7 Missing ⚠️
...orders/wooshippinglabels/address/AddressSection.kt 0.00% 2 Missing ⚠️
.../wooshippinglabels/address/FetchOriginAddresses.kt 86.66% 0 Missing and 2 partials ⚠️
...hippinglabels/WooShippingLabelCreationViewModel.kt 0.00% 0 Missing and 1 partial ⚠️
...ls/datasource/WooShippingConfigurationDataStore.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #13211   +/-   ##
=========================================
  Coverage     40.62%   40.62%           
- Complexity     6375     6384    +9     
=========================================
  Files          1347     1349    +2     
  Lines         77301    77350   +49     
  Branches      10618    10643   +25     
=========================================
+ Hits          31404    31427   +23     
- Misses        43129    43145   +16     
- Partials       2768     2778   +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ThomazFB ThomazFB self-assigned this Dec 27, 2024
Copy link
Contributor

@ThomazFB ThomazFB left a comment

Choose a reason for hiding this comment

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

Looks good! Nice work with the use cases and data stores separations.

One thing I was thinking here and wanted to share in case it's useful for later: the ObserveStoreOptions and ObserveOriginAddresses are extremely similar in logic, only changing the data sources, but I'm not sure if there's a way to create a generic operation out of it or if it's worth to do so.

Base automatically changed from issue/13198-cache-account-settings to trunk December 28, 2024 01:42
@atorresveiga atorresveiga added this to the 21.4 milestone Dec 28, 2024
@atorresveiga atorresveiga merged commit e79bf23 into trunk Dec 28, 2024
17 checks passed
@atorresveiga atorresveiga deleted the issue/12437-get-origin-address-network branch December 28, 2024 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: shipping labels Related to creating, ordering, or printing shipping labels. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get origin address

5 participants