[Authentication] Add logic to login via magic link

Automattic / simplenote-android

Simplenote for Android

https://simplenote.com

GNU General Public License v2.0

1.75k stars 299 forks source link

[Authentication] Add logic to login via magic link #1660

Closed notandyvee closed 1 week ago

notandyvee commented 2 weeks ago

Fix

This PR adds the logic necessary to login via magic link. Instead of entering a password for login, you request an email. You go to the your email and click the login link in order to auto-magically login without password.

Test

TBD

Review

Nothing special was added. I avoided a major overhaul. For example, instead of json parsing with a library I used JSONObject and used OkHttp. See SimpleHttp. If we do more with network requests and json parsing it would be worth using retrofit and a better json parsing library, like Moshi.

Release

This change requires release notes.

dangermattic commented 2 weeks ago

	4 Warnings
:warning:	`strings.xml` files should only be updated on release branches, when the translations are downloaded by our automation.
:warning:	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
:warning:	Class `OkHttpMagicLinkRepository` is missing tests, but `unit-tests-exemption` label was set to ignore this.
:warning:	Class `SimpleHttp` is missing tests, but `unit-tests-exemption` label was set to ignore this.

Generated by :no_entry_sign: Danger

wpmobilebot commented 2 weeks 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 Type	Debug
	Commit	8cd6d4ac9f21b30912659818a5f55079e01d2546
	Direct Download	`simplenote-android-prototype-build-pr1660-8cd6d4a-01907a07-5b12-4167-a30f-703b57ca067c.apk`

notandyvee commented 1 week ago

Ok @danilo04 , ready for review. Sorry it took so long. I had trouble with unit tests. I attempted to migrate to flows as a result, but I don't feel comfortable using outdated libraries. I tried updating a few libs and it caused a cascading effect of updating way too many things. We can save it for another time. Just wanted to be clear why I didn't use flows.

Additionally, in order to test this end-to-end, you must build it in release mode with the right creds. DM me if you need help. Won't work in debug at the moment due to the creds and debug package name.

Also, do I need to do anything special with strings? It's giving me a warning. But I'm pretty sure I only edited the base strings.xml.

notandyvee commented 1 week ago

@danilo04 Ready for re-review. I will not add unit tests for the 2 classes the build was complaining about since it would essentially be testing library code. So added the unit test exemption. Appreciate the thorough review.