[$500] Room - Inconsistency in behaviour while adding VOIP number as contact via start chat&room
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.21-1 Reproducible in staging?: y Reproducible in production?: y 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 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
Action Performed:
- Go to https://staging.new.expensify.com/
- Tap on a room chat
- Tap on header---members----Invite
- Paste the VOIP number (+18183305299)
- Tap invite
- Note user can add VOIP number as contact
- Navigate to LHN
- Tap fab---Start chat
- Paste the VOIP number (+18183305299)
- Select the VOIP number (+18183305299) from the list
- Note error message is displayed
Expected Result:
User must be not be allowed to add a landline or VOIP contact to a room or other places in the app. The error message that should be returned is the same as if you try to invite them to a chat via steps 7-11 above. "The provided phone number belongs to a landline or VoIP, please use your email address instead."
Actual Result:
In room, Voip number allowed to add as contact but in start chat adding voip number showing error.Inconsistency in behaviour displayed.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [x] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/93399543/dc656da1-24d7-4059-8131-3d6f460edbaa
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01907badd5c1783209
- Upwork Job ID: 1742844313960140800
- Last Price Increase: 2024-01-18
Job added to Upwork: https://www.upwork.com/jobs/~01907badd5c1783209
Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Bug0 Triage Checklist (Main S/O)
- [ ] This "bug" occurs on a supported platform (ensure
Platformsin 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
Triggered auto assignment to Contributor-plus team member for initial proposal review - @0xmiroslav (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Discrepancy in behavior when adding a VoIP user in a workspace and starting a chat.
What is the root cause of that problem?
1 - Allow creating server request for invalid phone numbers (landline or VoIP), 2 - Inconsistency server response when dealing with invalid phone numbers (landline or VoIP).
Where users can invite/add others members/users: 1 - Invite member/user to a workspace 2 - Search a user and 'start chat' 3 - Invite member/user to a regular room 4 - FAB > Start chat 5 - FAB > Request money 6 - FAB > Send Money 7 - FAB > Assign Task
Backend response for then: 1 - "onyxData": [], - "message": "The provided phone number belongs to a landline or VoIP..." 2 - "onyxData": [ { ... createChat": { "1705430486982131": "The provided phone number belongs to a landline or VoIP...
3 - "onyxData": [ { "key": "report_3251340957676970", "onyxMethod": "merge", "value": { "participantAccountIDs": [ 16258318, 16177509, 16256252, 16256248, 16258226, 16205380 ], "participants": [ "[email protected]", "+18183305299", "+18183305295", "+18183305296", "+18173305299", "[email protected]" ], "visibleChatMemberAccountIDs": [ 16258318, 16177509, 16256252, 16256248, 16258226, 16205380 ] } },
4 -"onyxData": [ { ... createChat": { "1705430486982131": "The provided phone number belongs to a landline or VoIP... 5 - "onyxData": [], - "message": "The provided phone number belongs to a landline or VoIP..." 6 - "onyxData": [], - "message": "The provided phone number belongs to a landline or VoIP..."
7 - "onyxData": [ { "key": "report_8859913735881996", "onyxMethod": "merge", "value": { "description": "", "lastVisibleActionCreated": "2024-01-16 19:27:01.357", "managerID": 16258666, "ownerAccountID": 16205380, "parentReportID": "1566167401886507", "reportID": "8859913735881996", "reportName": "tes2", "state": "OPEN", "stateNum": 0, "statusNum": 0, "type": "task" } }, { "key": "personalDetailsList", "onyxMethod": "merge", "value": { "16205380": { "accountID": 16205380, "avatar": "https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/default-avatar_22.png", "displayName": "[email protected]", "firstName": "", ...
Online - UI behavior: 1 - Almost good - a "There was a problem adding ..." instead of "The provided phone number belongs to a landline or VoIP...". 2 - Looks good - Open a disabled report chat with a "The provided phone number belongs to a landline or VoIP..." message. 3 - Big Issue - We can add the member successfully. 4 - Looks good - Open a disabled report chat with a "The provided phone number belongs to a landline or VoIP..." message. 5 - Issue - Open a report full with only skeleton loading. 6 - Issue - Open a report full with only skeleton loading. 7 - Big Issue - We create a task and assign the member successfully.
Off - UI behavior:
1 - Looks good - Able to invite, disable look.
2 - Looks good - Able to start a chat and a conversation, disabled look.
3 - Issue - Able to invite, 'pressed' look after invite instead of disabled. https://github.com/Expensify/App/issues/34671
4 - Looks good - Able to start a chat and a conversation, disabled look.
5 - Looks good - Able to create a iou report and a conversation, disabled look.
6 - Looks good - Able to create a iou report and a conversation, disabled look.
7 - Looks good - Able to create a task report and a conversation, disabled look.
What changes do you think we should make in order to solve the problem?
~Create a landline or VoIP form validation.~ https://github.com/Expensify/App/issues/24519#issuecomment-1687078956
Since we need the backend to verify a number, we need Standardize backend responses when the check for VOIP and landline is true.
What alternative solutions did you explore? (Optional)
Updated the Expected behaviour in the OP
Expected Result:
User must be not be allowed to add a landline or VOIP contact to a room or other places in the app. The error message that should be returned is the same as if you try to invite them to a chat via steps 7-11 above. "The provided phone number belongs to a landline or VoIP, please use your email address instead."
@brunovjk @0xmiroslav and others, let's make sure we're testing any other ways/places a user can be added to ensure we surface the "The provided phone number belongs to a landline or VoIP, please use your email address instead." error everywhere. I want to fix all instances at once and not manage multiple GH issues.
I got this error while trying to add the number to a room. Is this still reproducible?
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@mallenexpensify, @0xmiroslav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
d'oh, tagged wrong person and not you @0xmiroslav , can you please review my comment above and the proposals? Thx
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@mallenexpensify @0xmiroslav this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@brunovjk Thanks for the proposal. I see you directly linked to https://github.com/Expensify/App/issues/24519#issuecomment-1687078956. Can you add some context behind it?
Can you add some context behind it?
@0xmiroslav although 'I think on top of the proposal, they should implement some sort of check in the front-end for whether the provided phone number is valid before creating an' report https://github.com/Expensify/App/issues/24519#issuecomment-1686742822 I understood that 'these validations are meant to happen on the backend since we can't easily verify if a phone number is VOIP (from its format, we can check but it's not dependable)' https://github.com/Expensify/App/issues/24519#issuecomment-1687078956
This is what I found on Github+Slack, maybe there is a better discussion on the subject, I don't know, but I understand the difficulty of trusting these validators on the frontend, and I wonder about the feasibility of standardizing responses when validation for an invalid number is true.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @cubuspl42 (External)
@cubuspl42 reassigning, please take over as C+. If you don't have bandwidth, unassign yourself. Thanks
We have one proposal so far
@brunovjk Do I understand correctly that you're suggesting no frontend changes, but to handle this on the backend instead?
@cubuspl42 Yes :) Does that make sense? I'm new here, still trying to understand how it all works.
It makes sense. Unfortunately, in such a case, the issue might have to be fixed internally without external compensation.
π π π
Triggered auto assignment to @danieldoglas, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@danieldoglas Would you take a look?
Very good, thank you @cubuspl42 :D
I agree with you. @mallenexpensify can you please turn this into an internal issue? I think this could probably be a part of vip-split.
Current assignee @cubuspl42 is eligible for the Internal assigner, not assigning anyone new.
Moved to Internal, @danieldoglas what's your reasoning for #vip-split? I want to post in that room and could use some 'ammo'/words to sell it. Thx
@danieldoglas can you comment on the above when you have a min?
what's your reasoning for #vip-split? I want to post in that room and could use some 'ammo'/words to sell it. Thx
Oh my bad, didn't see that comment before.
#vip-split is where I'm seeing the focus on 1:1 and group chats being created, as well as universal chat. So I think that's a good place where we should focus on limiting voip numbers.
@cubuspl42 @mallenexpensify this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!