Skip to content
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

Woo: Part one for orders/batch endpoint #3119

Merged
merged 8 commits into from
Dec 13, 2024
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package org.wordpress.android.fluxc.wc.order

import com.google.gson.Gson
import com.google.gson.GsonBuilder
import com.google.gson.reflect.TypeToken
import org.junit.Test
import org.wordpress.android.fluxc.UnitTestUtils
import org.wordpress.android.fluxc.model.order.ShippingLine
import org.wordpress.android.fluxc.network.rest.wpcom.wc.order.BatchOrderApiResponse
import kotlin.test.assertEquals
import kotlin.test.assertNull
import kotlin.test.assertTrue
Expand Down Expand Up @@ -143,4 +145,31 @@ class OrderEntityTest {
assertEquals("Flat Rate Shipping", shippingLinesList[0].methodTitle)
assertEquals("Local Pickup Shipping", shippingLinesList[1].methodTitle)
}

@Test
fun testDeserializeBatchOrderResponse() {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, I think this test would better be in a separate file, OrderEntityTest is originally about OrderEntity logic, which doesn't apply here, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, moved in 6e2cdef 👍🏼

val testGson = GsonBuilder()
.registerTypeAdapter(
BatchOrderApiResponse.OrderResponse::class.java,
BatchOrderApiResponse.OrderResponseDeserializer())
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: Instead of having to register the type adapter manually, we can use the @TypeAdapter annotation directly, this is something that recent (relatively 😄) versions of Gson added, this patch would show how it can be used:

Index: plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/BatchOrderApiResponse.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/BatchOrderApiResponse.kt b/plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/BatchOrderApiResponse.kt
--- a/plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/BatchOrderApiResponse.kt	(revision 3c423f4b9646625298b72fbc52b981b47a7484c6)
+++ b/plugins/woocommerce/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/BatchOrderApiResponse.kt	(date 1734005033319)
@@ -3,12 +3,14 @@
 import com.google.gson.JsonDeserializationContext
 import com.google.gson.JsonDeserializer
 import com.google.gson.JsonElement
+import com.google.gson.annotations.JsonAdapter
 import java.lang.reflect.Type
 import org.wordpress.android.fluxc.network.Response
 
 data class BatchOrderApiResponse(
     val update: List<OrderResponse>
 ) : Response {
+    @JsonAdapter(OrderResponseDeserializer::class)
     sealed class OrderResponse {
         data class Success(
             val order: OrderDto
@@ -30,7 +32,7 @@
         val status: Int
     )
 
-    class OrderResponseDeserializer : JsonDeserializer<OrderResponse> {
+    private class OrderResponseDeserializer : JsonDeserializer<OrderResponse> {
         override fun deserialize(
             json: JsonElement,
             typeOfT: Type,
Index: example/src/test/java/org/wordpress/android/fluxc/wc/order/OrderEntityTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/example/src/test/java/org/wordpress/android/fluxc/wc/order/OrderEntityTest.kt b/example/src/test/java/org/wordpress/android/fluxc/wc/order/OrderEntityTest.kt
--- a/example/src/test/java/org/wordpress/android/fluxc/wc/order/OrderEntityTest.kt	(revision 3c423f4b9646625298b72fbc52b981b47a7484c6)
+++ b/example/src/test/java/org/wordpress/android/fluxc/wc/order/OrderEntityTest.kt	(date 1734004691032)
@@ -149,9 +149,6 @@
     @Test
     fun testDeserializeBatchOrderResponse() {
         val testGson = GsonBuilder()
-            .registerTypeAdapter(
-                BatchOrderApiResponse.OrderResponse::class.java,
-                BatchOrderApiResponse.OrderResponseDeserializer())
             .create()
 
         val batchOrderJson = UnitTestUtils.getStringFromResourceFile(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for mentioning that! Applied in 254cf5c

.create()

val batchOrderJson = UnitTestUtils.getStringFromResourceFile(
this.javaClass, "wc/orders-batch.json"
)

val response = testGson.fromJson(batchOrderJson, BatchOrderApiResponse::class.java)
val orders = response.update

assertEquals(2, orders.size)

val firstOrder = orders[0] as BatchOrderApiResponse.OrderResponse.Success
assertEquals(1032L, firstOrder.order.id)
assertEquals("224.00", firstOrder.order.total)

val secondOrder = orders[1] as BatchOrderApiResponse.OrderResponse.Error
assertEquals(525L, secondOrder.id)
assertEquals("woocommerce_rest_shop_order_invalid_id", secondOrder.error.code)
assertEquals(400, secondOrder.error.data.status)
}
}
160 changes: 160 additions & 0 deletions example/src/test/resources/wc/orders-batch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
{
"update": [
{
"id": 1032,
"parent_id": 0,
"status": "completed",
"currency": "USD",
"version": "9.4.3",
"prices_include_tax": false,
"date_created": "2024-12-04T11:21:17",
"date_modified": "2024-12-04T11:23:19",
"discount_total": "0.00",
"discount_tax": "0.00",
"shipping_total": "3.00",
"shipping_tax": "0.00",
"cart_tax": "0.00",
"total": "224.00",
"total_tax": "0.00",
"customer_id": 0,
"order_key": "wc_order_XXXXXXXXXXXX",
"billing": {
"first_name": "",
"last_name": "",
"company": "",
"address_1": "",
"address_2": "",
"city": "",
"state": "",
"postcode": "",
"country": "",
"email": "",
"phone": ""
},
"shipping": {
"first_name": "",
"last_name": "",
"company": "",
"address_1": "",
"address_2": "",
"city": "",
"state": "",
"postcode": "",
"country": "",
"phone": ""
},
"payment_method": "",
"payment_method_title": "",
"transaction_id": "",
"customer_ip_address": "",
"customer_user_agent": "",
"created_via": "rest-api",
"customer_note": "Asdasd",
"date_completed": "2024-12-04T11:23:19",
"date_paid": "2024-12-04T11:23:19",
"cart_hash": "",
"number": "1032",
"meta_data": [
{
"id": 18608,
"key": "_wc_order_attribution_source_type",
"value": "mobile_app"
}
],
"line_items": [
{
"id": 162,
"name": "Album",
"product_id": 71,
"variation_id": 0,
"quantity": 1,
"tax_class": "",
"subtotal": "216.00",
"subtotal_tax": "0.00",
"total": "216.00",
"total_tax": "0.00",
"taxes": [],
"meta_data": [],
"sku": "woo-album",
"price": 216,
"image": {
"id": "100",
"src": "https://example.com/images/album-1.jpg"
},
"parent_name": null,
"bundled_by": "",
"bundled_item_title": "",
"bundled_items": []
}
],
"tax_lines": [],
"shipping_lines": [
{
"id": 166,
"method_title": "Reg ship",
"method_id": "flat_rate",
"instance_id": "",
"total": "3.00",
"total_tax": "0.00",
"taxes": [],
"meta_data": []
}
],
"fee_lines": [
{
"id": 165,
"name": "Just for you",
"tax_class": "",
"tax_status": "none",
"amount": "",
"total": "5.00",
"total_tax": "0.00",
"taxes": [],
"meta_data": []
}
],
"coupon_lines": [],
"refunds": [],
"payment_url": "https://example.com/checkout/order-pay/1032/",
"is_editable": false,
"needs_payment": false,
"needs_processing": true,
"date_created_gmt": "2024-12-04T04:21:17",
"date_modified_gmt": "2024-12-04T04:23:19",
"date_completed_gmt": "2024-12-04T04:23:19",
"date_paid_gmt": "2024-12-04T04:23:19",
"currency_symbol": "$",
"_links": {
"self": [
{
"href": "https://example.com/wp-json/wc/v3/orders/1032",
"targetHints": {
"allow": [
"GET",
"POST",
"PUT",
"PATCH",
"DELETE"
]
}
}
],
"collection": [
{
"href": "https://example.com/wp-json/wc/v3/orders"
}
]
}
},
{
"id": "525",
"error": {
"code": "woocommerce_rest_shop_order_invalid_id",
"message": "Invalid ID.",
"data": {
"status": 400
}
}
}
]
}
1 change: 1 addition & 0 deletions fluxc-processor/src/main/resources/wc-wp-api-endpoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
/orders/<id>/shipment-trackings/providers/
/orders/<id>/receipt/
/orders/<id>/actions/send_order_details
/orders/batch

/products/<id>/
/products/<id>/variations/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.wordpress.android.fluxc.network.rest.wpcom.wc.order

import com.google.gson.JsonDeserializationContext
import com.google.gson.JsonDeserializer
import com.google.gson.JsonElement
import java.lang.reflect.Type
import org.wordpress.android.fluxc.network.Response

data class BatchOrderApiResponse(
Copy link
Member

Choose a reason for hiding this comment

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

minor, I would make this internal, I know we haven't been doing it often in the past, but I think it's still something we should consider for new features, as it would reduce leaking the networking models to the upper layers.

Copy link
Contributor Author

@hafizrahman hafizrahman Dec 13, 2024

Choose a reason for hiding this comment

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

This makes sense. However when I do so after the last commit, 6e2cdef, the unit test is unable to see this class, even though the package location is now the same. Still see the same issue after adding @VisibleForTesting, too.

Not sure yet why this happens, do you know? I'll merge this PR for now to continue work and add this change later.

Copy link
Member

Choose a reason for hiding this comment

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

This is because the test is defined in another module (example), I think it would be better to move the test to the same module plugins/woocommerce

val update: List<OrderResponse>
) : Response {
sealed class OrderResponse {
data class Success(
val order: OrderDto
) : OrderResponse()

data class Error(
val id: Long,
val error: ErrorResponse
) : OrderResponse()
}

data class ErrorResponse(
val code: String,
val message: String,
val data: ErrorData
)

data class ErrorData(
val status: Int
)

class OrderResponseDeserializer : JsonDeserializer<OrderResponse> {
override fun deserialize(
json: JsonElement,
typeOfT: Type,
context: JsonDeserializationContext
): OrderResponse {
val jsonObject = json.asJsonObject

return if (jsonObject.has("error")) {
OrderResponse.Error(
id = jsonObject.get("id").asLong,
error = context.deserialize(jsonObject.get("error"), ErrorResponse::class.java)
)
} else {
OrderResponse.Success(
context.deserialize(jsonObject, OrderDto::class.java)
)
}
}
}
}
Loading