[HOLD for payment 2024-11-20] [$250] Category rules - System message for updating "Require receipts over" is not clear

[IuliiaHerets]

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: 9.0.37-3
Reproducible in staging?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by:

Action Performed:

Precondition:

• Rules are enabled.

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:

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

Screenshots/Videos

Bug6607635_1726664589291.20240918_205739.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021838586050489749187
• Upwork Job ID: 1838586050489749187
• Last Price Increase: 2024-09-24
• Automatic offers:
• hoangzinh | Reviewer | 104165839
• FitseTLT | Contributor | 104165842

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

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]

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

[IuliiaHerets]

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

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

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

App/src/pages/home/report/ReportActionItem.tsx

Line 668 in 37bf6b2

} else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.REMOVED_FROM_APPROVAL_CHAIN)) {

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @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.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[FitseTLT]

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
   

   App/src/pages/home/report/ReportActionItem.tsx

   Line 666 in 3657a67

   } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.DELETE_EMPLOYEE) {

2. For LHN we need to handle the case here
   

   App/src/libs/SidebarUtils.ts

   Line 431 in 3657a67

   } else if (ReportActionsUtils.isActionOfType(lastAction, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME)) {

3. For copy to clipboard here
   

   App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

   Line 470 in 3657a67

   } else if (reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE) {

   
   4.For header title of thread created with this type of parentReportAction here
   

   App/src/libs/ReportUtils.ts

   Line 3766 in 3657a67

   if (!isEmptyObject(parentReportAction) && ReportActionsUtils.isModifiedExpenseAction(parentReportAction)) {

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[Maab18th]

Hi,
The problem arises when a user updates the "Require receipts over" setting in the categories section. The system message generated is unclear, specifically stating "updated the category 'Benefits' by adding a 0," which doesn't provide accurate feedback on the changes made. To tackle the issue I will follow the following approach

Root Cause Analysis:
The unclear message likely stems from how the update process is handling and displaying the value of the "Require receipts over" field. It seems that the system isn't correctly interpreting or formatting the value entered by the user, which leads to the confusing output.

Proposed Fix:
Modify the message handling logic in the backend so that it accurately reflects the chnges made in the "Require receipts over" field.
Ensure that the value entered by the user is properly captured and integrated into the feedback message. For example, if a user selects "Always require receipt," the message should clearly indicate: "Category X updated to always require a receipt."
Add validation for the value field to ensure non-zero, valid inputs are processed correctly.

Testing:
I will perform tests to verify the updated message on all affected platforms (web and mobile) to ensure the message is now clear and reflects the actual change made in the settings.

By implementing this solution, the system will deliver more meaningful feedback to users, ensuring a better user experience when configuring category rules.

[melvin-bot]

📣 @Maab18th! 📣
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.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[muttmuure]

@hoangzinh a few proposals for you

[hoangzinh]

Thanks for proposals, everyone. Both proposals have the same approach. @nkdengineer is first, but @FitseTLT is clearer and has more details on the solution. But in my opinion, those details are not a critical point in selecting a proposal, they can be worked out in PR. Therefore, I think we can go with @nkdengineer's proposal

Link to proposal #49450 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[FitseTLT]

Thanks for proposals, everyone. Both proposals have the same approach. @nkdengineer is first, but @FitseTLT is clearer and has more details on the solution. But in my opinion, those details are not a critical point in selecting a proposal, they can be worked out in PR. Therefore, I think we can go with @nkdengineer's proposal

Link to proposal #49450 (comment)

🎀👀🎀 C+ reviewed

@hoangzinh I really disagree with you that my proposal is only more detailed. The first proposal will cause regressions and points I have added are not simple points that are guarented to be dealt with in the PR phase because I have seen and dealt with bugs (here are few, #37292 ,#34989) in the past dealing with discrepancies with report actions content displayed and content to copy to clipboard or LHN or title of the chat thread opened with the report action which is caused by other contributors missing it same as the first proposal. So my proposal is the only correct and appropriate proposal here and it is very discouraging for the future to ignore these types of second proposals that have important differences.

@stitesExpensify please take a 👀 to my proposal

[hoangzinh]

@FitseTLT Oh, I haven't been aware that we had a few issues related to those points. Seems a valid point. Let's see what @stitesExpensify thoughts on it.

[stitesExpensify]

I am very torn on this one. I think that because there have been other bugs for very similar PRs, the fact that @FitseTLT mentioned those other cases and @nkdengineer did not, @FitseTLT should be chosen.

As an aside, this seems like a very bad pattern to be duplicating these issues in multiple places. We should probably consolidate them somehow

[stitesExpensify]

Thank you very much for your proposal @nkdengineer. If there hadn't already been regressions for missing those specific details, you would have been chosen.

[melvin-bot]

📣 @hoangzinh 🎉 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]

Current assignee @hoangzinh is eligible for the External assigner, not assigning anyone new.

[FitseTLT]

the policyCategory. maxAmountNoReceipt is fixed and properly working but the policy. maxExpenseAmountNoReceipt is not saving changes in the database so when you change the value the API returns the updated value but after you clear cache the value get back to the default value. It is not directly related to this issue but want to report it if you guys want to handle the BE changes.

Let's report this as a separate issue. As you said, it's not related.

BTW guys regarding the unrelated issue here don't forget to report it as I cannot create issues. And to remind you it is a BE bug 👍

[marcaaron]

maxExpenseAmountNoReceipt is not saving changes in the database

Looked into this and it's not the case that it won't save. I believe it's due to the backend sending an Onyx.set() with the policy that has some of the necessary fields missing so we end up defaulting. Nice catch though. I'll create a new issue for it.

[melvin-bot]

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

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.60-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix - change maxExpenseAmountNoReceipt to maxAmountNoReceipt for policy… #52058

If no regressions arise, payment will be issued on 2024-11-20. 🎊

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

• @hoangzinh requires payment automatic offer (Reviewer)
• @FitseTLT requires payment automatic offer (Contributor)

[melvin-bot]

@hoangzinh @muttmuure @hoangzinh The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[marcaaron]

I think this is ready for payment now?

[melvin-bot]

@hoangzinh, @marcaaron, @FitseTLT, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[hoangzinh]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: It's a BE bug [HOLD for payment 2024-11-20] [$250] Category rules - System message for updating "Require receipts over" is not clear #49450 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

I guess it's covered in BE, but if it's not here is my regression test proposal

Regression Test Proposal

Precondition:

Precondition:

• Enable Rules on a workspace

Test:

1. Sign in to Expensify
2. Go to Settings -> Workspaces -> select workspace above
3. Go to Categories.
4. Click any category.
5. Click "Require receipts over".
6. Select "Always require receipt".
7. Go to #admins room.
8. Verify there is a system message about "updated the category {category_name} by changing Receipts from Default to Required"

Do we agree 👍 or 👎

[muttmuure]

I think we already cover these kinds of messages (Applause reported this after all)

[muttmuure]

Everybody is paid

[Beamanator]

Just noting here that [BugBot] - ENSURE_BUGBOT We updated a category field that has no message logic. Please implement or fix this problem for: 'maxExpenseAmountNoReceipt' was just reopened 🤔 Which I thought we assumed this Issue was going to fix?

[marcaaron]

Oh, yeah, that user probably has some messed up category data now 😬 let's talk on the Server issue about how to fix it or see how many people were affected. I see it is happening very infrequently so maybe it's only 1 or 2 users with this problem?