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

ParaskP7 · 2023-09-11T11:48:07Z

Parent FluxC#2799
Accompanying FluxC#2851

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

FYI: This change is breaking, meaning that this client, which depends on the Term model, is now being updated to use 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 and clients side.

~~@mkevins~~ @AjeshRPai let me know how I did on this PR, that is, in terms of updating this client to use the non-default constructors for the Term model in order to utilize the nullability annotations added on its fields and thus make its use null proof. If you also feel that this is a good way to proceed with such client PRs, I'll then use this pattern across the board and follow a similar approach on every such PR.

FluxC Update List:

Deprecation Resolution List (breaking):

Warning Resolution List:

To test:

Smoke test any TaxonomyStore related functionality and TermModel related instantialization 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:

1. Categories Settings Screen [CategoriesListFragment.kt]

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

Go to Menu sub-tab on the initial My Site screen and click on Site Settings under the
Manage section.
Find the Categories under the Writing section and click on it.
Verify that the Categories screen is displayed and that everything works as expected.

2. Tags Settings Screen [SiteSettingsTagListActivity.kt]

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

Go to Menu sub-tab on the initial My Site screen and click on Site Settings under the
Manage section.
Find the Tags under the Writing section and click on it.
Verify that the Tags screen is displayed and that everything works as expected.

3. Prepublishing Screens [PrepublishingTagsFragment.kt + PrepublishingCategoriesFragment.kt]

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

Go to Posts screen and edit any post.
Click on the UPDATE button to have the bottom sheet appear (top-right).
Click on the Tags row and add, remove or edit any tag for this post. Click back.
Click on the Categories row and add, remove or create a new category for this post. Click back.
Click on the UPDATE NOW button to update this post (bottom).
Verify that this post's Tags/Categories are updated and that everything works as expected.

Merge instructions:

Wait for the accompanying FluxC#2851 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
- Potential new NPE start happening on the Prepublishing screens, related to the term.name being null, which had it's (seemingly unnecessary) guarding code removed.
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 'TermModel' is updated to its new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2851 This step is required in order to check that these 'FluxC' related changes work as expected for WPAndroid.

Warning: "Type mismatch. Required: String Found: String?"

@nonnull

Compile Warnings: - "Unnecessary safe call on a non-null receiver of type TermModel" - "Condition 'event.term.name == null' is always 'false'" ------------------------------------------------------------------------ As per the removed comment, and although it specifies that sometimes the API will return a success response with a null name, which then will be treated as an error, because without a name, there is no category, after some testing, it has been verified that this is not needed anymore. A '409 duplicate' error is returned instead, that is, instead of a valid response but with a 'null' name, which the deleted code was guarding against. This previous 'term.name == null' change is related to this JP/WPAndroid #13256 PR below: #13256 However, as per this newly merge FluxC #2826 PR below, the 'TermWPComRestResponse' response class and its 'name' field was marked as '@nonnull'. wordpress-mobile/WordPress-FluxC-Android#2826 PS: If and whenever the backend starts sending a 'term.name' as 'nullable', then this need to be addressed on the backend side, that is, instead of making 'term.name' a nullable field on the FluxC side and start guarding against such occurrences everywhere.

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

Compile Warnings: "'constructor TermModel()' is deprecated. Deprecated in Java"

wpmobilebot · 2023-09-11T12:04:31Z

