[$250] Expense - In offline, expense created with violation red dot not shown in preview

[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.78-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
Issue reported by: Applause Internal Team
Device used: Redmi note 10s Android 13
App Component: Money Requests

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Go to workspace settings
3. Tap categories - upgrade it
4. Enter payroll & gl code and save it
5. Tap settings and enable members must categorize all expenses
6. Open workspace chat
7. Go offline
8. Create an expense with big amount, enter merchant and set a future date
9. Note expense preview shows no red dot
10. Open the expense and note only missing category field displays red dot
11. Go to workspace chat page
12. Turn online
13. Now note expense preview shows red dot
14. Open the expense and note now red dot is shown in amount, receipt and for date field

Expected Result:

In offline, expense created with violation must show red dot.

Actual Result:

In offline, for expense created with violation, red dot is not shown in expense preview and for amount, receipt and for date field also red dot is not displayed in offline.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6701614_1735010104764.Screenrecorder-2024-12-24-08-18-59-291_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021871678007124043126
• Upwork Job ID: 1871678007124043126
• Last Price Increase: 2024-12-24
• Automatic offers:
• daledah | Contributor | 105464992

Issue Owner

Current Issue Owner: @s77rt

[melvin-bot]

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

[github-actions]

nacasim11 Your proposal will be dismissed because you did not follow the proposal template.

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-12-24 12:07:32 UTC.

Proposal

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

Expense - In offline, expense created with violation red dot not shown in preview

What is the root cause of that problem?

We are not setting showInReview for the violations here

App/src/libs/Violations/ViolationsUtils.ts

Lines 201 to 208 in 126d92a

	// Remove 'missingCategory' violation if category is valid according to policy
	if (hasMissingCategoryViolation && isCategoryInPolicy) {
	newTransactionViolations = reject(newTransactionViolations, {name: 'missingCategory'});
	}
	// Add 'missingCategory' violation if category is required and not set
	if (!hasMissingCategoryViolation && policyRequiresCategories && !categoryKey) {
	newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION});

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

We should set showInReview same as how the BE sets

if (hasMissingCategoryViolation && isCategoryInPolicy) {
                newTransactionViolations = reject(newTransactionViolations, {name: 'missingCategory', showInReview: true});
            }

            // Add 'missingCategory' violation if category is required and not set
            if (!hasMissingCategoryViolation && policyRequiresCategories && !categoryKey) {
                newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
            }

We should apply similar fixes for other violations where needed too like getTagViolationsForSingleLevelTags and getTagViolationsForDependentTags, customUnit

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can make a test for ViolationsUtils.getViolationsOnyxData to assert if showInReview is set properly

What alternative solutions did you explore? (Optional)

[github-actions]

you did not follow the proposal template.

[daledah]

Edited by proposal-police: This proposal was edited at 2024-12-24 11:25:24 UTC.

Proposal

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

In offline, for expense created with violation, red dot is not shown in expense preview and for amount, receipt and for date field also red dot is not displayed in offline.

What is the root cause of that problem?

1. We didn't enable showInReview when building violation optimistic data

App/src/libs/Violations/ViolationsUtils.ts

Line 208 in 8b7096f

newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION});

so the logic to detect violation of transaction is false here

App/src/libs/TransactionUtils/index.ts

Line 886 in 8b7096f

(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION && (showInReview === undefined || showInReview === (violation.showInReview ?? false)),

2. We just have the logic to detect violation for category, not for amount and date

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

1. We should enable showInReview, we can alway put it as true or refer the BE logic
2. For date and amount, we also need to build the optimistic data for it

Here is the sample code for date

// Parse the string into a Date object
const inputDate = new Date(updatedTransaction.created);

// Get the current date
const currentDate = new Date();

// Normalize both dates to ignore the time component
const normalizedInputDate = new Date(inputDate.getFullYear(), inputDate.getMonth(), inputDate.getDate());
const normalizedCurrentDate = new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate());

if (!hasFutureDateViolation && normalizedInputDate > normalizedCurrentDate) {
     newTransactionViolations.push({name: 'futureDate', type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
}

if (hasFutureDateViolation && normalizedInputDate <= normalizedCurrentDate) {
    newTransactionViolations = reject(newTransactionViolations, {name: 'futureDate'});
}

We can use the same logic for amount (overLimit violation) and receiptRequired (This logic above is just demo code, we can improve it by using lib,...)

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can test getViolationsOnyxData function to cover the overLimit, futureDate and others

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

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

[melvin-bot]

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

[s77rt]

@FitseTLT Thanks for the proposal. If showInReview is not set wouldn't that make this condition return true?

App/src/libs/TransactionUtils/index.ts

Line 886 in ad99c20

(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION && (showInReview === undefined || showInReview === (violation.showInReview ?? false)),

[s77rt]

@daledah Thanks for the proposal. I have the same question above ^

[s77rt]

PS: I have also asked a related question here #51893 (comment)

[daledah]

@s77rt We're passing showInReview as true in

App/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

Line 120 in ad99c20

const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '-1', transactionViolations, true);

so it just matches the violation that enable showInReview. Please tell me if any points are unclear. Thanks

[s77rt]

@daledah Makes sense. Thanks for the clarification

[s77rt]

@FitseTLT Your proposal only covered the first problem (seeing red dot in field but not in preview) but didn't cover the second problem (not seeing red dot at all - missing violation).

[s77rt]

@daledah The proposal looks good to me but I don't think we need automated test here. This is something that should be caught by typescript (showInReview).

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[FitseTLT]

@FitseTLT Your proposal only covered the first problem (seeing red dot in field but not in preview) but didn't cover the second problem (not seeing red dot at all - missing violation).

I am sorry @s77rt but it is not clear what you are saying. What is my proposal lacking and the selected proposal has? The reason why the red dot occurs after going online is because the optimistic violation data we are setting lacks showInReview prop that the RCA and I have clearly stated that with the correct solution.
(seeing red dot in field but not in preview) and (not seeing red dot at all - missing violation) don't have different RCAs. Please take a look again Thx
cc @carlosmiceli

[s77rt]

@FitseTLT The other reported fields such as the date don't even have optimistic violation data. Adding showInReview is not enough, in fact there is no place to add it to.

[FitseTLT]

@FitseTLT The other reported fields such as the date don't even have optimistic violation data. Adding showInReview is not enough, in fact there is no place to add it to.

@s77rt How can that be in the scope of this issue there might even be other violations which we are not currently adding optimistically if it should be dealt then it should be covered on another new issue by correctly researching to find all violations we are missing in our optimistic data. I don't think this is at all in the scope of this issue because there are a lot of other violations we are not considering optimsitcally the contributor only chose that came to his mind. For instance here is a demo for overCategoryLimit violation RBR only comes after switching online because we missed to add in optimsitc data. This is a violation that occurs when you set flag amount over for a category rule. There can easily be many more we missed but it should be handled with the proper research on an independent issue. There are several violations but we are only handling few optimistically like tag, category, customUnitOutOfPolicy violations

2024-12-25.20-35-59.mp4

[s77rt]

@FitseTLT Those fields are referenced in the OP. True that we may have many of these missed optimistic data but that does not mean we should ignore them.

[FitseTLT]

@FitseTLT Those fields are referenced in the OP. True that we may have many of these missed optimistic data but that does not mean we should ignore them.

Didn't see that in the OP. Sorry in that case I agree 🙏 @s77rt

[melvin-bot]

📣 @daledah 🎉 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 📖