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

fix Reddot pinned chat appears for approver after failed scanned #39970

Merged
merged 20 commits into from
Jun 3, 2024
Merged

fix Reddot pinned chat appears for approver after failed scanned #39970

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
69263a4
fix Reddot pinned chat appears for approver for submitter's failed sc…
dukenv0307 Apr 9, 2024
fe5f752
fix lint
dukenv0307 Apr 9, 2024
1303c69
fix update hasMissingSmartscanFields
dukenv0307 Apr 16, 2024
f082a5a
fix update hasSmartscanError
dukenv0307 Apr 16, 2024
ec854c5
fix merge main
dukenv0307 Apr 23, 2024
48ad489
fix type
dukenv0307 Apr 23, 2024
590547f
fix type
dukenv0307 Apr 23, 2024
1be1d08
fix add comment
dukenv0307 Apr 26, 2024
de0ae91
Merge branch 'main' into fix/37044
dukenv0307 Apr 26, 2024
4d03c16
fix update comment
dukenv0307 May 3, 2024
4560da2
fix create shouldShowRBRForMissingSmartscanFields
dukenv0307 May 10, 2024
3fbf0f7
fix lint
dukenv0307 May 10, 2024
8866eb7
fix create hasMissingFields variable
dukenv0307 May 21, 2024
83500fc
fix merge main
dukenv0307 May 21, 2024
97ce514
fix suggestion logics
dukenv0307 May 27, 2024
4c35a47
fix create isRequestor
dukenv0307 May 27, 2024
0c68919
fix merge main
dukenv0307 May 27, 2024
6e342bc
Merge branch 'main' into fix/37044
dukenv0307 May 31, 2024
4ef770d
fix remove isLHNPreview and create actionWasTakenByCurrentUser
dukenv0307 May 31, 2024
728ae68
fix typecheck
dukenv0307 Jun 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ function MoneyRequestView({

const hasReceipt = TransactionUtils.hasReceipt(transaction);
let receiptURIs;
const hasErrors = canEdit && TransactionUtils.hasMissingSmartscanFields(transaction);
const hasErrors = TransactionUtils.hasMissingSmartscanFields(transaction);
if (hasReceipt) {
receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction);
}
Expand Down
4 changes: 2 additions & 2 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,10 @@ function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
}
} else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) {
if (ReportUtils.hasMissingSmartscanFields(report?.reportID ?? '') && !ReportUtils.isSettled(report?.reportID)) {
if (ReportUtils.hasMissingSmartscanFields(report?.reportID ?? '', true) && !ReportUtils.isSettled(report?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
}
} else if (ReportUtils.hasSmartscanError(Object.values(reportActions ?? {}))) {
} else if (ReportUtils.hasSmartscanError(Object.values(reportActions ?? {}), true)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we calling ReportUtils.hasSmartscanError from anywhere else? Do we need the second param?

Even if we need it, I don't think it should be a separate param, and instead we should have that check outside of the ReportUtils.hasSmartscanError function, since that function name sounds like it shouldn't care about isLHNPreview

Copy link
Contributor Author

@dukenv0307 dukenv0307 May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we calling ReportUtils.hasSmartscanError from anywhere else?

No

Do we need the second param?

I think we still need to create a isLHNPreview param then handle both case (isLHNPreview is true or false) in hasSmartscanError to reduce the duplicate logics.

As you can see, if we do like this, we just need to create a new variable, hasMissingField and apply a minor change to isReportPreviewError, the rest logic does not change.
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to create a isLHNPreview param then handle both case (isLHNPreview is true or false) in hasSmartscanError to reduce the duplicate logics.

I think I'm not following or I'm missing something, because if we always pass this as true, this doesn't make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I removed isLHNPreview in commit

reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
}
// All error objects related to the report. Each object in the sources contains error messages keyed by microtime
Expand Down
1 change: 1 addition & 0 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,7 @@ export {
isActionableJoinRequest,
isActionableJoinRequestPending,
isActionableTrackExpense,
getAllReportActions,
isLinkedTransactionHeld,
};

Expand Down
35 changes: 25 additions & 10 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2586,14 +2586,6 @@ function areAllRequestsBeingSmartScanned(iouReportID: string, reportPreviewActio
return transactionsWithReceipts.every((transaction) => TransactionUtils.isReceiptBeingScanned(transaction));
}

/**
* Check if any of the transactions in the report has required missing fields
*
*/
function hasMissingSmartscanFields(iouReportID: string): boolean {
return TransactionUtils.getAllReportTransactions(iouReportID).some((transaction) => TransactionUtils.hasMissingSmartscanFields(transaction));
}

/**
* Get the transactions related to a report preview with receipts
* Get the details linked to the IOU reportAction
Expand All @@ -2610,6 +2602,29 @@ function getLinkedTransaction(reportAction: OnyxEntry<ReportAction | OptimisticI
return allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? {};
}

/**
* Check if any of the transactions in the report has required missing fields
*
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
* NOTE: The function hasMissingSmartscanFields is used on both LHN preview and report action,
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
* so we need to create a isLHNPreview to handle both cases above
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
*/
function hasMissingSmartscanFields(iouReportID: string, isLHNPreview?: boolean): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't where we landed

I agree with this, we shouldn't change hasMissingSmartscanFields, instead we could create shouldShowRBRForMissingSmartscanFields method, which would in turn call hasMissingSmartscanFields conditionally

Let's change this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created shouldShowRBRForMissingSmartscanFields method

if (isLHNPreview) {
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
const reportActions = Object.values(ReportActionsUtils.getAllReportActions(iouReportID));
return reportActions.some((action) => {
if (!ReportActionsUtils.isMoneyRequestAction(action)) {
return false;
}
const transaction = getLinkedTransaction(action);
if (isEmptyObject(transaction)) {
return false;
}
return TransactionUtils.hasMissingSmartscanFields(transaction) && currentUserAccountID === action?.actorAccountID;
});
}
return TransactionUtils.getAllReportTransactions(iouReportID).some((transaction) => TransactionUtils.hasMissingSmartscanFields(transaction));
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Given a parent IOU report action get report name for the LHN.
*/
Expand Down Expand Up @@ -5736,13 +5751,13 @@ function canEditPolicyDescription(policy: OnyxEntry<Policy>): boolean {
/**
* Checks if report action has error when smart scanning
*/
function hasSmartscanError(reportActions: ReportAction[]) {
function hasSmartscanError(reportActions: ReportAction[], isLHNPreview: boolean) {
return reportActions.some((action) => {
if (!ReportActionsUtils.isSplitBillAction(action) && !ReportActionsUtils.isReportPreviewAction(action)) {
return false;
}
const IOUReportID = ReportActionsUtils.getIOUReportIDFromReportActionPreview(action);
const isReportPreviewError = ReportActionsUtils.isReportPreviewAction(action) && hasMissingSmartscanFields(IOUReportID) && !isSettled(IOUReportID);
const isReportPreviewError = ReportActionsUtils.isReportPreviewAction(action) && hasMissingSmartscanFields(IOUReportID, isLHNPreview) && !isSettled(IOUReportID);
const transactionID = (action.originalMessage as IOUMessage).IOUTransactionID ?? '0';
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? {};
const isSplitBillError = ReportActionsUtils.isSplitBillAction(action) && TransactionUtils.hasMissingSmartscanFields(transaction as Transaction);
Expand Down
Loading