-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [$1000] Bank account - Phone number field is not trimmed for spaces in connect bank account #25996
Comments
Triggered auto assignment to @garrettmknight ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Bank account - Phone number field is not trimmed for spaces in connect bank account What is the root cause of that problem?We're not trimming the space here:
What changes do you think we should make in order to solve the problem?We should trim the space before submitting companyPhone: parsePhoneNumber(values.companyPhone.trim(), {regionCode: CONST.COUNTRY.US}).number.significant, What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Bank account - Phone number field is not trimmed for spaces in connect bank account What is the root cause of that problem?This happens because we are not sanitizing the phone number input here; What changes do you think we should make in order to solve the problem?We should be using the companyPhone: parsePhoneNumber(values.companyPhone.replace(CONST.REGEX.NON_NUMERIC, ''), {regionCode: CONST.COUNTRY.US}).number.significant, This is better than just trimming with What alternative solutions did you explore? (Optional)xx |
ProposalPlease re-state the problem that we are trying to solve in this issue.Phone number field is not trimmed for spaces in connect bank account What is the root cause of that problem?When we save the value of company, it's trimmed before saving. We use
What changes do you think we should make in order to solve the problem?We can pass
What alternative solutions did you explore? (Optional)We can use a state in
We can do the same way this these other fields that are editable. ResultScreencast.from.11-09-2023.17.18.11.webm |
I think issue reporter was mistakenly marked to @dhanashree instead of @dhanashree-sawant, can we correct that if possible? It will help me to recieve emails about it |
@dhanashree-sawant updated the OP. Checking whether this is an issue for us on the backend or not before opening up. |
Confirmed that the expected behavior is that we'd update the app to clear leading spaces since we're cleaning that up on the backend. |
Job added to Upwork: https://www.upwork.com/jobs/~017eadfab4995c4b33 |
Current assignee @garrettmknight is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
Thanks for the proposals everyone. It does seem like Awesomephonenumber removes the spaces when using @dukenv0307 Your proposal seems promising but it's making the phone number field uneditable for me when we return to it. |
@Ollyws For taxID we also use this way and make this field uneditable so I think if we decided to display phoneNumber value with trimming, it will be fine. |
@garrettmknight, @Ollyws Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Asked for more clarification around which fields should be disabled in the slack thread. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@garrettmknight, @Ollyws Huh... This is 4 days overdue. Who can take care of this? |
@garrettmknight, @Ollyws Eep! 4 days overdue now. Issues have feelings too... |
PR is on Staging - will update to the awaiting payment hold when it goes to prod if Melv misses it. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-10-23. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-10-23. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
BugZero Checklist:
We can't really call this a regression from any PR, we just never considered to sanitize/trim the draft values after the form was submitted.
N/A
N/A
I don't think a regression test is helpful for this one as it's a minor aesthetic feature that doesn't affect the flow in any meaningful way. |
Summary of Payments:
New Upwork issue: https://www.upwork.com/en-gb/ab/applicants/1716919006499426304/job-details New offers out since the other job went stale - I'll pay when y'all accept. |
All paid up - closing. |
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:
Expected Result:
App should trim spaces before phone number value as it does for tax ID or legal name
Actual Result:
App should trim spaces before phone number value as it does for tax ID or legal name
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.57-5
Reproducible in staging?: Yes
Reproducible in production?: Yes
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
trim.spaces.in.phone.number.field.as.it.does.for.tax.ID.mp4
Recording.2989.mp4
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692249462405549
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: