Skip to content

Jetpack Focus: Open web links with Jetpack - Initialization#17497

Merged
develric merged 6 commits intotrunkfrom
issue/17494-open-web-links-initialization
Nov 22, 2022
Merged

Jetpack Focus: Open web links with Jetpack - Initialization#17497
develric merged 6 commits intotrunkfrom
issue/17494-open-web-links-initialization

Conversation

@zwarm
Copy link
Copy Markdown
Contributor

@zwarm zwarm commented Nov 20, 2022

Parent #17494

The purpose of this PR is to handle the initialization tasks needed to support the open web links with Jetpack flow.

Modifications Include

  • Created OpenWebLinksWithJetpackFlowFeatureConfig to manage the feature flow
  • Enabled the flow for WordPress only in build.gradle
  • Added new app pref for OPEN_WEB_LINKS_WITH_JETPACK_OVERLAY_LAST_SHOWN_TIMESTAMP
  • Created DeepLinkOpenWebLinksWithJetpackHelper to manage the rules related to determine when to show the overlay
  • Added DeepLinkOpenWebLinksWithJetpackHelperTest to test the rules
  • Added FirebaseRemoteConfigWrapper to support unit testing for frequency threshold
  • Added PackageManagerWrapper to support unit testing when checking if the deep link handler component is enabled/disabled

Merge Instructions

  1. Ensure that Jetpack Focus: Open web links with Jetpack - Add custom scheme deep link receiver #17495 has been merged
  2. Remove the "Not ready for merge" label
  3. Merge as normal

To test:
(1) To test the overlay shown rules.

  • Download this branch
  • Run the tests in DeepLinkOpenWebLinksWithJetpackHelperTest

(2) To test the feature flag

  • Launch the app.
  • Go to My Site -> Me -> App Settings -> Debug settings.
  • ✅ Verify that the open_web_lnks_with_jetpack_flow exists

Regression Notes

  1. Potential unintended areas of impact
    N/A

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

  3. What automated tests I added (or what prevented me from doing so)
    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.

@zwarm zwarm added this to the 21.3 milestone Nov 20, 2022
@zwarm zwarm self-assigned this Nov 20, 2022
@zwarm zwarm changed the base branch from trunk to issue/17494-custom-scheme-deep-link-receiver November 20, 2022 20:01
@zwarm zwarm requested review from AjeshRPai and develric November 20, 2022 20:09
@wpmobilebot
Copy link
Copy Markdown
Contributor

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17497-ccb280d.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commitccb280d
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Copy Markdown
Contributor

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17497-ccb280d.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commitccb280d
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

Copy link
Copy Markdown
Contributor

@AjeshRPai AjeshRPai left a comment

Choose a reason for hiding this comment

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

@zwarm

I have gone through the code changes and have run the unit tests. Everything looks good to me 👍🏼 .

I am approving but not merging the PR so that @develric can have a look. Please merge when convenient.

Base automatically changed from issue/17494-custom-scheme-deep-link-receiver to trunk November 21, 2022 21:20
Copy link
Copy Markdown
Contributor

@develric develric left a comment

Choose a reason for hiding this comment

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

Hi @zwarm 👋 , code LGTM and the to test section worked for me as well. I only left a comment about build.gradle feature activation flags in WP and JP. Let me know, and happy to move this forward after that 🙇

buildConfigField "String", "TRACKS_EVENT_PREFIX", '"wpandroid_"'
buildConfigField "String", "PUSH_NOTIFICATIONS_APP_KEY", '"org.wordpress.android"'
buildConfigField "boolean", "ENABLE_QRCODE_AUTH_FLOW", "false"
buildConfigField "boolean", "ENABLE_OPEN_WEB_LINKS_WITH_JP_FLOW", "false"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this mean it's disabled for WP and enabled for JP? If so I thought we needed the reverse maybe, wdyt?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@develric This has been changed in a subsequent PR and will have no effect until the future.

Copy link
Copy Markdown
Contributor

@develric develric left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification @zwarm 👍 ! Approving this one :shipit:

@develric develric merged commit 2c80914 into trunk Nov 22, 2022
@develric develric deleted the issue/17494-open-web-links-initialization branch November 22, 2022 14:36
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.

4 participants