Shipping methods networking #11478

atorresveiga · 2024-05-09T22:29:39Z

⚠️ This PR depends on this FluxC PR
Closes: #11391

Why

As a crucial part of the Enhancing Order Shipping Lines pffQ75-m4-p2, we are proposing to implement the shipping method selection feature, which is essential for our project's success.

Description

This PR adds the necessary logic to fetch the shipping methods from the API. The PR introduces the uses cases GetShippingMethodById which will return the shipping method using the method ID, and the GetShippingMethodsWithOtherValue which will return the list of shipping methods from the API and it will including the other shipping method. With the changes introduced here, the new UI finally connects with the data from the API.

Testing instructions

TC1

Open the app
Go to orders
Tap on add new order (+)
Tap on Add products and add a product, so the Add shipping button is enabled
Tap on Add shipping
Tap on select shipping method
Check that the shipping methods are displayed
Check that no shipping method is selected
Select a shipping method
Check that the app navigates back to the Add Shipping screen and the selected method is displayed on the Method section

TC2

After running the TC1 tap on the Method section
Check that the shipping methods are displayed
Check that the expected shipping method is selected

Images/gif

Screen_recording_20240509_162829.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.

wpmobilebot · 2024-05-09T23:06:18Z

📲 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
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`f146ba4`
	Direct Download	`woocommerce-prototype-build-pr11478-f146ba4.apk`

codecov-commenter · 2024-05-09T23:19:00Z

Codecov Report

Attention: Patch coverage is 76.19048% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 40.87%. Comparing base (5dd2dfe) to head (6374ec4).
Report is 18 commits behind head on trunk.

❗ Current head 6374ec4 differs from pull request most recent head f146ba4. Consider uploading reports for the commit f146ba4 to get more accurate results

Files	Patch %	Lines
...twork/shippingmethods/ShippingMethodsRestClient.kt	27.27%	8 Missing ⚠️
...creation/shipping/OrderShippingMethodsViewModel.kt	60.00%	2 Missing and 2 partials ⚠️
...in/com/woocommerce/android/model/ShippingMethod.kt	0.00%	0 Missing and 1 partial ⚠️
.../orders/creation/shipping/GetShippingMethodById.kt	87.50%	0 Missing and 1 partial ⚠️
...ation/shipping/GetShippingMethodsWithOtherValue.kt	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #11478      +/-   ##
============================================
+ Coverage     40.86%   40.87%   +0.01%     
- Complexity     5165     5176      +11     
============================================
  Files          1063     1067       +4     
  Lines         62111    62148      +37     
  Branches       8462     8470       +8     
============================================
+ Hits          25382    25405      +23     
- Misses        34446    34459      +13     
- Partials       2283     2284       +1

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

dangermattic · 2024-05-09T23:58:52Z

	1 Warning
⚠️	This PR is assigned to the milestone 18.6. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

ThomazFB

Looks good! I left some comments but nothing requiring changes, just technical discussions.

ThomazFB · 2024-05-10T03:05:21Z

...src/main/kotlin/com/woocommerce/android/ui/orders/creation/shipping/GetShippingMethodById.kt

+        val result = shippingMethodsRepository.fetchShippingMethods()
+        return result.model?.firstOrNull { shippingMethod -> shippingMethod.id == shippingMethodId } ?: other


Out of curiosity, does the API allow querying for a shipping method with a given ID? I'm wondering if we have a way to avoid fetching everything in a request only to filter it locally.

Great point! I totally missed the endpoint at first 🤦 . The fix is here

Awesome! Thanks for revisiting the API and finding such a great solution! 🎉

ThomazFB · 2024-05-10T03:07:03Z

...main/kotlin/com/woocommerce/android/ui/orders/creation/shipping/ShippingMethodsRepository.kt

