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.34k stars 2.77k forks source link

Category rules - System message for updating "Require receipts over" is not clear #49450

Open IuliiaHerets opened 2 days ago

IuliiaHerets commented 2 days 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!


Version Number: 9.0.37-3 Reproducible in staging?: Y Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by:

Action Performed:

Precondition:

  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Categories.
  3. Click any category.
  4. Click "Require receipts over".
  5. Select "Always require receipt".
  6. Go to #admins room.

Expected Result:

The system message for updating "Require receipts over" will be clear.

Actual Result:

The system message for updating "Require receipts over" is not clear. It shows "updated the category "Benefits" by adding a of 0".

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/d778069c-21a4-43e5-bbb9-65b8237d0d9b

View all open jobs on GitHub

melvin-bot[bot] commented 2 days ago

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

IuliiaHerets commented 2 days ago

We think that this bug might be related to #wave-control

IuliiaHerets commented 2 days ago

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

nkdengineer commented 2 days ago

Proposal

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

The system message for updating "Require receipts over" is not clear. It shows "updated the category "Benefits" by adding a of 0".

What is the root cause of that problem?

We don't have a case for this type of message then it displays the message text by default which is the unclear system message

Screenshot 2024-09-19 at 17 16 28

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

We can create a new translation key and a new util to get the message for this system message. The message can be confirmed by the design team. I think the translation key can be built based on categoryName, newValue, updatedField from originalMessage of the action like updated the category "${categoryName}" by adding ${updatedField} with the value ${newValue} (here is an example, the final message can be discussed).

else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_CATEGORIES)) {
    children = <ReportActionItemBasicMessage message={ReportActionsUtils.getPolicyChangeLogUpdateCategoryMessage(action)} />;
}

Then we can add a case for the action here and use the util above the get the message

https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/pages/home/report/ReportActionItem.tsx#L668

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 days ago

📣 @odoyhaha! 📣 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. Make sure you've read and understood the contributing guidelines.
  2. 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.
  3. 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.
  4. 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>
FitseTLT commented 2 days ago

Proposal

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

Category rules - System message for updating "Require receipts over" is not clear

What is the root cause of that problem?

We are not specifically handling the case for POLICYCHANGELOG_UPDATE_CATEGORY report action types in ReportActionItem we are displaying the default message fragments which contains unclear message as indicated in OP

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

We should create a util function that returns the correct message for POLICYCHANGELOG_UPDATE_CATEGORY case and the message content can be decided by the design team. But basically, we can use categoryName, updatedField, newValue, oldValue currency of the originalMessage as a param to a new copy we will create and construct the message similar to sth like updated the category {categoryName} by adding a {updatedField} of {newValue,currency}. Now we need to use this util to get the message for POLICYCHANGELOG_UPDATE_CATEGORY report actions in this places

  1. We need to add a new if case here https://github.com/Expensify/App/blob/3657a67bd5de993f52cf48b7a981b1f1a9d3ea90/src/pages/home/report/ReportActionItem.tsx#L666
  2. For LHN we need to handle the case here https://github.com/Expensify/App/blob/3657a67bd5de993f52cf48b7a981b1f1a9d3ea90/src/libs/SidebarUtils.ts#L431
  3. For copy to clipboard here https://github.com/Expensify/App/blob/3657a67bd5de993f52cf48b7a981b1f1a9d3ea90/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L470 4.For header title of thread created with this type of parentReportAction here https://github.com/Expensify/App/blob/3657a67bd5de993f52cf48b7a981b1f1a9d3ea90/src/libs/ReportUtils.ts#L3766

    What alternative solutions did you explore? (Optional)