📲 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	pr19158-88618fa
	Commit	`88618fa`
	Direct Download	`jetpack-prototype-build-pr19158-88618fa.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-09-11T12:04:52Z

📲 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	pr19158-88618fa
	Commit	`88618fa`
	Direct Download	`wordpress-prototype-build-pr19158-88618fa.apk`

Note: Google Login is not supported on these builds.

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

This 'FluxC' PR hash updates the library to that branch version where the 'TermModel' is updated to its new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2851 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 39b6fb7. This change is necessary in order to overcome and resolve new merge conflicts.

ParaskP7 · 2023-10-02T14:29:19Z

👋 @mkevins I've now unassigned you from this and assigned @AjeshRPai instead. It seems that you might be busy atm and I don't want to hold this PR any longer. 🙏

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

This 'FluxC' PR hash updates the library to that branch version where the 'TermModel' is updated to its new null proof version. FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2851 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 621e5b0. This change is necessary in order to overcome and resolve new merge conflicts.

AjeshRPai

Hey @ParaskP7

Tested the changes in this PR with the companion WP PR. Everything looks good to me

ParaskP7 · 2023-10-04T09:30:12Z

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

… into analysis/use-updated-and-null-proff-term-model-class

This 'FluxC' hash updates the library to that 'trunk' version where the 'TermModel' is updated to its new null proof version. Related FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2851

wpmobilebot · 2023-10-04T10:08:20Z

Found 1 violations:

The PR caused the following dependency changes:

-+--- org.wordpress:fluxc:{strictly 2.48.0} -> 2.48.0
-|    +--- 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:2.48.0
-|    +--- org.greenrobot:eventbus:3.3.1
-|    |    \--- org.greenrobot:eventbus-java:3.3.1
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.10.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.21 (*)
-|    |    \--- 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.21 (*)
-|    +--- 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.21 (*)
-|    |    \--- 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-55122d0f327b24441003653dcfe7d3cd03bc7972} -> trunk-55122d0f327b24441003653dcfe7d3cd03bc7972
+|    +--- 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-55122d0f327b24441003653dcfe7d3cd03bc7972
+|    +--- org.greenrobot:eventbus:3.3.1
+|    |    \--- org.greenrobot:eventbus-java:3.3.1
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.10.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.21 (*)
+|    |    \--- 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.21 (*)
+|    +--- 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.21 (*)
+|    |    \--- 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:1.6.0
-     \--- org.wordpress:fluxc:2.21.0 -> 2.48.0 (*)
+     \--- org.wordpress:fluxc:2.21.0 -> trunk-55122d0f327b24441003653dcfe7d3cd03bc7972 (*)

Please review and act accordingly

ParaskP7 added 5 commits September 11, 2023 14:06

Analysis: Require the term model related slug field to be non-null

f712d16

Warning: "Type mismatch. Required: String Found: String?"

Analysis: Resolve term model constructor deprecation warnings

b4c86cd

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

Analysis: Resolve term model constructor deprecation warnings for tests

3fa0226

Compile Warnings: "'constructor TermModel()' is deprecated. Deprecated in Java"

ParaskP7 added Core Tech Debt [Status] Not Ready for Merge labels Sep 11, 2023

ParaskP7 added this to the Future milestone Sep 11, 2023

ParaskP7 self-assigned this Sep 11, 2023

ParaskP7 mentioned this pull request Sep 11, 2023

[Nullability Annotations to Java Classes] Add Missing Nullability Annotations to All Taxonomy & Term SqlUtils & Model Classes (breaking) wordpress-mobile/WordPress-FluxC-Android#2851

Merged

4 tasks

ParaskP7 requested a review from mkevins September 11, 2023 12:14

ParaskP7 marked this pull request as ready for review September 11, 2023 12:14

ParaskP7 added 2 commits September 19, 2023 11:20

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

931f056

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

ParaskP7 requested review from AjeshRPai and removed request for mkevins October 2, 2023 14:28

ParaskP7 added 2 commits October 3, 2023 12:07

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

36411e9

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

AjeshRPai approved these changes Oct 3, 2023

View reviewed changes

ParaskP7 added 2 commits October 4, 2023 12:35

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

0284308

… into analysis/use-updated-and-null-proff-term-model-class

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

88618fa

This 'FluxC' hash updates the library to that 'trunk' version where the 'TermModel' is updated to its new null proof version. Related FluxC PR: https://github.com/wordpress-mobile/ WordPress-FluxC-Android/pull/2851

ParaskP7 removed the [Status] Not Ready for Merge label Oct 4, 2023

ParaskP7 merged commit a5e5a94 into trunk Oct 4, 2023

ParaskP7 deleted the analysis/use-updated-and-null-proff-term-model-class branch October 4, 2023 10:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ParaskP7 commented Sep 11, 2023 •

edited

Loading

wpmobilebot commented Sep 11, 2023 •

edited

Loading

wpmobilebot commented Sep 11, 2023 •

edited

Loading

ParaskP7 commented Oct 2, 2023

AjeshRPai left a comment

ParaskP7 commented Oct 4, 2023

wpmobilebot commented Oct 4, 2023

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

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

Conversation

ParaskP7 commented Sep 11, 2023 • edited Loading

To test:

Merge instructions:

Regression Notes

PR submission checklist:

UI Changes testing checklist:

wpmobilebot commented Sep 11, 2023 • edited Loading

wpmobilebot commented Sep 11, 2023 • edited Loading

ParaskP7 commented Oct 2, 2023

AjeshRPai left a comment

Choose a reason for hiding this comment

ParaskP7 commented Oct 4, 2023

wpmobilebot commented Oct 4, 2023

The PR caused the following dependency changes:

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

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

ParaskP7 commented Sep 11, 2023 •

edited

Loading

wpmobilebot commented Sep 11, 2023 •

edited

Loading

wpmobilebot commented Sep 11, 2023 •

edited

Loading