Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

All - Approvers - Resolve Duplicates - No system message after merging duplicate expenses #54516

Open
5 of 8 tasks
IuliiaHerets opened this issue Dec 24, 2024 · 2 comments
Open
5 of 8 tasks

All - Approvers - Resolve Duplicates - No system message after merging duplicate expenses #54516

IuliiaHerets opened this issue Dec 24, 2024 · 2 comments
Assignees
@laurenreidexpensify
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Overdue

Comments

@IuliiaHerets
Copy link

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-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5369780
Email or phone of affected tester (no customers): fischer9966 [email protected]
Issue reported by: Applause Internal Team
Device used: MacOS 14.7.1 / Safari, Chrome, Desktop. iPhone 15 pro / 18.2, Pixel 5 / Android 14
App Component: Chat Report View

Action Performed:

Precondition:

  1. Create admin account with Gmail
  2. Create Collect workspace, set Workflows to enabled
  3. Invite Employee and Approver with Gmail accounts to this workspace
  4. Enable Approvals in Workflows and set Approver account as Approver
  5. Enable Workflows > Delay submissions and set it to Manually

Steps:

  1. Login as the Employee and create two duplicate expenses in workspace chat
  2. Click/Tap on any expense
  3. Click/Tap "Review Duplicates" button
  4. Click/Tap "Keep this one" on any expense
  5. Select "Meal & Entertainment" to keep
  6. Select "SF Trip" to keep
  7. Tap "Confirm"

Expected Result:

Verify a system message is added that reads: “resolved the duplicate”

Actual Result:

No system message “resolved the duplicate” added after merging duplicate expenses

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6701675_1735025357613.iOS-resolve-dupes-merge-no-system-message-after-merge.mp4

View all open jobs on GitHub
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 24, 2024
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented Dec 24, 2024

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

@linhvovan29546
Copy link
Contributor

linhvovan29546 commented Dec 26, 2024 •
edited
Loading

Proposal

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

All - Approvers - Resolve Duplicates - No system message after merging duplicate expenses

What is the root cause of that problem?

In this issue, we use the mergeDuplicates function to resolve duplicates. However, we have not implemented a system message for duplicate expenses.

App/src/libs/actions/IOU.ts

Line 8544 in 918488a

function mergeDuplicates(params: TransactionMergeParams) {

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

We can use the ReportUtils.buildOptimisticDismissedViolationReportAction function to build optimisticReportAction as we handled in the resolveDuplicates function.

App/src/libs/actions/IOU.ts

Lines 8799 to 8803 in 918488a

const transactionThreadReportID = params.reportID ? getIOUActionForTransactions([params.transactionID], params.reportID).at(0)?.childReportID : undefined;
const optimisticReportAction = ReportUtils.buildOptimisticDismissedViolationReportAction({
reason: 'manual',
violationName: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
});

Example:

    const transactionThreadReportID = params.reportID ? getIOUActionForTransactions([params.transactionID], params.reportID).at(0)?.childReportID : undefined;
    const optimisticReportAction = ReportUtils.buildOptimisticDismissedViolationReportAction({
        reason: 'manual',
        violationName: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
    });

    const optimisticReportActionData: OnyxUpdate = {
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`,
        value: {
            [optimisticReportAction.reportActionID]: optimisticReportAction,
        },
    };

.....

    parameters = {
            ...params,
            dismissedViolationReportActionID: optimisticReportAction.reportActionID,
    }

Additionally, we need backend support is for a new parameter dismissedViolationReportActionID, similar to how it is handled in the resolveDuplicates function.

POC https://github.com/user-attachments/assets/66d74dc2-6153-4e66-b969-6f4a6dbb03a4

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

We can write IOUTest for IOU.mergeDuplicates

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laurenreidexpensify laurenreidexpensify

Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Overdue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@laurenreidexpensify @linhvovan29546 @IuliiaHerets