Skip to content

[HOLD for payment 2024-10-24] [$250] Expense Details - Square bracket displayed in system message for unapproved expense report #49160

Closed

Description

@IuliiaHerets

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: 9.0.34-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): biruknew45+906@gmail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Log in as an admin in OldDot.
  2. Create a Collect workspace.
  3. Add user B as an employee.
  4. As the employee, log in to NewDot and submit an expense in the workspace chat.
  5. As the employee, go to the expense details page.
  6. As the admin, go to the expense report in OldDot.
  7. Approve the expense and then unapprove it.
  8. As the employee in NewDot, check the system message when the expense is unapproved.

Expected Result:

The system message should correctly specify that the admin has unapproved the report.

Actual Result:

Square brackets ("[]") are displayed in the system message

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6602049_1726217027493!1

Bug6602049_1726217027521.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021838747538236628276
  • Upwork Job ID: 1838747538236628276
  • Last Price Increase: 2024-09-25
  • Automatic offers:
    • DylanDylann | Reviewer | 104127878
    • nkdengineer | Contributor | 104127879
Issue OwnerCurrent Issue Owner: @anmurali

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Sep 13, 2024
  2. melvin-bot commented on Sep 13, 2024

    @melvin-bot

    Triggered auto assignment to @anmurali (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.

  3. IuliiaHerets commented on Sep 13, 2024

    @IuliiaHerets
    Author

    We think that this bug might be related to #wave-collect - Release 1

  4. IuliiaHerets commented on Sep 13, 2024

    @IuliiaHerets
    Author

    @anmurali 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

  5. bernhardoj commented on Sep 14, 2024

    @bernhardoj
    Contributor

    Edited by proposal-police: This proposal was edited at 2024-09-14 07:08:01 UTC.

    Proposal

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

    The system message for unapproved if done from OD shows [].

    What is the root cause of that problem?

    This is a BE problem. When we receive the unapproved message, the object contains [] as the message. After we refresh, it now looks like this:
    image

    For unapproved action, we just show what is inside the action message array. It's different with the approved action where we format it manually and localize it.

    } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.APPROVED) {
    children = <ReportActionItemBasicMessage message={ReportUtils.getIOUApprovedMessage(action)} />;

    We already has the translation, but we never use it.

    unapproved: ({amount, currency}: UnapprovedParams) => `unapproved ${currency}${amount}`,

    The translation was added in #43079 but never being used.

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

    We need to fix this on the FE & BE by using the translations for the unapproved message, so the message is shown correctly and localized.

    1. Add a new check for unapproved action in ReportActionItem
    } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.UNAPPROVED) {
        children = <ReportActionItemBasicMessage message={ReportUtils.getIOUUnapprovedMessage(action)} />;
    
    1. Create the new function for getIOUUnapprovedMessage
    function getIOUUnapprovedMessage(reportAction: ReportAction) {
        return Localize.translateLocal('iou.unapprovedAmount', {amount: getFormattedAmount(reportAction)});
    }
    
    1. Update the translation to move the unapproved within iou object.
    iou: {
        ...,
        unapprovedAmount: ({amount}: UnapprovedParams) => `unapproved ${amount}`,
        ...,
    }
    
    1. Update getFormattedAmount to accept unapproved action.

      App/src/libs/ReportUtils.ts

      Lines 4447 to 4450 in 51a64cf

      function getFormattedAmount(reportAction: ReportAction) {
      if (!ReportActionsUtils.isSubmittedAction(reportAction) && !ReportActionsUtils.isForwardedAction(reportAction) && !ReportActionsUtils.isApprovedAction(reportAction)) {
      return '';
      }
    if (... && !ReportActionsUtils.isApprovedAction(reportAction)) {
    

    The changes on the FE is done, but the message will show unapproved without any amount and currency and that's because the report action original message of the unapproved from OD has an empty originalMessage.
    image

    Compared to the unapproved action done from ND.
    image

    So, we need a BE fix too to return the correct originalMessage.

    Alternatively, we can get the amount and the currency from the IOU/expense report that is available in ReportActionItem.

  6. melvin-bot commented on Sep 16, 2024

    @melvin-bot

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

  7. melvin-bot commented on Sep 18, 2024

    @melvin-bot

    @anmurali Eep! 4 days overdue now. Issues have feelings too...

  8. melvin-bot commented on Sep 20, 2024

    @melvin-bot

    @anmurali Still overdue 6 days?! Let's take care of this!

  9. melvin-bot commented on Sep 24, 2024

    @melvin-bot

    @anmurali 10 days overdue. Is anyone even seeing these? Hello?

  10. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Sep 25, 2024
  11. melvin-bot commented on Sep 25, 2024

    @melvin-bot
  12. changed the title [-]Expense Details - Square bracket displayed in system message for unapproved expense report[/-] [+][$250] Expense Details - Square bracket displayed in system message for unapproved expense report[/+] on Sep 25, 2024
  13. 29 remaining items

  14. melvin-bot commented on Oct 17, 2024

    @melvin-bot

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

  15. melvin-bot commented on Oct 17, 2024

    @melvin-bot

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

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

  16. melvin-bot commented on Oct 17, 2024

    @melvin-bot

    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:

    • [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
    • [@DylanDylann] 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:
    • [@DylanDylann] 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:
    • [@DylanDylann] Determine if we should create a regression test for this bug.
    • [@DylanDylann] 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.
    • [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  17. DylanDylann commented on Oct 17, 2024

    @DylanDylann
    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:

    [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: This change seems like a new improvement, not a bug
    [@DylanDylann] 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
    [@DylanDylann] 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
    [@DylanDylann] Determine if we should create a regression test for this bug. Yes
    [@DylanDylann] 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. Log in as an admin in OldDot.
    2. Create a Collect workspace.
    3. Add user B as an employee.
    4. As the employee, log in to NewDot and submit an expense in the workspace chat.
    5. As the employee, go to the expense details page.
    6. As the admin, go to the expense report in OldDot.
    7. Approve the expense and then unapprove it.
    8. As the employee in NewDot, check the system message when the expense is unapproved.
    9. Verify that: The system message should correctly specify that the admin has unapproved the report.

    Do we agree 👍 or 👎

  18. DylanDylann commented on Oct 28, 2024

    @DylanDylann
    Contributor

    Waiting for @anmurali to complete the payments and close

  19. melvin-bot commented on Oct 28, 2024

    @melvin-bot

    @iwiznia, @anmurali, @DylanDylann, @nkdengineer Huh... This is 4 days overdue. Who can take care of this?

  20. iwiznia commented on Oct 29, 2024

    @iwiznia
    Contributor

    Not overdue, @anmurali needs to pay

  21. anmurali commented on Oct 29, 2024

    @anmurali

    Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions