Issue/11799 show announcement on app upgrade #11804

khaykov · 2020-05-02T18:49:42Z

This PR adds logic for displaying feature announcements on the app upgrade.

We decided to show announcements independent of app version - eg. if you are on v4 of the app, and there is an announcement in v5, you will see even if you skip v5 and upgrade straight to v6 or v7 (until we remove or replace announcement manually).

To test (using wasabi build):

If you install this branch on top of develop - the announcement should be displayed when app starts.
If you perform the fresh installation, an announcement will not be displayed.
To trigger an announcement bump the versionCode number and restart the app.
Make sure the announcement is not shown in vanilla version.

PR submission checklist:

I have considered adding unit tests where possible.
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 · 2020-05-02T18:50:08Z

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

peril-wordpress-mobile · 2020-05-02T18:57:55Z

You can test the changes on this Pull Request by downloading the APK here.

oguzkocer · 2020-05-04T10:45:12Z

Hi there! 14.8 is being frozen, so this PR is being bumped to 14.9. If these changes have to be shipped with 14.8, please target release/14.8 and cc me in the PR so I can cut a new beta release. Also if you can keep the release dates in mind and bump the PRs & issues you are working on before the code freeze date, we'd really appreciate it.

Extra notes:
@khaykov There are a couple issues I want to note in this PR:

Can you make sure to add a reviewer for the PR when you create it?
Since this PR is opened on a Saturday before a code freeze, we expect one of 2 things to happen. Either ping platform9 and tell us that this PR needs to be included in the upcoming code freeze, in which case it really should have a reviewer and a notice that it's urgent. Or, target the next milestone. In the current state of the PR, it's not clear what the intention for the PR is, so we have to assume that it's OK to bump to the next release. Hope that makes sense!

Let me know if you have any questions!

…in view model.

khaykov · 2020-05-04T17:09:35Z

@oguzkocer Got it! Sorry, I totally missed that the code freeze is right around the corner for 14.8.

develric

Hi klym, I went into the code and left one comment about using list of announcements (not feeling strong about it so let me know wdyt). I was going into testing, but unless I'm doing something wrong for me the call to WordPress > initAnalytics (that updates the last versionCode with AppPrefs.setLastAppVersionCode(versionCode);) is called before the WPMainActivity > initViewModel > start (that makes the check on the versionCode); let me know if I'm missing something, thanks 😊

develric · 2020-05-04T22:58:37Z

WordPress/src/main/java/org/wordpress/android/util/BuildConfigWrapper.kt

+        return BuildConfig.VERSION_CODE
+    }
+
+    fun isFeatureAnnouncementEnabled(): Boolean {


Since we are introducing the wrapper, wdyt if we use it in AppSettingsFragment.java Line 172 also?

I added the check to AppSettingsFragment in upcoming PR :)

develric · 2020-05-04T23:16:24Z

WordPress/src/main/java/org/wordpress/android/ui/whatsnew/FeatureAnnouncementProvider.kt

@@ -27,7 +27,11 @@ class FeatureAnnouncementProvider @Inject constructor() {
    )
    )

-    fun getLatestFeatureAnnouncement(): FeatureAnnouncement {
+    fun getLatestFeatureAnnouncement(): FeatureAnnouncement? {


Not sure it's worth nor how much work it can be (so feel free to just say you will never consider doing 😊), but since we are possibly considering expanding the announcements one day to include some of the previous one and to avoid this (sometimes tricky) nullable, maybe we could consider to use a list of FeatureAnnouncement (eventually ordered by app version code or being empty)...wdyt?

I added the getFeatureAnnouncements(): List<FeatureAnnouncement> method in upcoming PR, but it's not really solving the nullability of the announcement, it just moves the check upstream.

khaykov · 2020-05-05T05:14:26Z

Updated the logic around app versions. Hope I didn't mess up anything this time :)

The announcement should not be shown:

Fresh app install.
Updating from develop branch to this one without version code change.
If the announcement with the same version has been already shown.

The announcement should be shown:

When upgrading app (even when you skip some versions) and the announcement was not shown before.

develric

Hi @khaykov 👋! Thanks for the changes, works really well now 👍! LGTM

khaykov added 6 commits April 30, 2020 19:12

Added version code provider.

f4d1fb3

Added version code to feature announcement.

06492c9

Added feature announcement prefs

0deadf4

Updated main view model and tests

08be308

Linked feature announcement request to main activity.

f908cf4

Updated announcement display logic.

16165c8

khaykov added [Status] Needs Code Review What's New labels May 2, 2020

khaykov added this to the 14.8 milestone May 2, 2020

Fixed lint issues.

06052ec

oguzkocer modified the milestones: 14.8, 14.9 May 4, 2020

Added temporary feature flag to lock the feature announcement from ma…

9f50e49

…in view model.

khaykov requested a review from develric May 4, 2020 17:22

khaykov added 2 commits May 4, 2020 11:59

More solid upgrade check logic in relation to feature announcement.

1f42fec

Fixed test method name lenght

1d7fd07

develric reviewed May 5, 2020

View reviewed changes

Updated announcement display logic.

bf7db04

Updated comment.

abae59f

develric approved these changes May 6, 2020

View reviewed changes

develric merged commit 3f5f061 into develop May 6, 2020

khaykov deleted the issue/11799-show-announcement-on-app-upgrade branch June 26, 2020 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/11799 show announcement on app upgrade #11804

Issue/11799 show announcement on app upgrade #11804

khaykov commented May 2, 2020 •

edited

Loading

peril-wordpress-mobile bot commented May 2, 2020 •

edited

Loading

peril-wordpress-mobile bot commented May 2, 2020 •

edited

Loading

oguzkocer commented May 4, 2020

khaykov commented May 4, 2020

develric left a comment •

edited

Loading

develric May 4, 2020

khaykov May 5, 2020

develric May 4, 2020

khaykov May 5, 2020

khaykov commented May 5, 2020

develric left a comment

Issue/11799 show announcement on app upgrade #11804

Issue/11799 show announcement on app upgrade #11804

Conversation

khaykov commented May 2, 2020 • edited Loading

peril-wordpress-mobile bot commented May 2, 2020 • edited Loading

peril-wordpress-mobile bot commented May 2, 2020 • edited Loading

oguzkocer commented May 4, 2020

khaykov commented May 4, 2020

develric left a comment • edited Loading

Choose a reason for hiding this comment

develric May 4, 2020

Choose a reason for hiding this comment

khaykov May 5, 2020

Choose a reason for hiding this comment

develric May 4, 2020

Choose a reason for hiding this comment

khaykov May 5, 2020

Choose a reason for hiding this comment

khaykov commented May 5, 2020

develric left a comment

Choose a reason for hiding this comment

khaykov commented May 2, 2020 •

edited

Loading

peril-wordpress-mobile bot commented May 2, 2020 •

edited

Loading

peril-wordpress-mobile bot commented May 2, 2020 •

edited

Loading

develric left a comment •

edited

Loading