Skip to content

Changes related to deep linking email marketing#9957

Merged
malinajirka merged 6 commits intodevelopfrom
issue/9864-deep-linking-email-marketing
Jun 26, 2019
Merged

Changes related to deep linking email marketing#9957
malinajirka merged 6 commits intodevelopfrom
issue/9864-deep-linking-email-marketing

Conversation

@maxme
Copy link
Copy Markdown
Contributor

@maxme maxme commented May 29, 2019 •
edited
Loading

Fixes #9864

To test:

  • Should not open the app: https://wordpress.com/post/
  • Should not open the app: https://public-api.wordpress.com/nope/
  • Should not open the app: https://public-api.wordpress.com/bar/?redirect_to=https%3A%2F%2FCHANGEME.wordpress.com%2Fpost
  • Should open the app's editor on previously selected site: https://wordpress.com/post/wrong-hostname
  • Should open the app's editor with selected site CHANGEME: https://wordpress.com/post/CHANGEME.wordpress.com
  • Should open the app's editor with selected site CHANGEME: https://public-api.wordpress.com/mbar/?redirect_to=https%3A%2F%2Fwordpress.com%2Fpost%2FCHANGEME.wordpress.com
  • Should "open" the app, but then redirect to the browser: https://public-api.wordpress.com/mbar/?redirect_to=https%3A%2F%2Fwordpress.com%2Fnope

Note: this is a draft PR because backend isn't updated yet and the deep linking urls might changes (mobile-api)

This can be tested with ADB:

$ adb shell am start -a android.intent.action.VIEW \
 -c android.intent.category.BROWSABLE \
 -d "https://public-api.wordpress.com/mbar/?redirect_to=https%3A%2F%2Fwordpress.com%2Fpost%2FCHANGEME.wordpress.com"

Real test:

  • Create a new site.
  • Publish a post.
  • Switch to another site (to make sure the editor is opened for the right site later).
  • Wait for the "Congrats for your first post" marketing email.
  • Tap the "Create new post" button, you should be able to choose between your browser or the WordPress app, select the WordPress app, the editor should open on this site.

Demo video of this ^

cc @diegoreymendez

@maxme maxme added this to the 12.7 milestone May 29, 2019
@diegoreymendez diegoreymendez mentioned this pull request Jun 4, 2019
Merged
1 task
@nbradbury
Copy link
Copy Markdown
Contributor

nbradbury commented Jun 5, 2019

Just a friendly reminder to please make sure to manually merge develop into this branch before you merge this PR, as develop now contains the AndroidX migration. The automatic merge can succeed but still break the build. Better to find out now before it breaks develop. Thanks!

@loremattei loremattei modified the milestones: 12.7 ❄️, 12.8 Jun 17, 2019
@maxme
Copy link
Copy Markdown
Contributor Author

maxme commented Jun 25, 2019

This one is now ready for review: server side code has been deployed and marketing emails have been updated.

@maxme maxme marked this pull request as ready for review June 25, 2019 10:16
@maxme maxme requested a review from malinajirka June 25, 2019 10:16
malinajirka
malinajirka approved these changes Jun 26, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @maxme, I've tested all the scenarios and reviewed the code and it works as expected.

It's a shame Uri is Android's class which can't be easily used in unit tests. It'd be great to have unit tests for this logic. I guess, we could create a wrapper, but I'm not sure it's worth it.

@malinajirka malinajirka merged commit b5f9cdf into develop Jun 26, 2019
@malinajirka malinajirka deleted the issue/9864-deep-linking-email-marketing branch June 26, 2019 10:43
@hypest
Copy link
Copy Markdown
Contributor

hypest commented Mar 27, 2020

FWIW, looks like the changes in the manifest (https://github.com/wordpress-mobile/WordPress-Android/pull/9957/files#diff-5f7bbc0f00e829f6e38e74e2e89f5c43R375-R388) are breaking the wordpress://viewpost deeplink, causing Error: Activity not started, unable to resolve Intent. Not sure if that was intentional or not.

@maxme
Copy link
Copy Markdown
Contributor Author

maxme commented Mar 27, 2020

@hypest I'm able to reproduce the problem with:

adb shell am start \
 -W -a android.intent.action.VIEW \
 -c android.intent.category.BROWSABLE \
 -d "wordpress://viewpost?blogId=1\&postId=1"

I read the documentation again and I don't understand why this is not working, even though there is a side effect that I didn't anticipated (see the part about merging schemes and hosts in the doc: https://developer.android.com/training/app-links/deep-linking).

@maxme maxme mentioned this pull request Mar 27, 2020
Merged
3 tasks
@hypest
Copy link
Copy Markdown
Contributor

hypest commented Mar 27, 2020

Should have mentioned earlier, props to @renanferrari for noticing that the wordpress://viewport format wasn't working for him while trying to verify that deeplinking works, and reached out to me for collaboration.

@designsimply designsimply mentioned this pull request Nov 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka malinajirka approved these changes

Assignees

No one assigned

Labels

Site Creation

Projects

None yet

Milestone

12.8

Development

Successfully merging this pull request may close these issues.

Site Creation: Fix deep linking in first-post congratulations email.

6 participants

@maxme @nbradbury @hypest @malinajirka @diegoreymendez @loremattei