Skip to content

Conversation

@hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Nov 22, 2024

Closes: #12989

Description

This PR fixes an issue with how the /wp/v2/media response is handled, currently, when calling this API through WPCom for a Jetpack CP site, and the property media_details.sizes is empty, then we receive an array instead of an empty object, and this leads to a parsing error.

Depends on wordpress-mobile/WordPress-FluxC-Android#3112

Steps to reproduce

  1. Use a Jetpack CP site.
  2. Add a very small image to your website (something with less than 10 pixels)
  3. Open the app and sign in using WordPress.com
  4. Open a product.
  5. Tap on Add images, and choose the WordPress Media Library.

Testing information

  • Confirm that the WordPress Media Library loads without issues.

The tests that have been performed

^

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

@hichamboushaba hichamboushaba added the type: bug A confirmed bug. label Nov 22, 2024
@hichamboushaba hichamboushaba added this to the 21.2 ❄️ milestone Nov 22, 2024
@hichamboushaba hichamboushaba added feature: product details Related to adding or editing products, includes product settings. status: do not merge Dependent on another PR, ready for review but not ready for merge. labels Nov 22, 2024
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 22, 2024

Project dependencies changes

The following changes in project dependencies were detected (configuration vanillaReleaseRuntimeClasspath):

list
Removed Dependencies
androidx.paging:paging-common:2.1.2
androidx.paging:paging-runtime:2.1.2
com.goterl:lazysodium-android:5.0.2
com.squareup.okhttp3:okhttp-urlconnection:4.9.0
net.java.dev.jna:jna:5.5.0
org.wordpress.fluxc:fluxc-annotations:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520
org.wordpress.wellsql:wellsql-annotations:2.0.0
org.wordpress:wellsql:2.0.0

Upgraded Dependencies
org.wordpress.fluxc.plugins:woocommerce:trunk-b1086fc19b68a57592f6b337d8044b39296ed12c FAILED, (changed from trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520)
org.wordpress:fluxc:trunk-b1086fc19b68a57592f6b337d8044b39296ed12c FAILED, (changed from trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520)
tree
-+--- org.wordpress:fluxc:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520
-|    +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25 -> 2.0.21
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
-|    |         \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.8.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.6.1 (*)
-|    +--- androidx.room:room-ktx:2.6.1
-|    |    +--- androidx.room:room-common:2.6.1 (*)
-|    |    +--- androidx.room:room-runtime:2.6.1 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
-|    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
-|    |    +--- androidx.room:room-common:2.6.1 (c)
-|    |    \--- androidx.room:room-runtime:2.6.1 (c)
-|    +--- com.google.dagger:dagger:2.51.1
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- org.greenrobot:eventbus-java: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.8.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.7.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.7.0 (*)
-|    |    \--- 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:1.9.25 -> 2.0.21 (*)
++--- org.wordpress:fluxc:trunk-b1086fc19b68a57592f6b337d8044b39296ed12c FAILED
-+--- org.wordpress.fluxc.plugins:woocommerce:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520
-|    +--- org.wordpress:fluxc:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- com.google.dagger:dagger:2.51.1 (*)
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-|    +--- androidx.room:room-runtime:2.6.1 (*)
-|    +--- org.wordpress:wellsql:2.0.0 (*)
-|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-3d095f6f0a41e50faab2038a9f195f536b6e4520
-|    +--- androidx.room:room-ktx:2.6.1 (*)
-|    \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
++--- org.wordpress.fluxc.plugins:woocommerce:trunk-b1086fc19b68a57592f6b337d8044b39296ed12c FAILED
 +--- org.wordpress:login:1.19.0
 |    +--- com.github.bumptech.glide:glide:4.12.0 -> 4.16.0
-|    |    \--- androidx.exifinterface:exifinterface:1.3.6 (*)
+|    |    \--- androidx.exifinterface:exifinterface:1.3.6
+|    |         \--- androidx.annotation:annotation:1.2.0 -> 1.8.0 (*)
-|    \--- com.google.dagger:dagger:2.47 -> 2.51.1 (*)
+|    \--- com.google.dagger:dagger:2.47 -> 2.51.1
+|         \--- javax.inject:javax.inject:1
 \--- project :libs:cardreader
      +--- com.stripe:stripeterminal-localmobile:3.7.1
-     |    \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.24 -> 2.0.21 (*)
+     |    \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.24 -> 2.0.21
+     |         +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+     |         \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
+     |              \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
      \--- com.stripe:stripeterminal-core:3.7.1
-          +--- androidx.security:security-crypto:1.1.0-alpha03 (*)
+          +--- androidx.security:security-crypto:1.1.0-alpha03
+          |    +--- androidx.annotation:annotation:1.1.0 -> 1.8.0 (*)
+          |    +--- com.google.crypto.tink:tink-android:1.5.0
+          |    \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-          \--- androidx.room:room-ktx:2.6.1 (*)
+          \--- androidx.room:room-ktx:2.6.1
+               +--- androidx.room:room-common:2.6.1 (*)
+               +--- androidx.room:room-runtime:2.6.1 (*)
+               +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
+               +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
+               +--- androidx.room:room-common:2.6.1 (c)
+               \--- androidx.room:room-runtime:2.6.1 (c)

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 25, 2024

📲 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
Commit189b4f4
Direct Downloadwoocommerce-wear-prototype-build-pr12990-189b4f4.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 25, 2024

📲 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
Commit189b4f4
Direct Downloadwoocommerce-prototype-build-pr12990-189b4f4.apk

@hichamboushaba hichamboushaba changed the base branch from release/21.2 to trunk November 25, 2024 11:22
@hichamboushaba hichamboushaba marked this pull request as ready for review November 25, 2024 11:22
@JorgeMucientes JorgeMucientes self-assigned this Nov 26, 2024
@irfano irfano modified the milestones: 21.2 ❄️, 21.3 Nov 26, 2024
Copy link
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @hichamboushaba. It now works as expected.

Just a small clarification. This was not causing the app to crash right? My experience is that this was simply causing a "handled" parsing error which lead to the images with "sizes":[ ] being gone from the media picker. Correct? Just asking in case I missed something.

@hichamboushaba
Copy link
Member Author

Thanks @JorgeMucientes for the review.

Just a small clarification. This was not causing the app to crash right? My experience is that this was simply causing a "handled" parsing error which lead to the images with "sizes":[ ] being gone from the media picker. Correct? Just asking in case I missed something.

Yes, the app doesn't crash, but the impact was not just for those images, the media picker will be shown empty (assuming you didn't have any images cached before), as the failure to deserialize will be treated as fetching error.

@hichamboushaba hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 27, 2024
@hichamboushaba hichamboushaba merged commit a1944e8 into trunk Nov 27, 2024
16 checks passed
@hichamboushaba hichamboushaba deleted the issue/12989-wp-v2-media-fix-issue-with-empty-sizes branch November 27, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: product details Related to adding or editing products, includes product settings. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Media library fails to load if the site has a small media

5 participants