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

Use CreateLogin authToken instead of relying on reauthenticate #5438

Merged
merged 6 commits into from
Oct 25, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Sep 23, 2021

Details

Removes code that sends authToken to Authenticate and uses response of CreateLogin instead

Holding on Web-Expensify 32001 to deploy

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/178434

Tests

Easy to test on web but slightly trickier on mobile web. You'll need to change the route in Web-Expensify here to go to an ngrok route. So basically set up one ngrok route for expensify.com.dev and also set up a route for localhost:8080 then modify the web-e code to bring you to the ngrok route for localhost:8080. Then you can test the mobile web flow on a mobile device.

QA Steps

See https://github.com/Expensify/Web-Expensify/pull/32001

Tested On

  • Web
  • Mobile Web

The flow we are testing doesn't work on Desktop or iOS because there is no deep link support for the /transition route there yet. For Android technically these links can be used but it is very uncommon for that to happen + I wouldn't necessarily expect it to work. We can look into making the transition work for these platforms but doesn't feel like a priority right now.

  • N/A Desktop
  • N/A iOS
  • N/A Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron self-assigned this Sep 23, 2021
@marcaaron marcaaron marked this pull request as ready for review September 23, 2021 03:23
@marcaaron marcaaron requested a review from a team as a code owner September 23, 2021 03:23
@MelvinBot MelvinBot requested review from HorusGoul and removed request for a team September 23, 2021 03:24
iwiznia
iwiznia previously approved these changes Sep 23, 2021
HorusGoul
HorusGoul previously approved these changes Sep 23, 2021
@marcaaron marcaaron dismissed stale reviews from HorusGoul and iwiznia via efa7206 September 27, 2021 18:50
@marcaaron marcaaron changed the title [HOLD] Use CreateLogin authToken instead of relying on reauthenticate Use CreateLogin authToken instead of relying on reauthenticate Oct 21, 2021
@marcaaron
Copy link
Contributor Author

Ok taking this one off HOLD now. Should be ready for review. 🙇

@HorusGoul HorusGoul merged commit 59f8a3a into main Oct 25, 2021
@HorusGoul HorusGoul deleted the marcaaron-useCreateLoginToken branch October 25, 2021 09:35
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @HorusGoul in version: 1.1.8-15 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants