[HOLD for payment 2023-05-22] [$1000] On add new contact page red dot error of delete contact method has no horizontal padding

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Launch the app -> Login in account
2. Navigate to setting -> Navigate to profile -> Navigate to contact method
3. Try to add new contact (might be any random email) -> Navigate to same newly added contact (Might need to add multiple contact and delete to replicate error)
4. Try to delete that contact -> that action will navigate to back
5. Go again to same contact (notice red dot error has no horizontal space)

Expected Result:

There should be horizontal padding should be match other component

Actual Result:

There is no horizontal padding on error indicator

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.8.8
Reproducible in staging?: Needs reproduction ( I am not able to trigger the error following the steps)
Reproducible in production?: Needs reproduction
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Simulator.Screen.Recording.-.iPhone.14.-.2023-05-02.at.13.23.12.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhairyasenjaliya
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1683015381305629

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01787da3b34f6241f3
• Upwork Job ID: 1654173559666835456
• Last Price Increase: 2023-05-04

[MelvinBot]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[dhairyasenjaliya]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• On add new contact page red dot error of delete contact method has no horizontal padding 

What is the root cause of that problem?

• The root cause is we rendering <OfflineWithFeedback> without wrapping into any view or without adding any padding/margin of horizontal, where else other <OfflineWithFeedback> in same page are wrapped in <View> or has padding

What changes do you think we should make in order to solve the problem?

• We just need to add paddingHorizontal: 20 which is styles.ph5 which we are already using in other component of the page (optionally we can add mh5 as well)

   <OfflineWithFeedback
         pendingAction={lodashGet(loginData, 'pendingFields.deletedLogin', null)}
         errors={ErrorUtils.getLatestErrorField(loginData, 'deletedLogin')}
+        errorRowStyles={[styles.mt6, styles.ph5]}
         onClose={() => User.clearContactMethodErrors(contactMethod, 'deletedLogin')}
   >

What alternative solutions did you explore? (Optional)

• We can add paddingHorizontal to main parent <View> and need to adjust all other component


  App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.js

  Line 261 in ce3b938

  <OfflineWithFeedback

• We can also wrap <OfflineWithFeedback> inside <View> with paddingHorizontal

[trjExpensify]

@dhairyasenjaliya how are you reliably reproducing this?

@Beamanator might be able to give us some clues on how to get the error to show. Bit of a sidebar, but I think this is another error message that is kind of unclear.

[dhairyasenjaliya]

@trjExpensify well I was able to replicate yesterday perhaps the error was resolved by backend but on front end we definitely need to add padding on that component since it overflows

[Beamanator]

@Beamanator might be able to give us some clues on how to get the error to show. Bit of a sidebar, but I think this is another error message that is kind of unclear.

@trjExpensify honestly this one is very hard to get to show, errors can show up when:

1. You are trying to delete a contact method that was already deleted
2. The login you are trying to delete isn't valid
3. You're trying to delete a primary / default login

So maybeeeee you can reproduce with:

1. Log in to one account in old dot & new dot
2. Quickly set a new default login in old dot, then in newdot try to delete the login you just set as default

I think you'd have to do this pretty fast 🤷 Orrrr maybe you can go offline in newdot while you're setting the login as default, then queue up the "delete login" from newdot, go online, then you may be able to see the error

[dhairyasenjaliya]

@trjExpensify I just reproduced the error just by adding a new email as capital words let's say add this
mail -> RANDOM@MAIL.COM -> now try to delete this you will get an error

simulator-screen-recording-iphone-14-2023-05-04-at-161310_5lJ0UJ2y.mp4

[dhairyasenjaliya]

hey @Beamanator I'm working on migrating <Form> inside newContactMethod I notice we are throwing a backend error on deleting emails that are in the capital do you think we should send email always as the small letter we can add one more validation during form submission in order to prevent this auth error? or just fix create an issue to tackle fix this backend error for that

[trjExpensify]

Still can't get the auth error using capitals:

Why do we not allow capitals either? Shouldn't that just treat them as lowercase effectively?

errors can show up when:
The login you are trying to delete isn't valid

That's kind of weird isn't it, @Beamanator? Why would we throw an error when you're deleting a login that isn't valid? Can't you just delete it? 😕

@trjExpensify well I was able to replicate yesterday perhaps the error was resolved by backend but on front end we definitely need to add padding on that component since it overflows

@dhairyasenjaliya, you seem to know where the padding issue lies in the code, so let's just move forward and get that fixed. I'll add external to review your proposal.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01787da3b34f6241f3

[MelvinBot]

Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[MelvinBot]

Triggered auto assignment to @francoisl (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mananjadhav]

I think @dhairyasenjaliya's proposal is good to go.

@trjExpensify @Beamanator @francoisl I can reproduce this reliably with a new account created with magic code (passwordless auth) and using all caps email as @dhairyasenjaliya mentioned.

[dhairyasenjaliya]

All right waiting for 🟢 to create PR @trjExpensify

[trjExpensify]

That'll be on @francoisl to provide! :)

@trjExpensify @Beamanator @francoisl I can reproduce this reliably with a new account created with magic code (passwordless auth) and using all caps email as @dhairyasenjaliya mentioned.

Interesting. So is there potentially another issue here with why this DeleteLogin auth error shows under that condition? (New account with passwordless auth)?

[dhairyasenjaliya]

@trjExpensify do you think we need to investigate and track a new issue if user try to delete account with capital letter it throws error (but i do think probably backend needs some additional checks since we just hit the API to delete an contact)

[trjExpensify]

Yeah, I think it's very strange that you can 1) add an email login with all caps 2) but get an error when you try and delete it. Further, it doesn't seem to be in all condition, and something specific to the new account creation flows maybe? So we should nail down the exact conditions to reproduce it.

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[luacmartins]

@mallenexpensify just re-added the Bug label since @trjExpensify is ooo and the payment is overdue. Could you please help with that?

[mallenexpensify]

@mananjadhav @dhairyasenjaliya can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01787da3b34f6241f3

[dhairyasenjaliya]

Accepted @mallenexpensify

[mananjadhav]

Accepted @mallenexpensify

[trjExpensify]

I'm back, and settled both of these. 👍

[trjExpensify]

Ah wait, @dhairyasenjaliya should have been paid $1,250. Just given the $250 bonus now.

[trjExpensify]

Okay, we're done here. I agree that we don't need a regression test for a display issue. 👍

[dhairyasenjaliya]

Hey @trjExpensify pr was merged within 3 day so should be eligible for speed bonus as well

[trjExpensify]

Cool, I've done that as a bonus as well, so to confirm final amounts sent to you both:

$1,750 - @dhairyasenjaliya for the fix, bug report and #urgency bonus
$1,500 - @mananjadhav for the C+ review and #urgency bonus