Skip to content

Conversation

@AjeshRPai
Copy link
Contributor

@AjeshRPai AjeshRPai commented May 20, 2022

Fixes #16477

Dependant FluxC PR wordpress-mobile/WordPress-FluxC-Android#2415

This PR fixes the issue where the explore plans were visible on the Quickstart task list. This PR fixes the following scenario. which was missed from the previous PR to fix the removal of plans.

Quick start in Progress with Explore Plans Quick start in Progress after Removing Explore plans
Screenshot 2022-05-20 at 5 44 00 PM Screenshot 2022-05-20 at 5 44 34 PM

Note: The Quick start task title and description for Follow Other Sites and Check your stats has been changed as part of the QS-V2 Project.

To Test 1

🟢 Test Scenario 1 - Explore Plans + Other tasks pending

  • Install the app having the Explore plans listed (E.g. from release/19.7 branch).
  • Login to the app with a quick start in progress.
  • Complete all tasks, excluding Explore plans + one task like Check your stats.
  • Install the app from the current PR.
  • Notice that Quick Start is not completed, and other tasks are shown as Pending
  • Notice that Explore Plans is not shown in Quick start task list

🟢 Test Scenario 2 - Only Explore Plans Pending

  • Install the app having the Explore plans listed (E.g. from release/19.7 branch).
  • Login to the app with a quick start in progress.
  • Complete all tasks, excluding Explore plans
  • Install the app from the current PR.
  • Notice that Quick Start is completed and Notification is shown

Note: In this PR, we update the database schema version to remove the explore plans with migration. Hence, After installing the build from this branch, when you try to install another build from the trunk or release/19.7 with the previous schema version. The app will crash. To avoid that, clean and install any build after installing the build from this branch.

Merge Instructions

  • Make sure dependant PR is merge
  • Update FluxC Version
  • Remove Status not ready to merge label
  • Merge

Regression Notes

  1. Potential unintended areas of impact
    Quick start list not being shown properly

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing

  3. What automated tests I added (or what prevented me from doing so)

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.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 20, 2022

You can test the changes on this Pull Request by downloading the APKs:

@AjeshRPai AjeshRPai changed the title Removes: Removes Explore Plan from Quick start list Fix: Removes Explore Plan from Quick start list May 20, 2022
@AjeshRPai
Copy link
Contributor Author

@ParaskP7 Pinging you here(again 😄 ) to inform you that we would like to include this fix in the next beta release.

Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

Tested given scenarios. Works as described. 👍

@ParaskP7
Copy link
Contributor

Thanks for the ping @AjeshRPai ! 👋

When you are ready and @ashiagr approves this PR, just let me know and I'll merge it. 💯

PS: I am currently seeing 0 file changes atm, I am not sure why, but you are probably on top of that, right? 😅

@ParaskP7
Copy link
Contributor

😄

When you are ready and @ashiagr approves this PR, just let me know and I'll merge it. 💯

Then...

Tested given scenarios. Works as described. 👍

@ashiagr you are too fast for me! 😛

@AjeshRPai
Copy link
Contributor Author

AjeshRPai commented May 20, 2022

@ParaskP7 We have made the necessary changes in FluxC and there would only be a change in the fluxC Version in the gradle file in this PR. Dependant FluxC PR wordpress-mobile/WordPress-FluxC-Android#2415.

@ParaskP7
Copy link
Contributor

@ParaskP7 We have made the necessary changes in FluxC and there would only be a change in the fluxC Version in the gradle file in this PR. Dependant FluxC PR wordpress-mobile/WordPress-FluxC-Android#2415.

@AjeshRPai got it, but I am seeing that the FluxC version on the release/19.9 branch is already pointing to 1.42.1. Thus, this PR is of 0 file changes. We can merge it anyway, to include your 2 commits, but I just wanted to mention that in case we are missing something here. 🙏

@ParaskP7
Copy link
Contributor

FYI: @AjeshRPai I am seeing this previous Release PR, 2 days ago, which actually did the 1.42.1 FluxC update, but I also see that @ashiagr created the draft 1.42.1 FluxC release an hour ago. I am a bit confused to be honest. Please let me know that everything is as you expected and that we are not missing something. If you provide a bit more info for me to understand it better it would be great.

PS: Apologies for not picking-up fast enough with all this, I just got the release from Olivier yesterday and trying not to mess it up for him... 😅

@AjeshRPai
Copy link
Contributor Author

AjeshRPai commented May 20, 2022

@ParaskP7 This part was a little bit confusing to me as well. I will try to explain as much as I understand.

