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: update how we show pending and scanning status #53112

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+9 −13
Open

fix: update how we show pending and scanning status #53112

Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f79edd0
fix: update how we show pending and scanning status
nkdengineer Nov 26, 2024
29cffee
Merge branch 'main' into fix/52921
nkdengineer Dec 2, 2024
1bde136
fix: style request preview content
nkdengineer Dec 2, 2024
2ca1154
Merge branch 'main' into fix/52921
nkdengineer Dec 19, 2024
37213ad
fix logic show pending
nkdengineer Dec 19, 2024
f868601
Merge branch 'main' into fix/52921
nkdengineer Dec 23, 2024
9440394
fix lint
nkdengineer Dec 23, 2024
980012c
fix lint
nkdengineer Dec 23, 2024
f08b0e0
fix lint
nkdengineer Dec 23, 2024
f51600b
fix lint
nkdengineer Dec 23, 2024
d9225c8
Not show receipt scan in progress
nkdengineer Dec 23, 2024
ab74606
fix: conflict and update
nkdengineer Dec 27, 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
13 changes: 6 additions & 7 deletions src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,20 @@
const route = useRoute<PlatformStackRouteProp<TransactionDuplicateNavigatorParamList, typeof SCREENS.TRANSACTION_DUPLICATE.REVIEW>>();
const {shouldUseNarrowLayout} = useResponsiveLayout();
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${chatReportID || '-1'}`);

Check failure on line 77 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const [session] = useOnyx(ONYXKEYS.SESSION);
const [iouReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${iouReportID || '-1'}`);

Check failure on line 79 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check


const policy = PolicyUtils.getPolicy(iouReport?.policyID);
const isMoneyRequestAction = ReportActionsUtils.isMoneyRequestAction(action);
const transactionID = isMoneyRequestAction ? ReportActionsUtils.getOriginalMessage(action)?.IOUTransactionID : '-1';

Check failure on line 83 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [walletTerms] = useOnyx(ONYXKEYS.WALLET_TERMS);
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);

const sessionAccountID = session?.accountID;
const managerID = iouReport?.managerID ?? -1;

Check failure on line 89 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const ownerAccountID = iouReport?.ownerAccountID ?? -1;

Check failure on line 90 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(chatReport);

const participantAccountIDs =
Expand Down Expand Up @@ -117,9 +117,9 @@
const isOnHold = TransactionUtils.isOnHold(transaction);
const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial;
const isPartialHold = isSettlementOrApprovalPartial && isOnHold;
const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '-1', transactionViolations, true);

Check failure on line 120 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

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

Check failure on line 121 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

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

Check failure on line 122 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction);
const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction);
const isFetchingWaypointsFromServer = TransactionUtils.isFetchingWaypointsFromServer(transaction);
Expand Down Expand Up @@ -155,7 +155,7 @@
const shouldShowHoldMessage = !(isSettled && !isSettlementOrApprovalPartial) && !!transaction?.comment?.hold;

const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${route.params?.threadReportID}`);
const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '', report?.parentReportActionID ?? '');

Check failure on line 158 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Check failure on line 158 in src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

const reviewingTransactionID = ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID ?? '-1' : '-1';

/*
Expand Down Expand Up @@ -203,6 +203,10 @@
message = translate('iou.split');
}

if (TransactionUtils.isPending(transaction)) {
Copy link
Contributor

@ntdiary ntdiary Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have a small question: should we use isPending or hasPendingUI here? It depends on whether we want to display the Pending field for the following cases (i.e., isReceiptBeingScanned/isPending/hasPendingRTERViolation):

App/src/libs/TransactionUtils/index.ts

Lines 754 to 759 in 5bb0e9b

/**
* Check if the transaction is pending or has a pending rter violation.
*/
function hasPendingUI(transaction: OnyxEntry<Transaction>, transactionViolations?: TransactionViolations | null): boolean {
return isReceiptBeingScanned(transaction) || isPending(transaction) || (!!transaction && hasPendingRTERViolation(transactionViolations));
}

cc @grgia @shawnborton

BTW, we have removed the Receipt pending match with card transaction message on line 259 in the PR.

Copy link
Contributor

@shawnborton shawnborton Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Curious what @grgia thinks about that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is an isScanning that returns early, we no longer need to consider isReceiptBeingScanned.

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

Lines 249 to 258 in cefad67

if (isScanning) {
return {shouldShow: true, messageIcon: ReceiptScan, messageDescription: translate('iou.receiptScanInProgress')};
}
if (TransactionUtils.isPending(transaction)) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}
if (TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', iouReport, policy)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) {

only need to determine whether both isPending and hasPendingRTERViolation should show "Pending". :)

Copy link
Contributor

Choose a reason for hiding this comment

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

only need to determine whether both isPending and hasPendingRTERViolation should show "Pending". :)

Hi, @grgia, any thoughts on the comment above? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

friendly bump @grgia

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm if the receipt is just Scanning, it doesn't mean it's pending the same way that a credit card transaction would be pending. So for the scanning case, I don't think we should show that there.

Also, I thought the idea was to also remove the "Receipt scan in progress" small text from the bottom? cc @Expensify/design for a gut check there!

Copy link
Contributor

Choose a reason for hiding this comment

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

^^ same, I thought so too!

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. I thought this is what we were going for:
CleanShot 2024-12-19 at 11 05 36@2x

Copy link
Contributor

Choose a reason for hiding this comment

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

Then, @nkdengineer, when you have time, can you please update the code according to the above comment?

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'll update soon

message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.pending')}`;
}

if (isSettled && !iouReport?.isCancelledIOU && !isPartialHold) {
message += ` ${CONST.DOT_SEPARATOR} ${getSettledMessage()}`;
return message;
Expand Down Expand Up @@ -250,23 +254,17 @@
if (isScanning) {
return {shouldShow: true, messageIcon: ReceiptScan, messageDescription: translate('iou.receiptScanInProgress')};
}
if (TransactionUtils.isPending(transaction)) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}
if (TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', iouReport, policy)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')};
}
return {shouldShow: false};
};

const pendingMessageProps = getPendingMessageProps();

const getDisplayAmountText = (): string => {
if (isScanning) {
return translate('iou.receiptScanning');
return translate('iou.receiptStatusTitle');
}

if (isFetchingWaypointsFromServer && !requestAmount) {
Expand Down Expand Up @@ -366,6 +364,7 @@
<Icon
src={Expensicons.DotIndicator}
fill={theme.danger}
small
/>
)}
</View>
Expand Down
Loading