Jetpack Focus: Open web links with Jetpack - Update App pref setting by zwarm · Pull Request #17514 · wordpress-mobile/WordPress-Android

zwarm · 2022-11-23T01:34:59Z

This PR changes the way the DeepLinkOpenWebLinksWithJetpackHelper determines when to show the overlay. Instead of relying on the activity component to be enabled, a new app pref was added to manage the value. Also included in this PR is the logic to enable/disable the actual deep link component. A future PR will add the functionality to update the setting based on the overlay yes/no response.

This setting is only visible:

When the Open Web Links With Jetpack flow is enabled
Jetpack is installed on the device
The current app is WordPress

Note: had to comment out the following unused methods to pass ci checks. This will be uncommented in the next PR.
setOpenWebLinksWithJetpackOverlayLastShownTimestamp

To test:
##Test: Setting is not shown when Feature Flag is enabled and JP is not installed##

Do a clean install of the WP app from this PR
Launch WP and login
Navigate to Me -> App Settings -> Debug
Enable the "open web links with jetpack" feature flag and restart the app
Navigate to Me -> App Settings
✅ Verify you don't see the "Open web links with Jetpack" setting (as in the screenshot above)

##Test: Setting is not shown when Feature Flag is disabled and JP is not installed##

Launch WP
Navigate to Me -> App Settings -> Debug
Disable the "open web links with jetpack" feature flag and restart the app
Navigate to Me -> App Settings
✅ Verify you don't see the "Open web links with Jetpack" setting (as in the screenshot above)

##Test: Setting is not shown within the JP app when FF is enabled##

Do a clean install of the JP app from this PR
Launch JP and login
Navigate to Me -> App Settings -> Debug
Enable the "open web links with jetpack" feature flag and restart the app
Navigate to Me -> App Settings
✅ Verify you don't see the "Open web links with Jetpack" setting (as in the screenshot above)

##Test: Setting is shown within the WP app when FF is enabled and JP is installed##

Make sure WP & JP are installed on the device
Launch the WP app
Enable the "open web links with jetpack" feature flag and restart the app
Navigate to Me -> App Settings
✅ Verify you do see the "Open web links with Jetpack" setting (as in the screenshot above)

Regression Notes

Potential unintended areas of impact
The app showing shows when it shouldn't
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual and unit testing
What automated tests I added (or what prevented me from doing so)
Add new tests to DeepLinkOpenWebLinksWithJetpackHelperTest

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.

…pack app setting. Add helper method for enableDisableOpenWithJetpackComponents'

wpmobilebot · 2022-11-23T01:51:11Z

📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17514-f39db35.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`WordPress`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`f39db35`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

wpmobilebot · 2022-11-23T01:55:23Z

📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17514-f39db35.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`Jetpack`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`f39db35`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

develric · 2022-11-23T11:15:04Z

WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsFragment.java

+        try {
+            mOpenWebLinksWithJetpackHelper.enableDisableOpenWithJetpackComponents(newValue);
+        } catch (Exception e) {
+            ToastUtils.showToast(
+                    getActivity(),
+                    (newValue ? R.string.preference_open_web_links_with_jetpack_setting_change_enable_error
+                            : R.string.preference_open_web_links_with_jetpack_setting_change_disable_error),
+                    ToastUtils.Duration.LONG);
+            AppLog.e(AppLog.T.UTILS, "Unable to enable or disable open with Jetpack components ", e);
+        }
+    }


Since this can fail, wonder if we should AppPrefs.setIsOpenWebLinksWithJetpack here based on the success/fail result. Since the feature is disabled behind a feature flag we can also address this in one of the upcoming PRs in case you think what I said makes sense 🙇 .

Absolutely great idea @develric . Changed and pushed in commit f39db35

develric

Thanks for the PR and the detailed test steps (super straightforward to follow 🙇 )! Works as described and code LGTM. Just left a comment inline (let me know if it makes sense, and we can address also in subsequent PRs since all is behind a FF for now). I'll wait for the minor checkstyle issue to be fixed and your follow up, but for all the rest this is good to land 👍 !

…ter it is determined if we could change the component setting

develric

Thanks for the changes 🙇 ! I'm merging it 👍 !

zwarm added 5 commits November 22, 2022 20:28

Add get-set methods for OpenWebLinksWithJetpack

bd2fc0c

Change backing pref for the open web links with Jetpack pref

3f72af8

Add fun for enable and disable components

09b62f5

Add app pref error message for openWebLinksWithJetpack

da007c5

Adjust the alogrithm for showOverlay to use the isOpenWebLinksWithJet…

fa8f2c8

…pack app setting. Add helper method for enableDisableOpenWithJetpackComponents'

zwarm self-assigned this Nov 23, 2022

zwarm added [Type] Task Jetpack Focus labels Nov 23, 2022

zwarm added this to the 21.3 milestone Nov 23, 2022

zwarm requested a review from develric November 23, 2022 01:36

zwarm changed the title ~~Issue/17494 open with jp hook on off app settings~~ Jetpack Focus: Open web links with Jetpack - Update App pref setting Nov 23, 2022

zwarm mentioned this pull request Nov 23, 2022

Jetpack Focus: Open web links with Jetpack #17494

Closed

15 tasks

develric reviewed Nov 23, 2022

View reviewed changes

Refactor: Move the setting of the open web links with jetpack pref af…

f39db35

…ter it is determined if we could change the component setting

develric approved these changes Nov 23, 2022

View reviewed changes

develric merged commit e980fc0 into trunk Nov 23, 2022

develric deleted the issue/17494-open-with-jp-hook-on-off-app-settings branch November 23, 2022 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack Focus: Open web links with Jetpack - Update App pref setting#17514

Jetpack Focus: Open web links with Jetpack - Update App pref setting#17514
develric merged 6 commits intotrunkfrom
issue/17494-open-with-jp-hook-on-off-app-settings

zwarm commented Nov 23, 2022 •

edited

Loading

Uh oh!

wpmobilebot commented Nov 23, 2022 •

edited

Loading

Uh oh!

wpmobilebot commented Nov 23, 2022 •

edited

Loading

Uh oh!

develric Nov 23, 2022 •

edited

Loading

Uh oh!

zwarm Nov 23, 2022

Uh oh!

develric left a comment

Uh oh!

develric left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zwarm commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Notes

Uh oh!

wpmobilebot commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

develric Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zwarm Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

develric left a comment

Choose a reason for hiding this comment

Uh oh!

develric left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zwarm commented Nov 23, 2022 •

edited

Loading

wpmobilebot commented Nov 23, 2022 •

edited

Loading

wpmobilebot commented Nov 23, 2022 •

edited

Loading

develric Nov 23, 2022 •

edited

Loading