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.

Sign up for GitHub

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

Jump to bottom

Update User.fetch to get a valid login list on mobile #2255

Merged
merged 1 commit into from
Apr 7, 2021
Merged

Update User.fetch to get a valid login list on mobile #2255

merged 1 commit into from
Apr 7, 2021

Conversation

NikkiWines
Copy link
Contributor

@NikkiWines NikkiWines commented Apr 7, 2021 •
edited
Loading

Details

Fixes an issue where sending an array for names for API.get returned an incorrect response when on mobile.

Fixed Issues

Fixes https://github.com//pull/2169#issuecomment-814533130

Tests

  1. Log into E.cash with an email address that doesn't have any secondary logins
  2. Navigate to Settings > Profile
  3. Confirm you only see only + Add Phone Number and not ` + Add Email Address

QA Steps

Repeat test steps.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

E.com screenshot with no secondaries:
image

Web

Mobile Web

Desktop

iOS

Android

@NikkiWines NikkiWines requested a review from a team as a code owner April 7, 2021 01:47
@NikkiWines NikkiWines self-assigned this Apr 7, 2021
@MelvinBot MelvinBot requested review from MariaHCD and removed request for a team April 7, 2021 01:47
@NikkiWines
Copy link
Contributor Author

Issue is only on iOS and Android, but added screenshots for all of them to be sure

@NikkiWines NikkiWines changed the title update User.fetch to get valid login list on mobile Update User.fetch to get a valid login list on mobile Apr 7, 2021
marcaaron
marcaaron approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

LGTM.

Approving, but with the caveat that this will probably will happen again in the future. Not sure if we need to actively avoid sending arrays as request params or handle them automatically or something else.

@NikkiWines
Copy link
Contributor Author

Merging since Marc approved.

@NikkiWines NikkiWines merged commit e57f19c into master Apr 7, 2021
@NikkiWines NikkiWines deleted the nikki-fix-profile-page-secondarylogins branch April 7, 2021 02:11
@OSBotify OSBotify mentioned this pull request Apr 7, 2021
Closed
85 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcaaron marcaaron marcaaron approved these changes

@MariaHCD MariaHCD Awaiting requested review from MariaHCD

Assignees

@NikkiWines NikkiWines

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NikkiWines @marcaaron