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

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 17 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 @@ -233,7 +233,7 @@ function MoneyRequestView({
}

let receiptURIs;
const hasErrors = canEdit && TransactionUtils.hasMissingSmartscanFields(transaction);
const hasErrors = TransactionUtils.hasMissingSmartscanFields(transaction);
if (hasReceipt) {
receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction);
}
Expand Down
6 changes: 3 additions & 3 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,17 +521,17 @@ function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<
const parentReportAction: OnyxEntry<ReportAction> =
!report?.parentReportID || !report?.parentReportActionID ? null : allReportActions?.[report.parentReportID ?? '']?.[report.parentReportActionID ?? ''] ?? null;

if (parentReportAction?.actorAccountID === currentUserAccountID && ReportActionUtils.isTransactionThread(parentReportAction)) {
if (ReportActionUtils.isRequestor(parentReportAction) && ReportActionUtils.isTransactionThread(parentReportAction)) {
const transactionID = parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? parentReportAction?.originalMessage?.IOUTransactionID : null;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) {
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.shouldShowRBRForMissingSmartscanFields(report?.reportID ?? '') && !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
9 changes: 9 additions & 0 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,13 @@ function isLinkedTransactionHeld(reportActionID: string, reportID: string): bool
return TransactionUtils.isOnHoldByTransactionID(getLinkedTransactionID(reportActionID, reportID) ?? '');
}

/**
* Check if the current user is the requestor of the action
*/
function isRequestor(reportAction: OnyxEntry<ReportAction>): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I kinda missed this in the proposal and original review, but I was thinking about reports when thinking about isRequestor, but for report actions, this name isn't great.

How about we call this something like ReportActionUtils.actionWasTakenByCurrentUser(reportAction)

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 renamed it in commit

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, let me know when this is ready for re-review. Thanks

return currentUserAccountID === reportAction?.actorAccountID;
}

export {
extractLinksFromMessageHtml,
getDismissedViolationMessageText,
Expand Down Expand Up @@ -1264,7 +1271,9 @@ export {
isActionableJoinRequest,
isActionableJoinRequestPending,
isActionableTrackExpense,
getAllReportActions,
isLinkedTransactionHeld,
isRequestor,
};

export type {LastVisibleMessage};
41 changes: 31 additions & 10 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2732,14 +2732,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 @@ -2756,6 +2748,33 @@ 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
*/
function hasMissingSmartscanFields(iouReportID: string): boolean {
return TransactionUtils.getAllReportTransactions(iouReportID).some(TransactionUtils.hasMissingSmartscanFields);
}

/**
* Check if iouReportID has required missing fields
*/
function shouldShowRBRForMissingSmartscanFields(iouReportID: string): boolean {
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;
}
if (!ReportActionsUtils.isRequestor(action)) {
return false;
}
return TransactionUtils.hasMissingSmartscanFields(transaction);
});
}

/**
* Given a parent IOU report action get report name for the LHN.
*/
Expand Down Expand Up @@ -6164,13 +6183,14 @@ 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 hasMissingFields = isLHNPreview ? shouldShowRBRForMissingSmartscanFields(IOUReportID) : hasMissingSmartscanFields(IOUReportID);
const isReportPreviewError = ReportActionsUtils.isReportPreviewAction(action) && hasMissingFields && !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 Expand Up @@ -6931,6 +6951,7 @@ export {
shouldReportBeInOptionList,
shouldReportShowSubscript,
shouldShowFlagComment,
shouldShowRBRForMissingSmartscanFields,
shouldUseFullTitleToDisplay,
sortReportsByLastRead,
updateOptimisticParentReportAction,
Expand Down
Loading