[Pending Payment][$250] IOU - mWeb - Duplicate red dot error not shown in conversation & report page

[lanitochka17]

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.13
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Login with new gmail account
3. Create a new workspace
4. Tap on a workspace chat
5. Create 2 expenses with same amount and merchant
6. Open each expense and note duplicate red dot shown under merchant

Expected Result:

Duplicate red dot error must be shown in conversation & report page

Actual Result:

Duplicate red dot error not shown in conversation & report page but error displayed in expense details page

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

Bug6554809_1722096890143.review.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0102ecd093c6e7220c
• Upwork Job ID: 1818074477646562605
• Last Price Increase: 2024-08-06
• Automatic offers:
• nyomanjyotisa | Contributor | 103430539

Issue Owner

Current Issue Owner: @

[melvin-bot]

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

[lanitochka17]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[nyomanjyotisa]

Proposal

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

Duplicate red dot error not shown in conversation & report page

What is the root cause of that problem?

We don't set shouldShowRBR to true if there is any duplicate here

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

Line 130 in 08bb00b

const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold);

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

create new variable to check if there is any duplicate

const hasDuplicates = duplicates.length > 0;

And include it in shouldShowRBR

const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasDuplicates;

Need to move this code above the hasDuplicates declaration

What alternative solutions did you explore? (Optional)

The type of duplicate violation is warning so it wont be included to the shouldShowRBR since it only check hasNoticeTypeViolations or hasViolations

create new variable to check if it has violation with warning type

const hasWarningTypeViolation = TransactionUtils.hasWarningTypeViolation(transaction?.transactionID ?? '-1', transactionViolations);

And include it in shouldShowRBR

const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasWarningTypeViolation;

RESULT

-1-New-Expensify.12.mp4

[nyomanjyotisa]

Proposal updated

[Krishna2323]

Proposal

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

IOU - mWeb - Duplicate red dot error not shown in conversation & report page

What is the root cause of that problem?

• Notice & Warning type of violations is only accessible for paid policy group and
  and for users with beta access and are only shown when receipt is not scanning.
• In MoneyRequestPreviewContent.tsx we are only checking for violation and notice types of violation message.
  

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

  Lines 114 to 119 in 08bb00b

  	const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '-1', transactionViolations);
  	const hasNoticeTypeViolations = !!(
  	TransactionUtils.hasNoticeTypeViolation(transaction?.transactionID ?? '-1', transactionViolations) &&
  	ReportUtils.isPaidGroupPolicy(iouReport) &&
  	canUseViolations
  	);

  
  

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

  Line 130 in 08bb00b

  const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold);

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

We need to also check for Warning type of violations when policy is a paid group and user has beta access. For this we have several options:

1. We can remove hasNoticeTypeViolations and TransactionUtils.hasNoticeTypeViolation and modify TransactionUtils.hasViolation util function to accept third param shouldCheckOnlyViolationType which will be used to determine if we want to also consider Notice & Warning type of violations. If needed we can also modify this function to accept separate param for including notice and warning type.
   

   App/src/libs/TransactionUtils.ts

   Lines 693 to 700 in 08bb00b

   	/**
   	* Checks if any violations for the provided transaction are of type 'violation'
   	*/
   	function hasViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolations>): boolean {
   	return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some(
   	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION,
   	);
   	}

/**
 * Checks if any violations for the provided transaction are of type 'violation'
 */
function hasViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolations>, shouldCheckOnlyViolationType: boolean): boolean {
    return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => {
        if (shouldCheckOnlyViolationType) {
            return violation.type === CONST.VIOLATION_TYPES.VIOLATION;
        }
        return true;
    });
} 

2. We can modify TransactionUtils.hasNoticeTypeViolation function to also check for Warning type violations and we can change its name to TransactionUtils.hasOtherTypesViolation.
   

   App/src/libs/TransactionUtils.ts

   Lines 702 to 707 in 08bb00b

   	/**
   	* Checks if any violations for the provided transaction are of type 'notice'
   	*/
   	function hasNoticeTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
   	return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.NOTICE);
   	}

/**
 * Checks if any violations for the provided transaction are of type 'notice'
 */
function hasOtherTypesViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
    return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some(
        (violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.NOTICE || violation.type === CONST.VIOLATION_TYPES.WARNING,
    );
}

3. We can use TransactionUtils.hasWarningTypeViolation in MoneyRequestPreviewContent.tsx just like we are TransactionUtils.hasNoticeTypeViolation
   

   App/src/libs/TransactionUtils.ts

   Lines 709 to 718 in 08bb00b

   	/**
   	* Checks if any violations for the provided transaction are of type 'warning'
   	*/
   	function hasWarningTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
   	if (!Permissions.canUseDupeDetection(allBetas ?? [])) {
   	return false;
   	}
   	return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING);
   	}

    const hasWarningTypeViolations = !!(
        TransactionUtils.hasWarningTypeViolation(transaction?.transactionID ?? '-1', transactionViolations) &&
        ReportUtils.isPaidGroupPolicy(iouReport) &&
        canUseViolations
    );
    const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasWarningTypeViolations;

Also include hasWarningTypeViolations here:

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

Line 223 in 08bb00b

} else if (hasNoticeTypeViolations && transaction && !ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport?.reportID)) {

Some improvements that can be made:

• We can check for similar uses of transaction violations throughout the app and try to refactor it if needed.
• In useViolations.ts we already have similar kind of logic to include all type of violations.
  

  App/src/hooks/useViolations.ts

  Lines 55 to 59 in 08bb00b

  	/**
  	* @param violations – List of transaction violations
  	* @param shouldShowOnlyViolations – Whether we should only show violations of type 'violation'
  	*/
  	function useViolations(violations: TransactionViolation[], shouldShowOnlyViolations: boolean) {

• In MoneyRequestPreviewContent we should only return true for hasWarningTypeViolations and hasWarningTypeViolation when receipt is not being scanned.

What alternative solutions did you explore? (Optional)

[VictoriaExpensify]

Issue reproducible:

screen-20240730-095731.mp4

Agree this is buggy and should be fixed. Also agree this relates to VSB

[melvin-bot]

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

[melvin-bot]

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

[thesahindia]

@nyomanjyotisa, do you know what types of cases come under warning-type violations? We need to determine whether we need the red dot error for all warning-type violations cases or just for the duplicate case.

[nyomanjyotisa]

I'm not sure about all the cases for warning-type violations. I think we should ask in Slack

[Krishna2323]

@cead22 @JmillsExpensify, can you please take a look here? Do we need to show the red dot on preview for all warnings or just for duplicate?

[cead22]

Just for duplicates and for violations with type = violation. So I think the first proposal is more correct. @pecanoro what do you think?

[melvin-bot]

@VictoriaExpensify, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[pecanoro]

Yes, what @cead22 said sounds good!

[thesahindia]

let's go with @nyomanjyotisa's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[pecanoro]

Assigning @nyomanjyotisa to the issue

[melvin-bot]

This issue has not been updated in over 15 days. @stitesExpensify, @VictoriaExpensify, @nyomanjyotisa, @thesahindia eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[pecanoro]

Did automatization fail for this issue? It seems it's ready to be paid

[VictoriaExpensify]

Yeah it has - will get on it. Thanks for flagging @pecanoro

[melvin-bot]

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

[VictoriaExpensify]

Payment summary:
Contributor: @nyomanjyotisa paid $250 via Upwork
Contributor+: @thesahindia owed $250 via NewDot

Sorry for the delayed payment, thanks for your work on this issue.

[garrettmknight]

$250 approved for @thesahindia