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.

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

[PAID] Implement and use ReportUtils.transactionThreadHasViolations(report) #31095

Closed
cead22 opened this issue Nov 9, 2023 · 31 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering NewFeature Something to build that is a new item.

Comments

@cead22
Copy link
Contributor

cead22 commented Nov 9, 2023

  • Design Doc
  • Implementation should look like code snippet below
    • Which probably means we should connect ReportUtils to the BETAS onyx collection
    • Let's also connect it to the TRANSACTION_VIOLATIONS and REPORT_ACTIONS Onyx collections as shown below
    • And let's also add as shown below ReportUtils.reportHasViolations
  • Then let's use ReportUtils.transactionThreadHasViolations
    • In SidebarUtils.ts Let's update this
      • From: result.brickRoadIndicator = Object.keys(result.allReportErrors ?? {}).length !== 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
      • To: result.brickRoadIndicator = Object.keys(result.allReportErrors ?? {}).length !== 0 || ReportUtils.transactionThreadHasViolations(report) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
      • That way src/components/LHNOptionsList/OptionRowLHN.js should show the RBR
    • In ReportUtils.shouldReportBeInOptionList above if (isInGSDMode) { we’ll add the if in the snippet below to surface the iou reports with violations on the LHN
  • And let's use ReportUtils.reportHasViolations
    • In src/components/ReportActionItem/ReportPreview.js let's update the code so the hasErrors value takes into account the report violations const hasErrors = (hasReceipts && ReportUtils.hasMissingSmartscanFields(props.iouReportID)) || ReportUtils.reportHasViolations(props.iouReportID);
const transactionViolations = {};
Onyx.connect({
    key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
    callback: (violations, key) => {
        if (!key || !violations) {
            return;
        }

        const transactionID = CollectionUtils.extractCollectionItemID(key);
        transactionViolations[transactionID] = violations;
    },
});

const reportActions = {};
Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
    callback: (actions, key) => {
        if (!key || !actions) {
            return;
        }

        const reportID = CollectionUtils.extractCollectionItemID(key);
        reportActions[reportID] = actions;
    },
});
function transactionThreadHasViolations(report) {
    if (!permissions.canUseViolations()) {
        return false;
    }
    if (!report.parentReportActionID ?? 0) {
        return false;
    }
    const parentReportAction = lodashGet(reportActions, `${report.parentReportID}.${report.parentReportActionID}`);
    if (!parentReportAction) {
        return false;
    }
    const transactionID = parentReportAction['originalMessage']['IOUTransactionID'] ?? 0;
    if (!transactionID) {
        return false;
    }
    const reportID = parentReportAction['originalMessage']['IOUReportID'] ?? 0;
    if (!reportID) {
        return false;
    }
    if (!isCurrentUserSubmitter(reportID)) {
        return false;
    }
    return transactionHasViolation(transactionID);
}
function transactionHasViolation(transactionID) {
    const violations = lodashGet(transactionViolations, transactionID, []);
    return _.some(violations, violation => violation.type === 'violation');
}
function reportHasViolations(reportID) {
    let transactions = TransactionUtils.getAllReportTransactions(reportID);
    return _.some(transactions, (transaction) => transactionHasViolation(transaction.transactionID));
}
// Always show IOU reports with violations
if (isExpenseRequest(report) && transactionThreadHasViolations(report)) {
    return true;
}
@cead22 cead22 added Engineering Daily KSv2 Improvement Item broken or needs improvement. labels Nov 9, 2023
@cead22
Copy link
Contributor Author

cead22 commented Nov 9, 2023

@lindboe @cdanwards can you please comment here so I can assign you?

@cdanwards
Copy link
Contributor

Commenting for assignment

Copy link

melvin-bot bot commented Nov 14, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Nov 14, 2023
@cdanwards
Copy link
Contributor

Beginning work on this now.

Copy link

melvin-bot bot commented Nov 21, 2023

@cdanwards Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Nov 21, 2023
Copy link

melvin-bot bot commented Nov 23, 2023

@cdanwards Still overdue 6 days?! Let's take care of this!

Copy link

melvin-bot bot commented Nov 27, 2023

@cdanwards 10 days overdue. I'm getting more depressed than Marvin.

@cdanwards
Copy link
Contributor

Still working on this. Getting some clarification after ReportUtils got swapped to TS.

@melvin-bot melvin-bot bot removed the Overdue label Nov 27, 2023
@cdanwards
Copy link
Contributor

Alright there was a little trouble with converting things over to TS and solving that weird tests bug but got everything working and polishing it up for the PR!

@cead22
Copy link
Contributor Author

cead22 commented Nov 29, 2023

Awesome! @cdanwards can you add another condition to transactionThreadHasViolations to return false if the report is on a state that isn't open or processing? We only wanna show RBR on the left hand nav to the report submitter, and only if the report is open or processing

@cdanwards
Copy link
Contributor

@cead22 Definitely! I'll add that in.

@cead22 cead22 self-assigned this Dec 5, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2023
@cead22
Copy link
Contributor Author

cead22 commented Dec 8, 2023

Work in progress

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 8, 2023
@cead22
Copy link
Contributor Author

cead22 commented Dec 12, 2023

Still a work in progress

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 12, 2023
@cead22
Copy link
Contributor Author

cead22 commented Dec 14, 2023

@trevor-coleman can you comment here so I can assign you please?

@melvin-bot melvin-bot bot removed the Overdue label Dec 14, 2023
@cead22
Copy link
Contributor Author

cead22 commented Jan 19, 2024

PR was merged and is on staging. We found a bug that we're fixing now

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 19, 2024
@melvin-bot melvin-bot bot changed the title Implement and use ReportUtils.transactionThreadHasViolations(report) [HOLD for payment 2024-01-28] Implement and use ReportUtils.transactionThreadHasViolations(report) Jan 21, 2024
Copy link

melvin-bot bot commented Jan 21, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 21, 2024
Copy link

melvin-bot bot commented Jan 21, 2024

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

If no regressions arise, payment will be issued on 2024-01-28. 🎊

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

  • @cdanwards does not require payment (Contractor)

@melvin-bot melvin-bot bot added the Overdue label Jan 30, 2024
@cead22 cead22 closed this as completed Jan 31, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jan 31, 2024
@cead22 cead22 removed the Improvement Item broken or needs improvement. label Feb 23, 2024
@cead22 cead22 reopened this Feb 23, 2024
@cead22 cead22 added the NewFeature Something to build that is a new item. label Feb 23, 2024
Copy link

melvin-bot bot commented Feb 23, 2024

@cead22
Copy link
Contributor Author

cead22 commented Feb 23, 2024

@strepanier03 can you please handle payment to @situchan for the PR review, and please make it double the standard amount we pay for these. This was was a pretty complex review and this is something I offered @situchan last month to put the review at the top of the priority list. Thanks

@strepanier03
Copy link
Contributor

Will do @cead22!

@strepanier03 strepanier03 added Daily KSv2 and removed Weekly KSv2 labels Mar 1, 2024
@strepanier03
Copy link
Contributor

@situchan - An offer has been sent. I'll check later today to finish paying out and closing up.

@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2024
@strepanier03
Copy link
Contributor

Paid and contract closed. Thanks everyone!

@melvin-bot melvin-bot bot removed the Overdue label Mar 4, 2024
@strepanier03 strepanier03 changed the title [HOLD for payment 2024-01-28] Implement and use ReportUtils.transactionThreadHasViolations(report) [PAID] Implement and use ReportUtils.transactionThreadHasViolations(report) Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

4 participants