Skip to content
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 2024-07-10] [$250] [CRITICAL] [UX Reliability] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode #43602

Closed
1 of 6 tasks
m-natarajan opened this issue Jun 12, 2024 · 36 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed

Comments

@m-natarajan
Copy link

m-natarajan commented Jun 12, 2024

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:
Reproducible in staging?: need reproduction
Reproducible in production?: need reproduction
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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1718138583958989

Action Performed:

  1. Set # focus mode in the preference
  2. Have paid transactions with violations
  3. Observe the LHN

Expected Result:

There should be no expense report items in the LHN without GBR RBR or pinned

Actual Result:

  • There are 3 transactions in the LHN
  • None of them have an RBR
  • They don't disappear even if they are clicked in and out
  • They all have missing translation errors
    NOTE: Included the value of the transaction, report, and violations objects

Workaround:

unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Screenshot 2024-06-11 at 4 42 33 PM (1)

Screenshot 2024-06-11 at 4 42 19 PM (1)

transactionViolations_997722480123927492 from Onyx

[
    {
        "type": "violation",
        "name": "tagOutOfPolicy",
        "data": {
            "tagName": "Departments"
        }
    }
]

transactions_997722480123927492 from Onyx

{
    "amount": 0,
    "billable": false,
    "cardID": 0,
    "category": "Employee Office Meals",
    "comment": {
        "comment": "",
        "dismissedViolations": {
            "rter": {
                "[email protected]": "1705617523"
            }
        }
    },
    "created": "2024-01-15",
    "currency": "USD",
    "filename": "w_268a9217f35887f71c3bee2533cc8cdb8b1727fe.png",
    "merchant": "(none)",
    "modifiedAmount": -1000,
    "modifiedCreated": "",
    "modifiedCurrency": "USD",
    "modifiedMerchant": "Joe Coffee",
    "originalAmount": 0,
    "originalCurrency": "",
    "parentTransactionID": "",
    "receipt": {
        "receiptID": 686919710,
        "state": "SCANCOMPLETE",
        "source": "https://staging.expensify.com/receipts/w_268a9217f35887f71c3bee2533cc8cdb8b1727fe.png"
    },
    "reimbursable": true,
    "reportID": "3961426678972217",
    "status": "Posted",
    "tag": "COR (Eng, Concierge, SmartScan)",
    "transactionID": "997722480123927492",
    "hasEReceipt": false
}

transaction thread report_954645210570433 from Onyx

{
    "reportID": "954645210570433",
    "reportName": "Chat Report",
    "type": "chat",
    "chatType": "",
    "ownerAccountID": 0,
    "managerID": 0,
    "policyID": "0CFE78B25EBE2A0A",
    "participants": {
        "778531": {
            "hidden": false
        },
        "9645353": {
            "hidden": true
        },
        "10903701": {
            "hidden": true
        }
    },
    "isPinned": false,
    "lastReadTime": "2024-01-18 22:39:16.297",
    "lastReadSequenceNumber": 0,
    "lastVisibleActionCreated": "2024-01-18 22:38:43.554",
    "lastVisibleActionLastModified": "2024-01-18 22:38:43.554",
    "lastMessageText": "[email protected]",
    "lastActionType": "MODIFIEDEXPENSE",
    "lastActorAccountID": "778531",
    "notificationPreference": "always",
    "stateNum": 0,
    "statusNum": 0,
    "oldPolicyName": "",
    "isOwnPolicyExpenseChat": false,
    "lastMessageHtml": "[email protected]",
    "hasOutstandingChildRequest": false,
    "hasParentAccess": true,
    "parentReportID": "3961426678972217",
    "parentReportActionID": "352424088454139504",
    "writeCapability": "all",
    "description": "",
    "total": 0,
    "unheldTotal": 0,
    "currency": "USD",
    "chatReportID": "3961426678972217",
    "isWaitingOnBankAccount": false,
    "nonReimbursableTotal": 0,
    "isCancelledIOU": false,
    "permissions": [
        "read",
        "write",
        "share"
    ],
    "welcomeMessage": "",
    "errorFields": {},
    "participantAccountIDs": [
        778531,
        9645353,
        10903701
    ],
    "visibleChatMemberAccountIDs": [
        778531
    ]
}

expense report report_3961426678972217 from Onyx

{
    "reportID": "3961426678972217",
    "reportName": "Expense Report #3961426678972217",
    "type": "expense",
    "chatType": "",
    "ownerAccountID": 778531,
    "managerID": 778531,
    "policyID": "0CFE78B25EBE2A0A",
    "participants": {
        "778531": {
            "hidden": false
        },
        "9645353": {
            "hidden": false
        },
        "9713816": {
            "hidden": false
        },
        "10903701": {
            "hidden": true
        }
    },
    "participantAccountIDs": [
        778531,
        9645353,
        9713816,
        10903701
    ],
    "visibleChatMemberAccountIDs": [
        778531,
        9645353,
        9713816
    ],
    "isPinned": false,
    "lastReadTime": "2024-05-09 19:29:49.195",
    "lastReadSequenceNumber": 0,
    "lastVisibleActionCreated": "2024-05-09 18:31:00.614",
    "lastVisibleActionLastModified": "2024-05-09 18:31:00.614",
    "lastMessageText": "[email protected] exported this report to All Transaction Data Report (E…",
    "lastActionType": "EXPORTINTEGRATION",
    "lastActorAccountID": "9645353",
    "notificationPreference": "always",
    "stateNum": 6,
    "statusNum": 4,
    "oldPolicyName": "",
    "isOwnPolicyExpenseChat": false,
    "lastMessageHtml": "[email protected] exported this report to All Transaction Data Report (Expensify/Zedra/MA/SVFG)",
    "hasOutstandingChildRequest": false,
    "hasParentAccess": true,
    "parentReportID": "2753052891533545",
    "parentReportActionID": "7707689824575329558",
    "writeCapability": "all",
    "description": "",
    "total": -5151,
    "unheldTotal": -5151,
    "currency": "USD",
    "chatReportID": "2753052891533545",
    "isWaitingOnBankAccount": false,
    "nonReimbursableTotal": 0,
    "isCancelledIOU": false,
    "avatarUrl": null,
    "permissions": [],
    "welcomeMessage": "",
    "fieldList": null,
    "invoiceReceiver": null,
    "iouReportID": null,
    "isDeletedParentAction": null,
    "lastMentionedTime": null,
    "policyAvatar": null,
    "policyName": null,
    "tripData": null,
    "visibility": null
}

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01bc533ed41d18d04f
  • Upwork Job ID: 1801673103970237987
  • Last Price Increase: 2024-06-21
  • Automatic offers:
    • ZhenjaHorbach | Reviewer | 102830239
    • truph01 | Contributor | 102830241
Issue OwnerCurrent Issue Owner: @mallenexpensify
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Jun 12, 2024
Copy link

melvin-bot bot commented Jun 12, 2024

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Jun 14, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

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

@puneetlath puneetlath changed the title Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode [CRITICAL] [UX Reliability] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode Jun 14, 2024
@melvin-bot melvin-bot bot changed the title Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode [$250] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode Jun 14, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 14, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

⚠️ This issue has had its price increased by 4x or more. Please review the issue and ensure the price is correct.

Copy link

melvin-bot bot commented Jun 14, 2024

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

Copy link

melvin-bot bot commented Jun 14, 2024

Upwork job price has been updated to $250

@puneetlath puneetlath moved this to CRITICAL in [#whatsnext] #quality Jun 14, 2024
@puneetlath puneetlath changed the title [$250] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode [$250] [CRITICAL] [UX Reliability] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode Jun 14, 2024
@MarcFillari
Copy link

MarcFillari commented Jun 14, 2024

Adding screenshot/data for a similar issue I'm seeing but this one is related to the receipt requirement which has been added to the policy, which wasn't effective at the time this report was originally created/submitted.

Screenshot:
image

Onyx Data:

{
    "reportName": "Chat Report",
    "chatReportID": "5961089989295454",
    "chatType": "",
    "currency": "USD",
    "description": "",
    "hasOutstandingChildRequest": false,
    "hasParentAccess": true,
    "isCancelledIOU": false,
    "isOwnPolicyExpenseChat": false,
    "isPinned": false,
    "isWaitingOnBankAccount": false,
    "lastActionType": "CREATED",
    "lastActorAccountID": "",
    "lastMessageHtml": "",
    "lastMessageText": "",
    "lastReadSequenceNumber": 0,
    "lastReadTime": "2024-06-14 18:31:03.736",
    "lastVisibleActionCreated": "2024-02-15 18:40:47.162",
    "lastVisibleActionLastModified": "2024-02-15 18:40:47.162",
    "managerID": 0,
    "nonReimbursableTotal": 0,
    "notificationPreference": "hidden",
    "oldPolicyName": "",
    "ownerAccountID": 0,
    "parentReportActionID": "7151045519503683113",
    "parentReportID": "5961089989295454",
    "participantAccountIDs": [
        778531,
        9645353,
        10903701,
        14355615
    ],
    "participants": {
        "778531": {
            "hidden": true
        },
        "9645353": {
            "hidden": true
        },
        "10903701": {
            "hidden": true
        },
        "14355615": {
            "hidden": true
        }
    },
    "permissions": [
        "read",
        "write",
        "share"
    ],
    "policyID": "0CFE78B25EBE2A0A",
    "reportID": "7814931881226823",
    "stateNum": 0,
    "statusNum": 0,
    "total": 0,
    "type": "chat",
    "unheldTotal": 0,
    "visibleChatMemberAccountIDs": [],
    "welcomeMessage": "",
    "writeCapability": "all",
    "errorFields": {}
}

@truph01
Copy link
Contributor

truph01 commented Jun 15, 2024

Proposal

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

Transactions in the LHN without RBR don't disappear even if they are clicked in and out
They all have missing translation errors
NOTE: Included the value of the transaction, report, and violations objects

What is the root cause of that problem?

Let's take a look at the condition for displaying RBR in the LHN,

const shouldDisplayViolations = canUseViolations && ReportUtils.shouldDisplayTransactionThreadViolations(fullReport, transactionViolations, parentReportAction);

It uses shouldDisplayTransactionThreadViolations, means if there's violations but the transaction was settled, violations RBR will not be displayed.

Meanwhile, the condition for filtering reports to display in LHN itself is in

ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction as OnyxEntry<ReportAction>)

It uses doesTransactionThreadHaveViolations only, so if a transaction has violations and already settled, it will still be looked at as "display violations", thus will keep the transaction thread in the LHN incorrectly.

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

In

ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction as OnyxEntry<ReportAction>)

shouldDisplayTransactionThreadViolations needs to be used to be consistent with logic to display RBR

ReportUtils.shouldDisplayTransactionThreadViolations(report, transactionViolations, parentReportAction as OnyxEntry<ReportAction>)

What alternative solutions did you explore? (Optional)

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Jun 17, 2024

@truph01
Thanks for the proposal !
But was you able to reproduce this issue?

@ZhenjaHorbach
Copy link
Contributor

Plus in this PR we will refactor the condition for displaying violations for Sidebar
#43502
So maybe we need to add HOLD first

@truph01
Copy link
Contributor

truph01 commented Jun 17, 2024

But was you able to reproduce this issue?

@ZhenjaHorbach Yes I could reproduce it, just by following the steps in issue description.

So maybe we need to add HOLD first

@ZhenjaHorbach I'm not sure we should do this, this is a CRITICAL issue so I think it should get fixed as soon as possible. The PR you linked fix many different parts of the code so might take a while to get merged.

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Jun 17, 2024

But was you able to reproduce this issue?

@ZhenjaHorbach Yes I could reproduce it, just by following the steps in issue description.

So maybe we need to add HOLD first

@ZhenjaHorbach I'm not sure we should do this, this is a CRITICAL issue so I think it should get fixed as soon as possible. The PR you linked fix many different parts of the code so might take a while to get merged.

I tried several times to reproduce this issue
Could you record a video with an example, please?

@truph01
Copy link
Contributor

truph01 commented Jun 17, 2024

Could you record a video with an example, please?

@ZhenjaHorbach Below is the video (with a fresh new account):
https://github.com/Expensify/App/assets/170341821/6a83808f-ce42-4c12-9add-b9facc2ac822

The steps are like OP:

  1. Set # focus mode in the preference
  2. Have paid transactions with violations
    • Create a new workspace
    • Enable "Members must categorize all expenses"
    • Submit a new expense without a category
    • Now that expense will have "Missing category" violation
    • We can see the expense has RBR in LHN
    • Now pay the IOU of that expense
  3. Observe the LHN, we'll see the RBR of the expense disappears, but the expense is still there.

@ZhenjaHorbach
Copy link
Contributor

Could you record a video with an example, please?

@ZhenjaHorbach Below is the video (with a fresh new account): https://github.com/Expensify/App/assets/170341821/6a83808f-ce42-4c12-9add-b9facc2ac822

The steps are like OP:

  1. Set # focus mode in the preference

  2. Have paid transactions with violations

    • Create a new workspace
    • Enable "Members must categorize all expenses"
    • Submit a new expense without a category
    • Now that expense will have "Missing category" violation
    • We can see the expense has RBR in LHN
    • Now pay the IOU of that expense
  3. Observe the LHN, we'll see the RBR of the expense disappears, but the expense is still there.

Thanks a lot !
I'll try to reproduce it within a few hours
And then I'll check the proposition

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Jun 18, 2024

Thank you @truph01
Your proposal makes sense

If we decide to fix this as quickly as possible
I'm happy to go with this proposal

🎀👀🎀 C+ reviewed

Otherwise, we can add HOLD and wait for this PR

Copy link

melvin-bot bot commented Jun 18, 2024

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

Copy link

melvin-bot bot commented Jun 21, 2024

📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Jun 21, 2024

📣 @truph01 🎉 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 📖

@truph01
Copy link
Contributor

truph01 commented Jun 24, 2024

@ZhenjaHorbach PR #44233 is ready

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Jun 24, 2024

Fix: Transaction chat appears in the LHN without GBR/RBR #44233

Cool, thanks
I'll check the PR within an hour !
And I think this PR no longer has to be a draft

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Jun 24, 2024
@neil-marcellini
Copy link
Contributor

Cool, we fixed it appearing in the LHN.

@truph01 can you please also figure out why Puneet's reports were showing the missing translations errors? It looks related to the Departments field.

@truph01
Copy link
Contributor

truph01 commented Jun 28, 2024

I am checking it

@truph01
Copy link
Contributor

truph01 commented Jul 3, 2024

@ZhenjaHorbach Can you reproduce the missing translations errors as mentioned in above:

image

@ZhenjaHorbach
Copy link
Contributor

@ZhenjaHorbach Can you reproduce the missing translations errors as mentioned in above:

image

Unfortunately no 🙁
I've never seen this issue before
And now I can't reproduce this issue either

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 3, 2024
@melvin-bot melvin-bot bot changed the title [$250] [CRITICAL] [UX Reliability] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode [HOLD for payment 2024-07-10] [$250] [CRITICAL] [UX Reliability] Transaction chat appears in the LHN without GBR/RBR or pinned in #focus mode Jul 3, 2024
Copy link

melvin-bot bot commented Jul 3, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 3, 2024
Copy link

melvin-bot bot commented Jul 3, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 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 2024-07-10. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 3, 2024

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:

  • [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:
  • [@ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
  • [@ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@ZhenjaHorbach
Copy link
Contributor

@mallenexpensify
I do not remember exactly
But it should be High Priority label here since this is CRITICAL issue
Sorry if I'm wrong 😅

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 10, 2024
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Jul 11, 2024

Contributor: @truph01 paid $250 via Upwork
Contributor+: @ZhenjaHorbach paid $250 via Upwork.

@ZhenjaHorbach the High Priority needs to be applied in order for the price to be auto-doubled. Details in #expensify-open-source here . Apologies for the confusion.

@ZhenjaHorbach can you fill out the BZ checklist above? Thx

@ZhenjaHorbach
Copy link
Contributor

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:

  • [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:

Unfortunately, I didn’t find the PR that introduced the bug

  • [@ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

NA

  • [@ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

NA

  • [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
  • [@ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

  1. Set # focus mode in the preference
  2. Have paid transactions with violations
    2.1 Create a new workspace
    2.2. Enable "Members must categorize all expenses"
    2.3. Submit a new expense without a category
    2.4. Now that expense will have "Missing category" violation
    2.5 We can see the expense has RBR in LHN
    2.6. Now pay the IOU of that expense
  3. Observe the LHN, the expense disappears.

Do we agree 👍 or 👎

@mallenexpensify
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed
Projects
Development

No branches or pull requests

9 participants