Crash on the installation of a plugin that is already installed #18890

irfano · 2023-08-04T14:43:05Z

This PR prevents the crash on the installation of a plugin that is already installed.

Before merging this,

Disable caching in fetchPlugins() call WordPress-FluxC-Android#2793 should be merged, and the FluxC reference should be updated in this PR.

Fix on FluxC

Before, FluxC was using the cached data for the plugins of the site. This caused showing the installed plugin as not installed and a crash when the user tried to re-install it.
After the fix, FluxC will always fetch and show the fresh data.

Fix on WPAndroid

Even though we disable caching the plugins, this case still can be created by:

Opening the detail screen of the Jetpack plugin and keeping it open,
Installing the Jetpack plugin via the web,
Tapping the INSTALL button on the app (when it's already installed via the web).

To prevent this case, InstallSitePluginErrorType .PLUGIN_ALREADY_INSTALLED error type is caught on the app, and the new plugin state is fetched to update the screen.

To test:
Case 1:

Launch the app.
Add a self-hosted site that is not connected with Jetpack.
Go to "My Site → Plugins".
Search for the "Jetpack - WP Security, Backup, Speed, & Growth" plugin and tap on it.
Install the Jetpack plugin and ensure it's installed successfully without a crash.
Open the Plugins screen for your site on the web and verify the Jetpack plugin is installed.
Open the "MANAGE" screen in the "Installed" section on the Plugins screen and ensure the Jetpack plugin is in the list.
Delete the Jetpack plugin from the web.
Repeat item 4.
Verify the plugin is displayed as not installed.
Repeat 5.

Case 2:

Install the Jetpack plugin that is already installed by following the instructions in the Fix on WPAndroid section.
Verify the screen is updated successfully, and the plugin is displayed as installed.

Regression Notes

Potential unintended areas of impact
Disabling caching could also disable storing the site plugins in db.
What I did to test those areas of impact (or what existing automated tests I relied on)
I checked the db and verified SitePluginModel table is still existing as before.
What automated tests I added (or what prevented me from doing so)
"This crash can only be tested as a UI test, and I believe it is a too rare case to include it in the automated tests.

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.

wpmobilebot · 2023-08-04T15:18:01Z

📲 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	pr18890-5a1e0b7
	Commit	`5a1e0b7`
	Direct Download	`wordpress-prototype-build-pr18890-5a1e0b7.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-08-04T15:20:19Z

📲 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	pr18890-5a1e0b7
	Commit	`5a1e0b7`
	Direct Download	`jetpack-prototype-build-pr18890-5a1e0b7.apk`

Note: Google Login is not supported on these builds.

mkevins · 2023-08-07T03:17:31Z

WordPress/src/main/java/org/wordpress/android/ui/plugins/PluginDetailActivity.java

-        mIsInstallingPlugin = false;
-


Thank you for cleaning this up 👍 !

mkevins

Thanks for this fix @irfano , and also for improving the nearby code. The code changes look good. I tested this on a Pixel 3a (physical device), first by reproducing the crash on trunk, and then confirming that it is fixed with this PR.

One observation I made: on trunk, I was not able to reproduce the issue on case 1, but only on case 2. Also, on both trunk and this PR, I observed that I had to leave the search screen and come back again to validate step 10 (i.e. I had to do step 4 twice 🤷‍♂️). Since this is not a regression (i.e. this PR fixes case 2 and does not introduce any new issue with step 1), I'm still approving. I just wanted to mention this, in case it was unexpected, or if it is a known existing issue.

irfano · 2023-08-07T08:10:07Z

I was not able to reproduce the issue on case 1, but only on case 2.

Initially, I couldn't reproduce it either, but later on, it started happening. In any case, it has been fixed now.

Also, on both trunk and this PR, I observed that I had to leave the search screen and come back again to validate step 10 (i.e. I had to do step 4 twice 🤷‍♂️)

Yes, I noticed that as well. The search screen needs to be updated after the plugin is installed. I will search for existing issues and open a new one for it.

Thank you for the review and testing cases comprehensively. 🚀

peril-wordpress-mobile · 2023-08-07T08:32:34Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

wpmobilebot · 2023-08-07T08:37:09Z

Found 1 violations:

The PR caused the following dependency changes:

-+--- org.wordpress:fluxc:{strictly trunk-544130cbf7535c5cbcbcced125fc49f22befe3bb} -> trunk-544130cbf7535c5cbcbcced125fc49f22befe3bb
-|    +--- 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-544130cbf7535c5cbcbcced125fc49f22befe3bb
-|    +--- 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.1 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.1 (*)
-|    |    \--- 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.1 (*)
-|    +--- 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.1 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.1 (*)
++--- org.wordpress:fluxc:{strictly trunk-b5b3d70cf01f082169bba319cf1ee2ff09787ea3} -> trunk-b5b3d70cf01f082169bba319cf1ee2ff09787ea3
+|    +--- 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-b5b3d70cf01f082169bba319cf1ee2ff09787ea3
+|    +--- 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.1 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.1 (*)
+|    |    \--- 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.1 (*)
+|    +--- 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.1 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.1 (*)
 \--- org.wordpress:login:1.4.0
-     \--- org.wordpress:fluxc:2.21.0 -> trunk-544130cbf7535c5cbcbcced125fc49f22befe3bb (*)
+     \--- org.wordpress:fluxc:2.21.0 -> trunk-b5b3d70cf01f082169bba319cf1ee2ff09787ea3 (*)

Please review and act accordingly

irfano added 2 commits August 4, 2023 14:33

Update FluxC reference to the PR

656785d

Prevent the crash on site plugin installation

9eb4e3f

irfano added Plugins [Type] Crash [Status] Not Ready for Merge labels Aug 4, 2023

irfano added this to the 23.0 milestone Aug 4, 2023

irfano requested a review from mkevins August 4, 2023 14:43

irfano self-assigned this Aug 4, 2023

irfano mentioned this pull request Aug 4, 2023

Disable caching in fetchPlugins() call wordpress-mobile/WordPress-FluxC-Android#2793

Merged

Update RELEASE-NOTES

5becb83

Merge branch 'trunk' into issue/15144-crash-on-plugin-detail-screen

aff53a3

mkevins reviewed Aug 7, 2023

View reviewed changes

mkevins self-requested a review August 7, 2023 04:34

mkevins approved these changes Aug 7, 2023

View reviewed changes

irfano removed the [Status] Not Ready for Merge label Aug 7, 2023

Update FluxC version to the trunk

5a1e0b7

irfano merged commit 626a4ee into trunk Aug 7, 2023

irfano deleted the issue/15144-crash-on-plugin-detail-screen branch August 7, 2023 09:35

irfano mentioned this pull request Aug 31, 2023

Fix: NPE on SiteMode in PluginDetailActivity #19110

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crash on the installation of a plugin that is already installed #18890

Crash on the installation of a plugin that is already installed #18890

irfano commented Aug 4, 2023 •

edited

Loading

wpmobilebot commented Aug 4, 2023 •

edited

Loading

wpmobilebot commented Aug 4, 2023 •

edited

Loading

mkevins Aug 7, 2023

mkevins left a comment

irfano commented Aug 7, 2023

peril-wordpress-mobile bot commented Aug 7, 2023

wpmobilebot commented Aug 7, 2023

Crash on the installation of a plugin that is already installed #18890

Crash on the installation of a plugin that is already installed #18890

Conversation

irfano commented Aug 4, 2023 • edited Loading

Fix on FluxC

Fix on WPAndroid

Regression Notes

wpmobilebot commented Aug 4, 2023 • edited Loading

wpmobilebot commented Aug 4, 2023 • edited Loading

mkevins Aug 7, 2023

Choose a reason for hiding this comment

mkevins left a comment

Choose a reason for hiding this comment

irfano commented Aug 7, 2023

peril-wordpress-mobile bot commented Aug 7, 2023

wpmobilebot commented Aug 7, 2023

The PR caused the following dependency changes:

irfano commented Aug 4, 2023 •

edited

Loading

wpmobilebot commented Aug 4, 2023 •

edited

Loading

wpmobilebot commented Aug 4, 2023 •

edited

Loading