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.56k stars 2.9k forks source link

[Hold for payment] [$1000] Web - Unable to split bills in group chat if there is a previous existing chat #23470

Closed kbecciv closed 1 year ago

kbecciv commented 1 year 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!


Action Performed:

  1. Open the website
  2. Create a group chat between users with no chat history or unknown users
  3. Click + icon and click split bill
  4. Enter amount
  5. Click next

Expected Result:

Able to split the bill since there is a previous chat between these users

Actual Result:

Unable to split the bill since there is a previous chat between these users

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.43-7 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 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/564977d4-6226-4417-b882-866a94095253

https://github.com/Expensify/App/assets/93399543/8729ab78-b22c-43eb-b54e-a44f9979d2ea

Expensify/Expensify Issue URL: Issue reported by: Web developer | React Js | Vue Js | GH: @misgana96 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687533238591329

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01bf724daf2d526119
  • Upwork Job ID: 1684640842357768192
  • Last Price Increase: 2023-08-10
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

NicMendonca commented 1 year ago

@misgana96 @kbecciv I cannot reproduce this

misgana96 commented 1 year ago

@NicMendonca it is reproducable

https://github.com/Expensify/App/assets/48551032/e57d6b2d-fd6e-4ba2-9160-7b215bbef8e8

misgana96 commented 1 year ago

@NicMendonca after creating bill split wait for a seconds then you will see the issue

NicMendonca commented 1 year ago

I am not seeing that error message:

image
daordonez11 commented 1 year ago

@NicMendonca I have only seen this issue testing with "unknown" accounts, or just people you haven't interacted with previously on NewDot

NicMendonca commented 1 year ago

bingo! @daordonez11 - thank you!

image
NicMendonca commented 1 year ago

asked here:

https://expensify.slack.com/archives/C01SKUP7QR0/p1690415238521519

NicMendonca commented 1 year ago

@kbecciv I received the feedback that there are other bugs with issues requesting money, so this would be a dupe.

NicMendonca commented 1 year ago

Hmmmmm but can't find one that is open 🤔

NicMendonca commented 1 year ago

Okay, maybe we can use this one!

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@NicMendonca, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

NicMendonca commented 1 year ago

waiting for proposals

melvin-bot[bot] commented 1 year ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

NicMendonca commented 1 year ago

@aimane-chnaif thoughts on adjusting price for this one?

aimane-chnaif commented 1 year ago

@NicMendonca yes since no proposals yet

melvin-bot[bot] commented 1 year ago

@NicMendonca @aimane-chnaif 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!

pac-guerreiro commented 1 year ago

I'm Pedro Guerreiro, an expert from Callstack, and I can take a look into this issue for you 😄

daordonez11 commented 1 year ago

So, wanted to help a bit here. This is not an external issue, this is the resposne of the server when it fails:

{
  "code": 666,
  "jsonCode": 666,
  "type": "Expensify\\Libs\\Error\\ExpError",
  "UUID": "61F4D57E-F025-4289-B7F0-483FA8EB820D",
  "message": "There is a previously existing chat between these users.",
  "title": "",
  "data": {
    "onyxData": [
      {
        "onyxMethod": "merge",
        "key": "report_38115464321979",
        "value": {
          "errorFields": {
            "createChat": {
              "1691526491040356": "There is a previously existing chat between these users."
            }
          }
        }
      },
      {
        "onyxMethod": "merge",
        "key": "report_1789805569004711",
        "value": {
          "errorFields": {
            "createChat": {
              "1691526491040345": "There is a previously existing chat between these users."
            }
          }
        }
      },
      {
        "onyxMethod": "merge",
        "key": "report_6637370640272364",
        "value": {
          "errorFields": {
            "createChat": {
              "1691526491040347": "There is a previously existing chat between these users."
            }
          }
        }
      }
    ]
  },
  "htmlMessage": "",
  "onyxData": [
    {
      "onyxMethod": "merge",
      "key": "report_38115464321979",
      "value": {
        "errorFields": {
          "createChat": {
            "1691526491040356": "There is a previously existing chat between these users."
          }
        }
      }
    },
    {
      "onyxMethod": "merge",
      "key": "report_1789805569004711",
      "value": {
        "errorFields": {
          "createChat": {
            "1691526491040345": "There is a previously existing chat between these users."
          }
        }
      }
    },
    {
      "onyxMethod": "merge",
      "key": "report_6637370640272364",
      "value": {
        "errorFields": {
          "createChat": {
            "1691526491040347": "There is a previously existing chat between these users."
          }
        }
      }
    }
  ],
  "requestID": "7f3a81187e4925b9-MIA"
}

The thing is that there is no group/chat between the 3 users I'm using for testing. Hence this should be reviewed internally and field in the backend. Probably a specific logic when a known and an unknown user are included in a split bill.

Also this is the splits object that is being sent:

[{"email":"dloz1996@gmail.com","accountID":14845926,"amount":20},{"email":"dloz1996+8@gmail.com","accountID":14965186,"amount":20,"iouReportID":"5328676399491147","chatReportID":"1789805569004711","transactionID":"8260801238318196639","reportActionID":"1755659752730536714","createdChatReportActionID":"5665463947453820152","createdIOUReportActionID":"8334613277377779916","reportPreviewReportActionID":"5810211507024650633"},{"email":"dloz1996+9@gmail.com","accountID":552437737,"amount":20,"iouReportID":"3799221612866612","chatReportID":"6637370640272364","transactionID":"6717224901453266037","reportActionID":"810002963011284669","createdChatReportActionID":"6078289104957976954","createdIOUReportActionID":"1182832624204545631","reportPreviewReportActionID":"2806474711423898549"}] 

In this case dloz and dloz+8 have chat history and dloz dloz+9 have no chat history.

Even if there is an existing chat I think it should work.

cc @mountiny

pbkompasz commented 1 year ago

Proposal

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

What is the root cause of that problem?

The createSplitsAndOnyxData methods creates the splits and

https://github.com/Expensify/App/blob/316a54520293b636a44b1353c90ccaf461902ed6/src/libs/actions/IOU.js#L769-L785

However the SplitBillAndOpenReport API request returns 400. Screenshot from 2023-08-08 23-29-31 The splits and the reports are already created and the report.genericCreateReportFailureMessage is shown in every individual report. https://github.com/Expensify/App/blob/316a54520293b636a44b1353c90ccaf461902ed6/src/libs/actions/IOU.js#L554 Screenshot from 2023-08-08 23-39-06 Screenshot from 2023-08-08 23-39-15

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

  1. Fix the issue in the BE.
  2. We should handle the case where the API request returns an error better. The previous operations (create individual reports, splits) should be reversed. We should only show the report.genericCreateReportFailureMessage error message in the group chat report.

What alternative solutions did you explore? (Optional)

-

pbkompasz commented 1 year ago

Another bug I discovered was that a new group chat is created even when there is an existing one. When you click on this, it redirects to Concierge and the new group is deleted.

Screencast from 08.08.2023 23:52:43.webm

NicMendonca commented 1 year ago

@mountiny vit - can you double check this? https://github.com/Expensify/App/issues/23470#issuecomment-1670272821

melvin-bot[bot] commented 1 year ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

r-hamid commented 1 year ago

I have been going through the problem and it seems like the problem explained in the video is kinda not correct. I am testing this functionality on MacOS Safari, and I am unable to trigger the action while clicking on Split EUR 100.00.

PS: It is only happening in MacOS Safari, not in MacOS Chrome.

melvin-bot[bot] commented 1 year ago

📣 @r-hamid! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
mountiny commented 1 year ago

I think this should be added to the wave3 project as a clean, this seems like another case of when the unknown and known users are used. @daordonez11 could you confirm if the bug is reproducible if all the members of the split bill are known to each other?

melvin-bot[bot] commented 1 year ago

Current assignee @aimane-chnaif is eligible for the Internal assigner, not assigning anyone new.

mountiny commented 1 year ago

cc @luacmartins in case you have seen this before with the split bill

daordonez11 commented 1 year ago

@mountiny I can not reproduce this issue with known users! Only unknown

mountiny commented 1 year ago

Thanks for confirming

luacmartins commented 1 year ago

I've seen that happening for Request money and unknown users in the past too, not sure if that's still the case though.

luacmartins commented 1 year ago

@puneetlath @Beamanator did we track this as part of secure login cleanups? It seems like Split bill and potentially Request money fails the first time we request money from an unknown user.

JmillsExpensify commented 1 year ago

I think this is more so a wave2 consideration. Removing from wave3.

mvtglobally commented 1 year ago

Issue not reproducible during KI retests. (First week)

melvin-bot[bot] commented 1 year ago

@NicMendonca @mountiny @aimane-chnaif 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!

melvin-bot[bot] commented 1 year ago

@NicMendonca @mountiny @aimane-chnaif 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!

mountiny commented 1 year ago

Waiting for another round of KI retests

mountiny commented 1 year ago

Waiting

NicMendonca commented 1 year ago

Waiting for another round of KI retests

@mountiny sorry if this is dim - when will this retest happen?

mountiny commented 1 year ago

@mvtglobally Have you retested this PR?

NicMendonca commented 1 year ago

bump @mvtglobally

NicMendonca commented 1 year ago

bump bump ^ @mvtglobally

mountiny commented 1 year ago

bumped in slack

mvtglobally commented 1 year ago

No longer reprohttps://github.com/Expensify/App/assets/43995119/857136dd-1c18-4d68-b45c-4f56eb2a6b5e

misgana96 commented 1 year ago

@mvtglobally It is still reproducible when splitting a bill between users with no chat history or unknown users Screencast from 01-10-2023 6:49:14 ከሰዓት.webm

kbecciv commented 1 year ago

@misgana96 Let me check between users with no chat history or unknown users, going to update shortly.