Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$500] IOU - IOU with hidden shows hmm not here &avatar beside IOU changes to green avatar #34839

Closed lanitochka17 closed 7 months ago

lanitochka17 commented 8 months ago

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


Version Number: 1.4.28 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:

  1. Launch app
  2. Tap fab --- Start chat
  3. Enter +84387025654 and select the contact
  4. Note conversation header changed as hidden
  5. Tap plus icon near compose--- Request money--- manual
  6. Enter an amount and tap next
  7. Tap request
  8. Note avatar displayed beside newly created IOU
  9. Tap on newly created IOU to open IOU details page
  10. Tap back button
  11. Note in conversation page, beside IOU newly created, avatar changed to default green offline avatar
  12. Tap on newly created IOU again to open IOU details page

Expected Result:

Request money with specific "hidden" number user and opening IOU details page must not show "hmm not here" page and avatar beside newly created IOU must not change to green offline default avatar

Actual Result:

Request money with specific "hidden" number user and opening IOU details page shows "hmm not here" page and avatar beside newly created IOU changes to green offline default avatar.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/d217d8f0-ad2d-4afe-ae2b-e3ed7e7eff24

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a1cca1664f122e46
  • Upwork Job ID: 1748454958646505472
  • Last Price Increase: 2024-01-19
melvin-bot[bot] commented 8 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01a1cca1664f122e46

melvin-bot[bot] commented 8 months ago

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

melvin-bot[bot] commented 8 months ago

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

JeremyCroff commented 8 months ago

Clicking the IOU triggers a call for https://www.expensify.com/api?command=AuthenticatePusher. This is returning a 404 for the hidden user.

Do you have steps to view the IOU succesfully on a hidden user? This will help determine if viewing IOUs for hidden users having problems is isolated to this user, or across the board.

It might be a service issue. The openReport call succesfully is able to retrieve the data for this user, so logic on back end should be compared.

wildan-m commented 8 months ago

Proposal

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

There are two separate issues:

  1. avatar beside IOU changes to green avatar
  2. IOU with hidden shows hmm not here

What is the root cause of that problem?

For the 1st issue, is caused by no smsdomain on the added phone number For 2nd issue, seems there is a backend error on RequestMoney that will make a unique constraint error occur Need to clarify what keys are used as unique keys for this request. There might backend function that prevents some numbers from being auto-registered

{
  "message": "A record already exists with this ID",
  "status": "400",
  "title": "400 Unique Constraints Violation",
  "request": {
    "command": "RequestMoney",
    "data": {
      "debtorAccountID": 14798521,
      "amount": 22200,
      "currency": "IDR",
      "comment": "",
      "created": "2024-01-21",
      "merchant": "(none)",
      "iouReportID": "714667951058276",
      "chatReportID": "4344097945827472",
      "transactionID": "489382122902296293",
      "reportActionID": "3181795495238515883",
      "createdChatReportActionID": 0,
      "createdIOUReportActionID": "3679830707977126587",
      "reportPreviewReportActionID": "1031906093103224331",
      "taxCode": "",
      "taxAmount": 0,
      "billable": false,
      "appversion": "1.4.28-0",
      "apiRequestType": "write",
      "pusherSocketID": "642042.2643469",
      "shouldRetry": true,
      "canCancel": true
    },
    "successData": [
      {
        "onyxMethod": "merge",
        "key": "report_714667951058276",
        "value": {
          "pendingFields": null,
          "errorFields": null
        }
      },
      {
        "onyxMethod": "merge",
        "key": "transactions_489382122902296293",
        "value": {
          "pendingAction": null,
          "pendingFields": null
        }
      },
      {
        "onyxMethod": "merge",
        "key": "reportActions_4344097945827472",
        "value": {
          "1031906093103224331": {
            "pendingAction": null
          }
        }
      },
      {
        "onyxMethod": "merge",
        "key": "reportActions_714667951058276",
        "value": {
          "3679830707977126587": {
            "pendingAction": null,
            "errors": null
          },
          "3181795495238515883": {
            "pendingAction": null,
            "errors": null
          }
        }
      }
    ],
    "failureData": [
      {
        "onyxMethod": "merge",
        "key": "report_4344097945827472",
        "value": {
          "lastReadTime": "2024-01-21 04:27:36.693",
          "pendingFields": null
        }
      },
      {
        "onyxMethod": "merge",
        "key": "report_714667951058276",
        "value": {
          "pendingFields": null,
          "errorFields": {
            "createChat": {
              "1705811827924000": "report.genericCreateReportFailureMessage"
            }
          }
        }
      },
      {
        "onyxMethod": "merge",
        "key": "transactions_489382122902296293",
        "value": {
          "errors": {
            "1705811827924000": "iou.error.genericCreateFailureMessage"
          },
          "pendingAction": null,
          "pendingFields": null
        }
      },
      {
        "onyxMethod": "set",
        "key": "transactionsDraft_1",
        "value": null
      },
      {
        "onyxMethod": "merge",
        "key": "reportActions_4344097945827472",
        "value": {
          "1031906093103224331": {
            "created": "2024-01-21 04:37:07.923",
            "errors": {
              "1705811827924000": "iou.error.genericCreateFailureMessage"
            }
          }
        }
      },
      {
        "onyxMethod": "merge",
        "key": "reportActions_714667951058276",
        "value": {
          "3679830707977126587": {
            "errors": {
              "1705811827924000": "iou.error.genericCreateFailureMessage"
            }
          },
          "3181795495238515883": {
            "errors": {
              "1705811827924000": null
            }
          }
        }
      }
    ]
  }
}

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

For 1st issue, we can add sms domain to searchValue using addSMSDomainIfPhoneNumber function

Change these codes to:

        // Generates an optimistic account ID for new users not yet saved in Onyx
        const optimisticAccountID = UserUtils.generateAccountID(searchValue);
        const login = addSMSDomainIfPhoneNumber(searchValue)
        const personalDetailsExtended = {
            ...personalDetails,
            [optimisticAccountID]: {
                accountID: optimisticAccountID,
                login: login,
                avatar: UserUtils.getDefaultAvatar(optimisticAccountID),
            },
        };
        userToInvite = createOption([optimisticAccountID], personalDetailsExtended, null, reportActions, {
            showChatPreviewLine,
        });
        userToInvite.isOptimisticAccount = true;
        userToInvite.login = login;
        userToInvite.text = userToInvite.text || login;
        userToInvite.alternateText = userToInvite.alternateText || login;

        // If user doesn't exist, use a default avatar
        userToInvite.icons = [
            {
                source: UserUtils.getAvatar('', optimisticAccountID),
                name: login,
                type: CONST.ICON_TYPE_AVATAR,
            },
        ];

2nd issue needs a backend fix or clarification on the unique keys in the error message

What alternative solutions did you explore? (Optional)

brunovjk commented 8 months ago

If I try variations like +84387025653 or +84387025655 I get the expected failure message: "The provided phone number does not match the country ...". Why am I able to Start Chat with +84387025654 in the first place?

When I add the OP's number, instead of receiving the message "The provided phone number does not match the country ..." as a response, the response is "400 Unique Constraints Violation".

I have a proposal about this error here https://github.com/Expensify/App/issues/33801#issuecomment-1874685653, but I believe that the root of this issue is in the backend, '+84387025653', '+84387025655', '+84387025654', '+84 ...' should have the same response.

bfitzexpensify commented 8 months ago

@situchan your thoughts on the comments above?

bfitzexpensify commented 7 months ago

Bump on this one @situchan - thanks

situchan commented 7 months ago

I agree it's backend issue. It's mysterious that +84387025653 and +84387025654 behave differently from backend. Getting engineer to take a look 🎀 👀 🎀

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

@bfitzexpensify, @roryabraham, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 7 months ago

@bfitzexpensify, @roryabraham, @situchan Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 7 months ago

@bfitzexpensify @roryabraham @situchan 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!

melvin-bot[bot] commented 7 months ago

@bfitzexpensify, @roryabraham, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

bfitzexpensify commented 7 months ago

It sounds like things are behaving as expected for most numbers, but not OPs number.

@lanitochka17 have you been testing with +84387025654 for a while? I'm wondering if it's something specific to that number having been used in the past.

bfitzexpensify commented 7 months ago

Bump on the question in https://github.com/Expensify/App/issues/34839#issuecomment-1924699042 @lanitochka17

bfitzexpensify commented 7 months ago

Reviewing this again - given the problem is limited to a single phone number, and that phone number belongs to a tester and so wouldn't reflect typical usage, I think we can close this out.