App icon indicating copy to clipboard operation
App copied to clipboard

[$500] Workspace – “Submitted $amount” system message appears for a moment when Employee submit IOU

Open lanitochka17 opened this issue 1 year ago • 20 comments

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.54-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: https://expensify.testrail.io/index.php?/tests/view/4434476 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log in as Employee of a Collect WS
  3. Open WS chat and request a money
  4. Open IOU and press Submit button in the header

Expected Result:

“You submitted this report to Admin (email)” system message appears when Employee submit IOU

Actual Result:

“Submitted $amount” system message appears for a moment when Employee submit IOU and then appears “You submitted this report to Admin (email)”

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/78819774/27cf0e98-eb17-44e5-94e6-fe238e107bd7

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01304a705cb67bed22
  • Upwork Job ID: 1770177446924410880
  • Last Price Increase: 2024-03-19
  • Automatic offers:
    • situchan | Reviewer | 0
    • FitseTLT | Contributor | 0

lanitochka17 avatar Mar 19 '24 12:03 lanitochka17

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

melvin-bot[bot] avatar Mar 19 '24 12:03 melvin-bot[bot]

@garrettmknight FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 avatar Mar 19 '24 12:03 lanitochka17

Confirmed the bug - I think it fits in #wave-collect.

garrettmknight avatar Mar 19 '24 19:03 garrettmknight

Job added to Upwork: https://www.upwork.com/jobs/~01304a705cb67bed22

melvin-bot[bot] avatar Mar 19 '24 19:03 melvin-bot[bot]

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

melvin-bot[bot] avatar Mar 19 '24 19:03 melvin-bot[bot]

Proposal

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

Workspace – “Submitted $amount” system message appears for a moment when Employee submit IOU

What is the root cause of that problem?

The optimistic submited report action contains message with only one fragment of submitted amount https://github.com/Expensify/App/blob/0d700c31dfadce758d4a623789e6bd1857573131/src/libs/actions/IOU.ts#L3893-L3894 https://github.com/Expensify/App/blob/0d700c31dfadce758d4a623789e6bd1857573131/src/libs/ReportUtils.ts#L3175-L3176

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

The optimistic submitted report action should have all the message fragments same as what the BE returns like

[
  {
    "type": "TEXT",
    "style": "strong",
    "text": "You"
  },
  {
    "type": "TEXT",
    "style": "normal",
    "text": " submitted this report"
  },
  {
    "type": "TEXT",
    "style": "normal",
    "text": " to "
  },
  {
    "type": "TEXT",
    "style": "strong",
    "text": "F A T ([email protected])"
  }
]

Submitted to account can be taken from policy.ownerAccountID so We can update buildOptimisticSubmittedReportAction to

  function buildOptimisticSubmittedReportAction(amount: number, currency: string, expenseReportID: string, policy): OptimisticSubmittedReportAction {
    const originalMessage = {
        amount,
        currency,
        expenseReportID,
    };
    const ownerPersonalDetails = getPersonalDetailsForAccountID(policy.ownerAccountID);

    return {
        actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED,
        actorAccountID: currentUserAccountID,
        automatic: false,
        avatar: getCurrentUserAvatarOrDefault(),
        isAttachment: false,
        originalMessage,
        message: [
            {
                type: CONST.REPORT.MESSAGE.TYPE.TEXT,
                style: 'strong',
                text: 'You',
            },
            {
                type: CONST.REPORT.MESSAGE.TYPE.TEXT,
                style: 'normal',
                text: ' submitted this report',
            },
            {
                type: CONST.REPORT.MESSAGE.TYPE.TEXT,
                style: 'normal',
                text: ' to ',
            },
            {
                type: CONST.REPORT.MESSAGE.TYPE.TEXT,
                style: 'strong',
                text: `${ownerPersonalDetails.displayName} (${ownerPersonalDetails.login})`,
            },
        ],

We can move this change to getIOUReportActionMessage (only updating SUBMITTED case) And moreover We can think of more complex logic especially to determine the first and last fragments, especially the last because the way display name and login are displayed depend on their existence.

What alternative solutions did you explore? (Optional)

FitseTLT avatar Mar 19 '24 20:03 FitseTLT

Proposal

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

“Submitted $amount” system message appears for a moment when Employee submit IOU

What is the root cause of that problem?

Currently we display submitted and the amount using getIOUReportActionMessage :

https://github.com/Expensify/App/blob/ca0cf248d1a2c57b2a7623703df8d256ea02b52e/src/libs/ReportUtils.ts#L3174-L3176

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

Remove the following condition, as we get the data as system message

https://github.com/Expensify/App/blob/ca0cf248d1a2c57b2a7623703df8d256ea02b52e/src/libs/ReportUtils.ts#L3174-L3176

What alternative solutions did you explore? (Optional)

N/A

allgandalf avatar Mar 19 '24 20:03 allgandalf

Proposal

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

The issue involves a momentary display of the "Submitted $amount" system message when an Employee submits an IOU in the Expensify workspace. This message is unexpected and causes confusion among users.

What is the root cause of that problem?

The root cause of this problem appears to be a timing issue in the system message display mechanism. It seems that the "Submitted $amount" message is briefly shown before being replaced by the correct "You submitted this report to Admin (email)" message.

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

To solve this issue, we need to adjust the timing or sequence of events related to the display of system messages when an IOU is submitted. This may involve modifying the backend logic or frontend rendering to ensure that the correct message is displayed immediately without any intermediate messages.

What alternative solutions did you explore? (Optional)

An alternative solution could involve implementing a more robust message handling mechanism that prevents the display of any interim messages during the submission process. Additionally, we could explore optimizing the system response time to minimize the delay in displaying the correct message.

By addressing the timing issue and ensuring consistent messaging behavior, we can enhance the user experience and eliminate confusion during IOU submission in the Expensify workspace.

Please let me know if you need further clarification or assistance with implementing this solution.

Best regards, Mudassir Ali.

iammudassirali avatar Mar 20 '24 04:03 iammudassirali

@situchan can you take a look at these proposals when you get a chance?

garrettmknight avatar Mar 21 '24 20:03 garrettmknight

reviewing

situchan avatar Mar 21 '24 20:03 situchan

@FitseTLT's proposal looks good to me. We need to make sure that optimistic message and server message are exactly the same, not only text but also style. 🎀 👀 🎀 C+ reviewed

Screenshot 2024-03-22 at 1 34 26 PM Screenshot 2024-03-22 at 1 34 31 PM

situchan avatar Mar 22 '24 07:03 situchan

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

melvin-bot[bot] avatar Mar 22 '24 07:03 melvin-bot[bot]

Sounds good, assigning @FitseTLT to the issue

pecanoro avatar Mar 22 '24 15:03 pecanoro

📣 @situchan 🎉 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

melvin-bot[bot] avatar Mar 22 '24 15:03 melvin-bot[bot]

📣 @FitseTLT 🎉 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] avatar Mar 22 '24 15:03 melvin-bot[bot]

PR will be up in 3 days

FitseTLT avatar Mar 25 '24 14:03 FitseTLT

@FitseTLT Any updates?

pecanoro avatar Mar 27 '24 23:03 pecanoro

@FitseTLT friendly bump

pecanoro avatar Mar 28 '24 21:03 pecanoro

will create it EOD

FitseTLT avatar Mar 29 '24 07:03 FitseTLT

I can take over this issue as a C+ reviewer because @situchan is OOO in April

DylanDylann avatar Mar 30 '24 05:03 DylanDylann

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

melvin-bot[bot] avatar Apr 01 '24 16:04 melvin-bot[bot]

@situchan is out OOO, reassigning a C+ via the auto-assigner for fairness.

mallenexpensify avatar Apr 01 '24 16:04 mallenexpensify

https://github.com/Expensify/App/pull/39321 is in review. I'll complete the reviewer checklist tomorrow morning.

hungvu193 avatar Apr 02 '24 17:04 hungvu193

@garrettmknight, @pecanoro, @hungvu193, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Apr 09 '24 18:04 melvin-bot[bot]

PR is in review

hungvu193 avatar Apr 09 '24 23:04 hungvu193

@garrettmknight, @pecanoro, @hungvu193, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Apr 17 '24 18:04 melvin-bot[bot]

Still working through the PR

garrettmknight avatar Apr 18 '24 12:04 garrettmknight

@garrettmknight, @pecanoro, @hungvu193, @FitseTLT Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Apr 25 '24 18:04 melvin-bot[bot]

Hmm,, it seems the automation didn't get triggered to update the title and payment

pecanoro avatar Apr 26 '24 13:04 pecanoro

@hungvu193 had to send a new offer: https://www.upwork.com/nx/wm/offer/102083093

garrettmknight avatar Apr 29 '24 17:04 garrettmknight