[Violations] [$1000] Violations are not shown as approver

[m-natarajan]

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: 1.4.41-2
Reproducible in staging?: y
Reproducible in production?: y
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/p1707848268463619

Action Performed:

1. Go to OldDot staging.expensify.com and open or create a Collect policy
2. Make tags required for the policy

• Go to Settings > [Workspace] > Tags
• Enable "People must tag expenses"
• Add a tag

3. Set violations for category as to be required
4. Go to the policy page
5. In the JS console with the policy open do:

p = Policy.getCurrent();
p.policy.isPolicyExpenseChatEnabled = "true";
p.save();

6. Invite a user to the policy as employee
7. As an employee sign in to the NewDot, create a expense report without category and tag set
8. Open the report as approver logged in to NewDot

Expected Result:

Error message for violations are shown

Actual Result:

Not seeing violations as approver

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

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ebca0f5b1a041068
• Upwork Job ID: 1757526242287419392
• Last Price Increase: 2024-02-22

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[brandonhenry]

Proposal

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

Approvers are not seeing the expected error messages for violation of expense report policies, specifically when category and tag settings are required but not set by the employee on the report.

What is the root cause of that problem?

The root cause may involve the policy enforcement logic within the approver's view. It appears the validation checks that trigger error messages for missing categories or tags are not executing correctly, or the UI is not displaying the results of these checks as it should.

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

• Review the expense report validation flow to ensure that the checks for required fields are triggered when an approver reviews a report.
• Update the UI components responsible for displaying the error messages to approvers to ensure they are activated correctly when violations are detected.
• Ensure that the state management for the report correctly reflects the presence of violations and binds this state to the UI elements for error message display.
• Conduct tests on all supported platforms to ensure that upon opening a report with set violations, the approver is presented with the appropriate error messages.

What alternative solutions did you explore? (Optional)

• An alternative solution could be to implement a pre-approval validation step, prompting the employee to correct the violations before submission to the approver. However, this may not be desirable as it shifts the responsibility from the approver to the employee.

[bfitzexpensify]

@cead22 is this something you're planning on working on yourself, or is this something that contributors can tackle?

[cead22]

Contributors can tackle it, I just assigned myself so I can be the reviewer of the PR since I've been leading violations and am familiar with the coe

[bfitzexpensify]

Sweet - OK, awaiting proposals.

[parasharrajat]

Thanks @brandonhenry for submitting a proposal. Although your proposal outlines the steps, it is missing technical details. Could you please update it? Please check old issues for example proposals.

[brandonhenry]

Was going to update my proposal here, but I am having a really hard time reproducing this issue. I'm not able to see this details request screen in ND. I'm also not able to access any workspace that supports tags inside of ND. There's no way to view a workspace other than free workspace in ND in prod right now. I don't think this is an urgent issue but maybe thats just me

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[parasharrajat]

@m-natarajan Could you please update the steps so that it explains @brandonhenry queries #36441 (comment) cc: @bfitzexpensify

[bfitzexpensify]

Posted for help in the #qa channel

[kavimuru]

I have edited the OP. Please check if this helps.

[bfitzexpensify]

Thanks @kavimuru!

[bfitzexpensify]

No full proposals after a week, let's increase the bounty

[melvin-bot]

Upwork job price has been updated to $1000

[brandonhenry]

Updated Proposal

Problem Statement

Approvers are not seeing the expected error messages for violation of expense report policies, specifically when category and tag settings are required but not set by the employee on the report.

Root Cause

The root cause of the problem is that the getErrorForField function, which is responsible for determining and displaying error messages for field violations, does not include checks for category and tag violations. As a result, even when these fields are required by the policy and left unset by the employee, no error messages are displayed to the approver.

App/src/components/ReportActionItem/MoneyRequestView.tsx

Line 204 in d33bf6e

const getErrorForField = useCallback(

Proposed Changes

1. Update getErrorForField Function: Modify getErrorForField function to include checks for category and tag violations. This will ensure that error messages are displayed for these fields when they are required by the policy but not set by the employee.

2. Ensure Correct Dependency Handling: Update the dependency array of the useCallback hook for the getErrorForField function to include transactionCategory and transactionTag. This will ensure that the function updates correctly when these values change.

const getErrorForField = useCallback(
    (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => {
        const fieldChecks: Partial<Record<ViolationField, {isError: boolean; translationPath: TranslationPaths}>> = {
            amount: {
                isError: transactionAmount === 0,
                translationPath: 'common.error.enterAmount',
            },
            merchant: {
                isError: !isSettled && !isCancelled && isPolicyExpenseChat && isEmptyMerchant,
                translationPath: 'common.error.enterMerchant',
            },
            date: {
                isError: transactionDate === '',
                translationPath: 'common.error.enterDate',
            },
            category: {
                isError: !transactionCategory,
                translationPath: 'common.error.selectCategory',
            },
            tag: {
                isError: !transactionTag,
                translationPath: 'common.error.selectTag',
            },
        };

        // .. code is the same
    },
    [transactionAmount, isSettled, isCancelled, isPolicyExpenseChat, isEmptyMerchant, transactionDate, transactionCategory, transactionTag, hasErrors, canUseViolations, hasViolations, translate, getViolationsForField],
);

[cead22]

Is this still reproducible?

[bfitzexpensify]

Asked QA to retest in Slack

[melvin-bot]

@cead22 @parasharrajat @bfitzexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@cead22 @parasharrajat @bfitzexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[bfitzexpensify]

QA are still able to reproduce @cead22

Recording.1758.mp4

[cead22]

Thanks, I think I found the bug

[cead22]

There are two things happening here:

1. We're not returning transaction violations for anyone other than the user that made the request in OpenReport
2. We don't push transaction violations to all the relevant users from the RequestMoney API command

I'll fix 1. for this issue, and 2. as part of #37013

[cead22]

PR submitted for item number 1. above

[melvin-bot]

@cead22 @parasharrajat @bfitzexpensify this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

[melvin-bot]

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

[cead22]

PR submitted for item number 2. above

[melvin-bot]

@cead22, @parasharrajat, @bfitzexpensify Eep! 4 days overdue now. Issues have feelings too...

[JmillsExpensify]

Nice! With both of those merged and on production, can we close this issue now?

[cead22]

Yup!