[Nullability Annotations to Java Classes] Use Updated and Null Proof `MediaModel` Class (`breaking`) #19506

ParaskP7 · 2023-11-01T14:23:27Z

Parent FluxC#2799
Accompanying FluxC#2886

This PR accompanying the above FluxC PR, and uses an updated and null-proof MediaModel.java class.

FYI: This change is breaking, meaning that any client that depended on the MediaModel.java model should update to the new APIs (non-default constructors) as the existing API (default constructor) are now deprecated. As such, this change is inherently risky, meaning that there are compile-time changes associated with this change and thus needs testing to verify correctness, both on the library's and client's side.

@AjeshRPai I added you as the main reviewer, not so randomly (it being a continuation of FluxC#2878 and #19428), since I just wanted someone from the Jetpack/WordPress mobile team to be aware of and sign-off on that change. Feel free to merge this PR directly yourself if you deem so.

FluxC Update List:

Deprecation Resolution List (breaking):

Nullability Checks List:

Warnings Suppression List:

Suppress spread operator on page list event listener

To Test (`REST`):

Smoke test any media related functionality on both, the WordPress and Jetpack apps, and see if everything is working as expected. For a couple of examples, you can expand and follow the inner and more explicit test steps within:

To Test (`XMLRPC`):

Create a new self-hosted WordPress site for XMLRPC testing purposes (jurassic.ninja).
Smoke test any media related functionality on both, the WordPress and Jetpack apps, and see if everything is working as expected. For a couple of examples, you can expand and follow the inner and more explicit test steps within:

Media Screens [MediaBrowserActivity.java + MediaGridFragment.kt + MediaSettingsActivity.java + MediaPreviewActivity.java + MediaPreviewFragment.java]

ℹ️ This test applies to both, the Jetpack and WordPress app.

Go to Media screen, verify it is shown and functioning as expected.
For example try changing tabs, searching and adding a new media.
Tap on any media and verify that the Media Settings screen is shown and functioning as expected.
For example try updating the media title or adding a media description.
Tap on the media preview and verify that the Media Preview screen is shown and functioning as expected.
For example try swipe left-right to navigate between media previews.

Merge instructions:

Wait for the accompanying FluxC#2886 PR to be merged.
Update wordPressFluxCVersion to point to the trunk hash that includes the above solution.
Remove the [Status] Not Ready for Merge label.
Merge this PR.

Regression Notes

Potential unintended areas of impact
- N/A
What I did to test those areas of impact (or what existing automated tests I relied on)
- See To test section above.
What automated tests I added (or what prevented me from doing so)
- N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
Talkback.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
Large and small screen sizes. (Tablet and smaller phones)
Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

This 'FluxC' PR hash updates the library to that branch version where the 'MediaModel' is updated to its new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2886 This step is required in order to check that these 'FluxC' related changes work as expected for WPAndroid.

Warnings: - Kotlin: "'constructor MediaModel()' is deprecated. Deprecated in Java" - Java: "'MediaModel()' is deprecated"

FYI: This change changes the logic to only checking for not blank instead.

Detekt Warning: "SpreadOperator: In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty." Now that this warning is suppressed close to source, it is no longer needed to be in the baseline.

wpmobilebot · 2023-11-01T14:41:36Z

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

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr19506-5a9a89b
	Commit	`5a9a89b`
	Direct Download	`wordpress-prototype-build-pr19506-5a9a89b.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-11-01T14:42:44Z

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

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr19506-5a9a89b
	Commit	`5a9a89b`
	Direct Download	`jetpack-prototype-build-pr19506-5a9a89b.apk`

Note: Google Login is not supported on these builds.

… into analysis/use-updated-and-null-proof-media-model-class # Conflicts: # build.gradle

This 'FluxC' PR hash updates the library to that branch version where the 'MediaModel' is updated to its new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2886 This step is required in order to check that these 'FluxC' related changes work as expected for WPAndroid. ------------------------------------------------------------------------ FYI: This change is an addition to f6f9375. This change is necessary in order to overcome and resolve new merge conflicts.

ParaskP7 · 2023-11-07T15:32:32Z

FYI: I'll resolve the build.gradle merge conflict when I'll be updating wordPressFluxCVersion to point to the trunk hash that includes the above solution (see Merge instructions).

AjeshRPai

Hey @ParaskP7

I have tested the following scenarios and gone through the code changes. Everything looks good to me 👍🏼

Publish a post and page with featured image + without featured image
Publish a post and page with image and video
Edit the image in a post and page
Upload image and video
Add image from tenor library
Update the title, description and caption of an image and a video through media
Download/Save the media to the device - from the detail screen of image/video
Delete an image/video

I am not sure if I have missed any other scenarios. I am approving but not merging so that you can take of care updating the fluxC version once the FluxC PR is merged.

ParaskP7 · 2023-11-08T08:42:30Z

Awesome, thank for reviewing and testing this @AjeshRPai , you rock! 🙇 ❤️ 🚀

FYI: I'll proceed with the merging myself. 🙏

… into analysis/use-updated-and-null-proof-media-model-class # Conflicts: # build.gradle

This 'FluxC' hash updates the library to that 'trunk' version where the 'Media' model classes are updated to their new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2886

wpmobilebot · 2023-11-08T11:38:28Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2023-11-08T11:39:31Z

Found 1 violations:

The PR caused the following dependency changes:

-+--- org.wordpress:fluxc:{strictly trunk-8712ec791066f2d86bb9aa6d21710ee7075fac05} -> trunk-8712ec791066f2d86bb9aa6d21710ee7075fac05
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-8712ec791066f2d86bb9aa6d21710ee7075fac05
-|    +--- org.greenrobot:eventbus:3.3.1
-|    |    \--- org.greenrobot:eventbus-java:3.3.1
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.1
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.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.0 (*)
-|    +--- 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.8.21 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.21 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6 (*)
-|    +--- androidx.security:security-crypto:1.0.0
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
-|    |    \--- com.google.crypto.tink:tink-android:1.5.0
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0
-|    |    \--- org.apache.commons:commons-lang3:3.12.0
-|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.0
-|    |    +--- androidx.annotation:annotation-experimental:1.1.0 -> 1.3.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.1 -> 2.2.0 (*)
-|    |    +--- androidx.room:room-common:2.5.0
-|    |    |    +--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
-|    |    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 -> 1.8.21 (*)
-|    |    +--- androidx.sqlite:sqlite:2.3.0
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
-|    |    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
-|    |    \--- androidx.sqlite:sqlite-framework:2.3.0
-|    |         +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-|    |         +--- androidx.sqlite:sqlite:2.3.0 (*)
-|    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
-|    +--- androidx.room:room-ktx:2.4.2
-|    |    +--- androidx.room:room-common:2.4.2 -> 2.5.0 (*)
-|    |    +--- androidx.room:room-runtime:2.4.2 -> 2.5.0 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.8.22 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.7.3 (*)
-|    +--- com.google.dagger:dagger:2.42 -> 2.46.1
-|    |    \--- 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:{strictly trunk-e71a4dc765b7785f753e9224512e0a76c43104e4} -> trunk-e71a4dc765b7785f753e9224512e0a76c43104e4
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-e71a4dc765b7785f753e9224512e0a76c43104e4
+|    +--- org.greenrobot:eventbus:3.3.1
+|    |    \--- org.greenrobot:eventbus-java:3.3.1
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.1
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.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.0 (*)
+|    +--- 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.8.21 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.21 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6 (*)
+|    +--- androidx.security:security-crypto:1.0.0
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
+|    |    \--- com.google.crypto.tink:tink-android:1.5.0
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0
+|    |    \--- org.apache.commons:commons-lang3:3.12.0
+|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.0
+|    |    +--- androidx.annotation:annotation-experimental:1.1.0 -> 1.3.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.1 -> 2.2.0 (*)
+|    |    +--- androidx.room:room-common:2.5.0
+|    |    |    +--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
+|    |    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 -> 1.8.21 (*)
+|    |    +--- androidx.sqlite:sqlite:2.3.0
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
+|    |    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
+|    |    \--- androidx.sqlite:sqlite-framework:2.3.0
+|    |         +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+|    |         +--- androidx.sqlite:sqlite:2.3.0 (*)
+|    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.22 (*)
+|    +--- androidx.room:room-ktx:2.4.2
+|    |    +--- androidx.room:room-common:2.4.2 -> 2.5.0 (*)
+|    |    +--- androidx.room:room-runtime:2.4.2 -> 2.5.0 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 -> 1.8.22 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.7.3 (*)
+|    +--- com.google.dagger:dagger:2.42 -> 2.46.1
+|    |    \--- 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:login:trunk-9963d78096edf65f8704b803e5b93c08fc9174cd
-     \--- org.wordpress:fluxc:2.21.0 -> trunk-8712ec791066f2d86bb9aa6d21710ee7075fac05 (*)
+     \--- org.wordpress:fluxc:2.21.0 -> trunk-e71a4dc765b7785f753e9224512e0a76c43104e4 (*)

Please review and act accordingly

ParaskP7 added 13 commits November 1, 2023 13:42

Analysis: Resolve media model constructor deprecation warnings

1e8aa5a

Warnings: - Kotlin: "'constructor MediaModel()' is deprecated. Deprecated in Java" - Java: "'MediaModel()' is deprecated"

Analysis: Un-guard usages of non-null media model url

fbcfd00

FYI: This change changes the logic to only checking for not blank instead.

Analysis: Guard usages of nullable media model upload date

4d3b22e

Analysis: Guard usages of nullable media model upload state

eff8564

Analysis: Un-guard usages of non-null on media changed media list

d2ccda4

Analysis: Guard usages of nullable on media uploaded media

0c67de2

Analysis: Guard usages of nullable upload stock media error message

99c33d4

Analysis: Use one argument media error constructor on upload utils

a5f6bb4

Analysis: Guard usages of nullable get site media with id medial model

63112c9

Analysis: Guard usages of nullable get media with local id medial model

6a8cf68

Analysis: Un-guard usages of non-null get media for post with state list

db19fc3

ParaskP7 added Core Tech Debt [Status] Not Ready for Merge labels Nov 1, 2023

ParaskP7 added this to the Future milestone Nov 1, 2023

ParaskP7 requested a review from AjeshRPai November 1, 2023 14:23

ParaskP7 mentioned this pull request Nov 1, 2023

[Nullability Annotations to Java Classes] Add Missing Nullability Annotations to Media Model Classes (breaking) wordpress-mobile/WordPress-FluxC-Android#2886

Merged

6 tasks

ParaskP7 added 2 commits November 7, 2023 15:42

Merge branch 'trunk' of github.com:wordpress-mobile/WordPress-Android…

14d741e

… into analysis/use-updated-and-null-proof-media-model-class # Conflicts: # build.gradle

AjeshRPai approved these changes Nov 8, 2023

View reviewed changes

ParaskP7 added 2 commits November 8, 2023 12:59

Merge branch 'trunk' of github.com:wordpress-mobile/WordPress-Android…

7e568f5

… into analysis/use-updated-and-null-proof-media-model-class # Conflicts: # build.gradle

Build: Update fluxc version to trunk hash (#2886)

5a9a89b

This 'FluxC' hash updates the library to that 'trunk' version where the 'Media' model classes are updated to their new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2886

ParaskP7 removed the [Status] Not Ready for Merge label Nov 8, 2023

ParaskP7 merged commit 1f1ba95 into trunk Nov 8, 2023
2 checks passed

ParaskP7 deleted the analysis/use-updated-and-null-proof-media-model-class branch November 8, 2023 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Nullability Annotations to Java Classes] Use Updated and Null Proof `MediaModel` Class (`breaking`) #19506

[Nullability Annotations to Java Classes] Use Updated and Null Proof `MediaModel` Class (`breaking`) #19506

ParaskP7 commented Nov 1, 2023 •

edited

Loading

wpmobilebot commented Nov 1, 2023 •

edited

Loading

wpmobilebot commented Nov 1, 2023 •

edited

Loading

ParaskP7 commented Nov 7, 2023

AjeshRPai left a comment

ParaskP7 commented Nov 8, 2023

wpmobilebot commented Nov 8, 2023

wpmobilebot commented Nov 8, 2023

[Nullability Annotations to Java Classes] Use Updated and Null Proof MediaModel Class (breaking) #19506

[Nullability Annotations to Java Classes] Use Updated and Null Proof MediaModel Class (breaking) #19506

Conversation

ParaskP7 commented Nov 1, 2023 • edited Loading

To Test (REST):

To Test (XMLRPC):

Merge instructions:

Regression Notes

PR submission checklist:

UI Changes testing checklist:

wpmobilebot commented Nov 1, 2023 • edited Loading

wpmobilebot commented Nov 1, 2023 • edited Loading

ParaskP7 commented Nov 7, 2023

AjeshRPai left a comment

Choose a reason for hiding this comment

ParaskP7 commented Nov 8, 2023

wpmobilebot commented Nov 8, 2023

wpmobilebot commented Nov 8, 2023

The PR caused the following dependency changes:

[Nullability Annotations to Java Classes] Use Updated and Null Proof `MediaModel` Class (`breaking`) #19506

[Nullability Annotations to Java Classes] Use Updated and Null Proof `MediaModel` Class (`breaking`) #19506

ParaskP7 commented Nov 1, 2023 •

edited

Loading

To Test (`REST`):

To Test (`XMLRPC`):

wpmobilebot commented Nov 1, 2023 •

edited

Loading

wpmobilebot commented Nov 1, 2023 •

edited

Loading