Automattic / simplenote-android

Simplenote for Android
https://simplenote.com
GNU General Public License v2.0
1.78k stars 301 forks source link

[Bug] If a deeplink is malformed or missing data we show auth screen. #1673

Closed notandyvee closed 1 month ago

notandyvee commented 2 months ago

Fix

There is an edge case with magic link deeplinking. If the url is malformed or the client cannot properly find the email/code as an example, the app doesn't ever show up. You do see a toast, but this is weird behavior. This PR updates that so that we show the auth activity in this case.

Before After
before-magic-link-no-work after-magic-link-no-working

Additionally, while I was here I realize the magic link on this screen was missing a loading indicator. It's not clear this is happening. On slower networks this could be an issue. A loading dialog will do for now.

Test

What I did to test is to put this link in google keep: https://app.simplenote.com/login?email=&auth_code=0V6GUL. You can open the link and it will trigger the deeplink. Do not paste it on chrome. Doing that ignores any deeplink.

Release

N/A

wpmobilebot commented 2 months ago

📲 You can test the changes from this Pull Request in Simplenote Android by scanning the QR code below to install the corresponding build.

App Name Simplenote Android
Build TypeDebug
Commitb88bf41eb4bf48b4440fc4b2b80ca08d973350fc
Direct Downloadsimplenote-android-prototype-build-pr1673-b88bf41-0191daa8-a502-4e30-8937-e8caf4ebd5c1.apk
mokagio commented 1 month ago

@notandyvee @danilo04 I'm working on more release automation and aim to have a quick 2.34 release to test it, see p1725313949594039-slack-C036Y8QL4

It would be great to get this reviewed so we can make it part of it?

I'll make sure to track here when the release starts, so that we can adjust the base branch accordingly:

Thanks!

dangermattic commented 1 month ago
1 Warning
:warning: This PR is assigned to the milestone 2.34 ❄️. This milestone is due in less than 4 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by :no_entry_sign: Danger

mokagio commented 1 month ago

@notandyvee @danilo04 FYI, I merged trunk in here because of a change in the GitHub check requirement which would otherwise have prevented this PR to land in either trunk or release/2.34.

Also, given the release 2.34 code freeze has started, I updated the target branch for this PR to it.