We created the tag 1.42.1 to fix the plans issue with the Release PR. What we intended to do today was to delete the present tag 1.42.1 since it included only our change from the previous PR and create the tag 1.42.1 again to include the change from this PR. But this didn't go as planned; we got some build error from S3 in the FLUXC CI Build that 1.42.1 is already published. Now we have created release/1.42.2 and we are planning to create tag 1.42.2. We will update the fluxC Version in this PR and remove the not ready for merge label once the tag is successfully published.

@ashiagr Hope this is correct 😅

Again: Am sorry for not providing this info earlier so that you could get the context.

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@wpmobilebot
Copy link
Contributor

Found 1 violations:

The PR caused the following dependency changes:

-+--- org.wordpress:fluxc:{strictly 1.42.1} -> 1.42.1
-|    +--- org.wordpress:wellsql:1.7.0
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:1.7.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:1.42.1
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.2 (*)
-|    +--- 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.2.0
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.3.1 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.2.0 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.1.0 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.10 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.3.1 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.1.0 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.1.0-beta01 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.2 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.6.10 (*)
-|    +--- com.google.code.gson:gson:2.8.5
-|    +--- org.apache.commons:commons-text:1.1 (*)
-|    +--- androidx.room:room-runtime:2.4.2 (*)
-|    +--- androidx.room:room-ktx:2.4.2
-|    |    +--- androidx.room:room-common:2.4.2 (*)
-|    |    +--- androidx.room:room-runtime:2.4.2 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 (*)
-|    +--- com.google.dagger:dagger:2.29.1 -> 2.41 (*)
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.5.2 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.5.2 (*)
++--- org.wordpress:fluxc:{strictly 1.42.2} -> 1.42.2
+|    +--- org.wordpress:wellsql:1.7.0
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:1.7.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:1.42.2
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.2 (*)
+|    +--- 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.2.0
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.3.1 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.2.0 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.1.0 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.10 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.3.1 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.1.0 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.1.0-beta01 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.2 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.6.10 (*)
+|    +--- com.google.code.gson:gson:2.8.5
+|    +--- org.apache.commons:commons-text:1.1 (*)
+|    +--- androidx.room:room-runtime:2.4.2 (*)
+|    +--- androidx.room:room-ktx:2.4.2
+|    |    +--- androidx.room:room-common:2.4.2 (*)
+|    |    +--- androidx.room:room-runtime:2.4.2 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.10 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 (*)
+|    +--- com.google.dagger:dagger:2.29.1 -> 2.41 (*)
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.5.2 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.5.2 (*)
 \--- org.wordpress:login:0.14.0
-     \--- org.wordpress:fluxc:trunk-1436f46fc296ede0e65e117bbfced38fa34cec6d -> 1.42.1 (*)
+     \--- org.wordpress:fluxc:trunk-1436f46fc296ede0e65e117bbfced38fa34cec6d -> 1.42.2 (*)

Please review and act accordingly

@ParaskP7
Copy link
Contributor

ParaskP7 commented May 20, 2022

Oh, this all makes sense now @AjeshRPai ! Thank you so much for taking the time to explain it to me! 🙇

@ashiagr
Copy link
Contributor

ashiagr commented May 20, 2022

Thanks, @AjeshRPai! Your explanation is correct.

👋 @ParaskP7,

We didn't realize that 1.42.1 FluxC tag that was created 2 days ago was already published to S3 and cannot be republished to point to another commit until we saw this error. To resolve the issue, we created a new 1.42.2 FluxC tag which has all the changes from 1.42.1 tag + FluxC fix for this PR, more details here: wordpress-mobile/WordPress-FluxC-Android#2417.

I'll update 1.42.1 tag draft release with the details.

Let me know if anything is not clear. I'll not merge this PR until everyone is on the same page. Really sorry about any inconvenience due to this. 🙏

@ParaskP7
Copy link
Contributor

👋 @ashiagr and thanks for your input, I am all onboard now with what's happening, thank you! 🙇

@ashiagr
Copy link
Contributor

ashiagr commented May 20, 2022

When you are ready and @ashiagr approves this PR, just let me know and I'll merge it.

@ParaskP7 this PR is ready to be merged.

Thank you, both of you!

EDIT: I'm merging it to keep release/19.9 up-to-date before the weekend.

@ashiagr ashiagr merged commit 7bf0b48 into release/19.9 May 20, 2022
@ashiagr ashiagr deleted the issue/16477-remove-explore-plans-quickstart branch May 20, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants