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

[$500] Workspace - In #announceroom, split bill reports display as loading for workspace members. #27154

Closed izarutskaya closed 1 year ago

izarutskaya 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. Login with user A's account.
  2. Open workspace
  3. Invite user B to the workspace.
  4. Open the #announceroom chat.
  5. Spilt bill with workspace members
  6. Login with user B's account.
  7. Open the #announceroom chat in B's workspace.
  8. Notice that the split bill reports are stuck on the loading screen for workspace members.

Expected Result:

after splitting bills, the split bill report should not be displayed as loading for workspace members.

Actual Result:

after splitting the bill, the split bill report is displayed as loading for workspace members.

Workaround:

Unknown

Platforms:

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

Version Number: v1.3.67-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

Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/115492554/95fb7e7c-18e2-4046-8b3d-9f01a36bc3d8

https://github.com/Expensify/App/assets/115492554/d21c0e7c-cd3d-4310-b845-5a2e016b0c4f

Expensify/Expensify Issue URL:

Issue reported by: @Ad-eeb

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693924728887059

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01100fea2e9b78b3b8
  • Upwork Job ID: 1701244048739500032
  • Last Price Increase: 2023-09-25
  • Automatic offers:
    • situchan | Contributor | 26874200
    • Ad-eeb | Reporter | 26874202
melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @adelekennedy (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)

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~01100fea2e9b78b3b8

melvin-bot[bot] commented 1 year ago

Current assignee @adelekennedy 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 - @abdulrahuman5196 (External)

alitoshmatov commented 1 year ago

Backend is not sending transaction data.

Here inside originalMessage there is a IOUTransactionID but this data is not sent from pusher.

Screenshot 2023-09-11 at 20 47 08
adelekennedy commented 1 year ago

@abdulrahuman5196 should this be internal?

adelekennedy commented 1 year ago

@abdulrahuman5196 bump on the above

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? ๐Ÿ’ธ

salonikumawat28 commented 1 year ago

I looked into this issue and it seems like there needs to be changes in Backend. Here is an explanation: When Person1 split bill for 2 persons(Person1 and Person2), we create 2 transaction ids in client side:

  1. Split transaction id representing the split https://github.com/Expensify/App/blob/5ec6105d3b849565c48f1187c78c051fda5e02c6/src/libs/actions/IOU.js#L1013-L1018
  2. Transaction id for person2 - For all persons involved in split except current person, we create the split with all the data including transaction id: https://github.com/Expensify/App/blob/5ec6105d3b849565c48f1187c78c051fda5e02c6/src/libs/actions/IOU.js#L994-L1007

We create onyx optimistic data for both these transactions. But as response of API.write request, we get the transaction of Person2 only in return, we don't get the transaction as mentioned in Point 1 above.

As we don't get the transaction mentioned in Point 1 from backend, we see the issues. Here is snippet of example request payload and response which shows this problem:

REQUEST:

reportID: 2675033063704124
amount: 130000
splits: [{"email":"545saloni+3@gmail.com","accountID":15694778,"amount":65000},{"email":"545saloni+4@gmail.com","accountID":15705003,"amount":65000,"iouReportID":"6189850050239895","chatReportID":"7536945066043077","transactionID":"1898044165078485633","reportActionID":"4880171455368989281","createdChatReportActionID":"7280778505637454476","createdIOUReportActionID":"3788054532130349476","reportPreviewReportActionID":"6883759768728262211"}]
currency: GBP
transactionID: 2381798152516133750
reportActionID: 3602112367821823958
policyID: 57A98B2EBCCAEEAD
apiRequestType: write
email: 545saloni+3@gmail.com

and:

RESPONSE:

{
    "jsonCode": 200,
    "requestID": "809bd0a7fc3e886e-LHR",
    "onyxData": [
        {
            "onyxMethod": "merge",
            "key": "transactions_1898044165078485633",
            "value": {
                "amount": 65000,
                "category": "",
                "comment": {
                    "comment": "",
                    "originalTransactionID": "2381798152516133750",
                    "source": "split"
                },
                "created": "2023-09-20 17:33:43",
                "currency": "GBP",
                "filename": "",
                "merchant": "Request",
                "modifiedAmount": 0,
                "modifiedCreated": "",
                "modifiedCurrency": "",
                "modifiedMerchant": "",
                "receipt": [],
                "reportID": "6189850050239895",
                "tag": "",
                "transactionID": "1898044165078485633"
            }
        }
    ]
}

Because of this issue, other clients never gets the split transaction of Point 1. Even the same person1 when logs out and logs in, they will no longer see split transaction. As announce rooms shows the overall split transaction i.e. transaction of Point 1, we see issue in announce room but we will see this issue everywhere we use this split transaction.

melvin-bot[bot] commented 1 year ago

@abdulrahuman5196, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @dangrous (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

adelekennedy commented 1 year ago

added the engineering label by mistake - at ease @dangrous

adelekennedy commented 1 year ago

bumped in Slack

melvin-bot[bot] commented 1 year ago

Current assignee @adelekennedy is eligible for the Bug assigner, not assigning anyone new.

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

melvin-bot[bot] commented 1 year ago

@adelekennedy 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 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? ๐Ÿ’ธ

situchan commented 1 year ago

It seems no C+ assigned here. I can help this move forward.

melvin-bot[bot] commented 1 year ago

๐Ÿ“ฃ @situchan ๐ŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role ๐ŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review ๐Ÿง‘โ€๐Ÿ’ป Keep in mind: Code of Conduct | Contributing ๐Ÿ“–

melvin-bot[bot] commented 1 year ago

๐Ÿ“ฃ @Ad-eeb ๐ŸŽ‰ An offer has been automatically sent to your Upwork account for the Reporter role ๐ŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

adelekennedy commented 1 year ago

just assigned you @situchan

adelekennedy commented 1 year ago

@situchan I think we only have the one proposal above to review

situchan commented 1 year ago

I don't see any formal proposal yet. @salonikumawat28 can you please follow proposal template so you should re-state title, explain the correct root cause and suggest solution. I don't think this happens only on #announce room but all group chats where split bill is applicable.

salonikumawat28 commented 1 year ago

@situchan I didn't give formal proposal because I think the changes needs to be in backend. I explained the reasoning behind it in my comment above. I have updated my comment above to make it more clear.

Either the changes should be in backend and if so, the issue should be internal. Otherwise, its possible that we are not creating the API request as backend expects. For the latter case, can you point me where to find the API schema of SplitBill so that I can make sure we are making correct API request.

adelekennedy commented 1 year ago

update above

melvin-bot[bot] commented 1 year ago

@adelekennedy @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@adelekennedy @situchan this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Thanks!

melvin-bot[bot] commented 1 year ago

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

melvin-bot[bot] commented 1 year ago

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

situchan commented 1 year ago

@salonikumawat28 are you still able to reproduce this?

melvin-bot[bot] commented 1 year ago

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

situchan commented 1 year ago

@salonikumawat28 are you still able to reproduce this?

bump ^

salonikumawat28 commented 1 year ago

@situchan Sorry for late response. I can still reproduce this. I tried it on staging.new.expensify.com

adelekennedy commented 1 year ago

Still is reproducible, but @situchan re-reading this how big of a problem is this? There's been so little movement here I think we should close and pay out some percentage for reviewing proposals

situchan commented 1 year ago

I think this bug cannot be be simply ignored. @youssef-lr should backend send transaction data in pusher?

adelekennedy commented 1 year ago

@youssef-lr bump on the above!

melvin-bot[bot] commented 1 year ago

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

youssef-lr commented 1 year ago

@adelekennedy sorry I missed your ping.

I think this bug cannot be be simply ignored. @youssef-lr should backend send transaction data in pusher?

Correct @situchan, this is a duplicate of https://github.com/Expensify/App/issues/27200 which I have created a backend PR for. So I'll be closing this one.