+        const val OTHER_ID = "other"
+    }
+
+    suspend fun fetchShippingMethods(site: SiteModel = selectedSite.get()): WooResult<List<ShippingMethod>> {


Since this is a networking PR, I imagine this will be tackled later. But just to make sure: do you plan to add database support for the Methods after fetching? It could be beneficial to avoid requesting all the data every time one of the Use Cases is triggered, especially the ID one.

It could be beneficial to avoid requesting all the data every time one of the Use Cases is triggered, especially the ID one.

Good point! I will add a task to improve this and rely more on local data in an upcoming PR

wpmobilebot · 2024-05-10T04:46:12Z

Found 1 violations:

The PR caused the following dependency changes:

expand

-+--- org.wordpress:fluxc:2.78.0
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:2.78.0
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
-|    |    +--- com.google.crypto.tink:tink-android:1.5.0
-|    |    \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0 (*)
-|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
-|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
-|    |    +--- androidx.room:room-common:2.5.2 (*)
-|    |    +--- androidx.room:room-runtime:2.5.2 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
-|    +--- com.google.dagger:dagger:2.42 -> 2.50
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
++--- org.wordpress:fluxc:trunk-cfe34975d20df76a2b7b4522dccd2faf53ef0f7c
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-cfe34975d20df76a2b7b4522dccd2faf53ef0f7c
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
+|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
+|    |    +--- com.google.crypto.tink:tink-android:1.5.0
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0 (*)
+|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
+|    |    +--- androidx.room:room-common:2.5.2 (*)
+|    |    +--- androidx.room:room-runtime:2.5.2 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
+|    +--- com.google.dagger:dagger:2.42 -> 2.50
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:2.78.0
-     +--- org.wordpress:wellsql:2.0.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:2.78.0
-     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
-     +--- org.wordpress:fluxc:2.78.0 (*)
-     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-     +--- com.google.dagger:dagger:2.42 -> 2.50 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
-     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-cfe34975d20df76a2b7b4522dccd2faf53ef0f7c
+     +--- org.wordpress:wellsql:2.0.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-cfe34975d20df76a2b7b4522dccd2faf53ef0f7c
+     +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
+     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
+     +--- org.wordpress:fluxc:trunk-cfe34975d20df76a2b7b4522dccd2faf53ef0f7c (*)
+     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+     +--- com.google.dagger:dagger:2.42 -> 2.50 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
+     \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)

Please review and act accordingly

Alejo added 7 commits May 9, 2024 16:11

update fluxc version

37ae301

add shipping methods repository

8b5c7ba

add get shipping methods use case

17485d4

add get shipping method by id use case

a6eb419

add ShippingMethod mapper

44970bc

use real data

577c929

fix typo

c83d80e

atorresveiga added type: task An internally driven task. feature: order creation Related to the Order Creation feature labels May 9, 2024

atorresveiga requested a review from ThomazFB May 9, 2024 22:29

atorresveiga added this to the 18.6 milestone May 9, 2024

rename package

6374ec4

atorresveiga mentioned this pull request May 9, 2024

Add shipping methods endpoint wordpress-mobile/WordPress-FluxC-Android#3007

Merged

Base automatically changed from issue/11390-select-shipping-method-ui to trunk May 9, 2024 23:58

atorresveiga removed the request for review from ThomazFB May 9, 2024 23:58

ThomazFB self-assigned this May 10, 2024

ThomazFB approved these changes May 10, 2024

View reviewed changes

Alejo added 3 commits May 9, 2024 21:49

get shipping method by id

c222a0e

fix unit tests

354d9a5

update FluxC version

f146ba4

atorresveiga enabled auto-merge May 10, 2024 04:48

atorresveiga merged commit 363fa88 into trunk May 10, 2024
14 checks passed

atorresveiga deleted the issue/11391-shipping-methods-networking branch May 10, 2024 05:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Shipping methods networking #11478

Shipping methods networking #11478

Uh oh!

atorresveiga commented May 9, 2024 •

edited

Loading

Uh oh!

wpmobilebot commented May 9, 2024 •

edited

Loading

Uh oh!

codecov-commenter commented May 9, 2024 •

edited

Loading

Uh oh!

dangermattic commented May 9, 2024

Uh oh!

ThomazFB left a comment

Uh oh!

ThomazFB May 10, 2024

Uh oh!

atorresveiga May 10, 2024

Uh oh!

ThomazFB May 10, 2024

Uh oh!

ThomazFB May 10, 2024 •

edited

Loading

Uh oh!

atorresveiga May 10, 2024

Uh oh!

wpmobilebot commented May 10, 2024

Uh oh!

Uh oh!

Uh oh!

		val result = shippingMethodsRepository.fetchShippingMethods()
		return result.model?.firstOrNull { shippingMethod -> shippingMethod.id == shippingMethodId } ?: other

Shipping methods networking #11478

Shipping methods networking #11478

Uh oh!

Conversation

atorresveiga commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Description

Testing instructions

Images/gif

Uh oh!

wpmobilebot commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dangermattic commented May 9, 2024

Uh oh!

ThomazFB left a comment

Choose a reason for hiding this comment

Uh oh!

ThomazFB May 10, 2024

Choose a reason for hiding this comment

Uh oh!

atorresveiga May 10, 2024

Choose a reason for hiding this comment

Uh oh!

ThomazFB May 10, 2024

Choose a reason for hiding this comment

Uh oh!

ThomazFB May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atorresveiga May 10, 2024

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented May 10, 2024

The PR caused the following dependency changes:

Uh oh!

Uh oh!

Uh oh!

atorresveiga commented May 9, 2024 •

edited

Loading

wpmobilebot commented May 9, 2024 •

edited

Loading

codecov-commenter commented May 9, 2024 •

edited

Loading

ThomazFB May 10, 2024 •

edited

Loading