Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

notandyvee
Copy link
Contributor

@notandyvee notandyvee commented Aug 1, 2024

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.

  • Trigger the malformed deeplink, like clicking the link.
  • You should see a toast that something went wrong and you should be taken to the authentication screen.

Release

N/A

@notandyvee notandyvee added the [Type] Bug Something isn't working. label Aug 1, 2024
@notandyvee notandyvee added this to the 2.33 milestone Aug 1, 2024
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Aug 1, 2024

📲 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
Commitb88bf41
Direct Downloadsimplenote-android-prototype-build-pr1673-b88bf41-0191daa8-a502-4e30-8937-e8caf4ebd5c1.apk

@danilo04 danilo04 self-assigned this Aug 30, 2024
@mokagio mokagio modified the milestones: 2.33, 2.34 Sep 10, 2024
@mokagio
Copy link
Contributor

mokagio commented Sep 10, 2024

@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:

  • trunk if reviewed and merged before the code freeze
  • release/2.34 otherwise

Thanks!

@dangermattic
Copy link
Collaborator

1 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 🚫 Danger

@mokagio mokagio changed the base branch from trunk to release/2.34 September 10, 2024 06:41
@mokagio
Copy link
Contributor

mokagio commented Sep 10, 2024

@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.

Copy link
Contributor

@danilo04 danilo04 left a comment

Choose a reason for hiding this comment

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

This is working as expected @notandyvee. Good job :shipit:

@notandyvee notandyvee merged commit bd79e42 into release/2.34 Sep 11, 2024
15 checks passed
@notandyvee notandyvee deleted the andy/firebird-issue-81 branch September 11, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug Something isn't working.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants