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: Pay someone - Add receipt placeholder is shown when receipt is not allowed when paying someone #41635

Merged
merged 14 commits into from
May 31, 2024
Merged

fix: Pay someone - Add receipt placeholder is shown when receipt is not allowed when paying someone #41635

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1ffb954
fix: Pay someone - Add receipt placeholder is shown when receipt is n…
Krishna2323 May 4, 2024
4a19ebe
Merge branch 'Expensify:main' into krishna2323/issue/41489
Krishna2323 May 10, 2024
f080274
fix: show notice violation RBR only for paid policy.
Krishna2323 May 10, 2024
2685299
Merge branch 'Expensify:main' into krishna2323/issue/41489
Krishna2323 May 17, 2024
503cfb9
hide receipt header if no receipt or empty state is present.
Krishna2323 May 17, 2024
7e2e29c
Merge branch 'Expensify:main' into krishna2323/issue/41489
Krishna2323 May 24, 2024
d865727
hide empty receipt placeholder when it can't be edited in unpaid policy.
Krishna2323 May 24, 2024
10b6fa9
Merge branch 'Expensify:main' into krishna2323/issue/41489
Krishna2323 May 28, 2024
467295e
const name update.
Krishna2323 May 28, 2024
922daca
fix conflicts.
Krishna2323 May 29, 2024
8808273
Merge branch 'main' into krishna2323/issue/41489
Krishna2323 May 29, 2024
71feb6b
resolve merge conflicts.
Krishna2323 May 30, 2024
e167f90
Merge branch 'main' into krishna2323/issue/41489
Krishna2323 May 30, 2024
fa50929
fix merge conflicts.
Krishna2323 May 30, 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
7 changes: 5 additions & 2 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import {isTaxTrackingEnabled} from '@libs/PolicyUtils';
import * as ReceiptUtils from '@libs/ReceiptUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import ViolationsUtils from '@libs/Violations/ViolationsUtils';
Expand Down Expand Up @@ -130,6 +131,7 @@ function MoneyRequestView({
const cardProgramName = isCardTransaction && transactionCardID !== undefined ? CardUtils.getCardDescription(transactionCardID) : '';
const isApproved = ReportUtils.isReportApproved(moneyRequestReport);
const isInvoice = ReportUtils.isInvoiceReport(moneyRequestReport);
const isPayReport = ReportActionsUtils.isPayAction(parentReportAction);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isPayReport = ReportActionsUtils.isPayAction(parentReportAction);
const isPaidReport = ReportActionsUtils.isPayAction(parentReportAction);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to isPaidReport and merged main.

const taxRates = policy?.taxRates;
const formattedTaxAmount = CurrencyUtils.convertToDisplayString(transactionTaxAmount, transactionCurrency);

Expand Down Expand Up @@ -328,15 +330,16 @@ function MoneyRequestView({
);

const shouldShowMapOrReceipt = showMapAsImage || hasReceipt;
const shouldShowReceiptEmptyState = !hasReceipt && !isInvoice && (canEditReceipt || isAdmin || isApprover);
const isReceiptAllowed = !isPayReport && !isInvoice;
const shouldShowReceiptEmptyState = isReceiptAllowed && !hasReceipt && !isApproved && !isSettled && (canEditReceipt || isAdmin || isApprover);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Krishna2323 I believe we should show Receipt Header and Empty state only for paid polices as we are doing the same in shouldShowNotesViolations else it will be quite confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sobitneupane, sorry for delay, I think the empty state should be shown for unpaid policy but Receipt Header should be hidden.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Krishna2323 Can the receipt be edited by the other user(in P2P IOU) or admin in unpaid policy? If yes, we should leave the show empty state?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bump @Krishna2323

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sobitneupane, the receipt can't be edited/added by other user(in P2P IOU) or admin in unpaid policy.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, I don't think we should show empty state as we are going to use it to allow user to add receipt. To be fair, the thumbnail represents Add Receipt not Empty Receipt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree with that, I will update the code if required and test accordingly today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sobitneupane, code is updated.

const noticeTypeViolations = transactionViolations?.filter((violation) => violation.type === 'notice').map((v) => ViolationsUtils.getViolationTranslation(v, translate)) ?? [];
const shouldShowNotesViolations = !isReceiptBeingScanned && canUseViolations && ReportUtils.isPaidGroupPolicy(report);

return (
<View style={[StyleUtils.getReportWelcomeContainerStyle(isSmallScreenWidth, true, shouldShowAnimatedBackground)]}>
{shouldShowAnimatedBackground && <AnimatedEmptyStateBackground />}
<View style={shouldShowAnimatedBackground && [StyleUtils.getReportWelcomeTopMarginStyle(isSmallScreenWidth, true)]}>
{!isInvoice && (
{isReceiptAllowed && (
Copy link
Contributor

Choose a reason for hiding this comment

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

We should show Receipt Audit Header only if the receipt is present of if we are showing receipt empty state. I reckon there will be the cases where isReceiptAllowed is true but there is no receipt or receipt empty state. Example case: Once the request without receipt is approved. What's your thought?

Screenshot 2024-05-13 at 14 05 22

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 agree on hiding the header, updating and pushing the code in few minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

<ReceiptAuditHeader
notes={noticeTypeViolations}
shouldShowAuditMessage={Boolean(shouldShowNotesViolations && didRceiptScanSucceed)}
Expand Down
5 changes: 5 additions & 0 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,10 @@ function isTrackExpenseAction(reportAction: OnyxEntry<ReportAction | OptimisticI
return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && (reportAction.originalMessage as IOUMessage).type === CONST.IOU.REPORT_ACTION_TYPE.TRACK;
}

function isPayAction(reportAction: OnyxEntry<ReportAction | OptimisticIOUReportAction>): boolean {
return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && (reportAction.originalMessage as IOUMessage).type === CONST.IOU.REPORT_ACTION_TYPE.PAY;
}

function isTaskAction(reportAction: OnyxEntry<ReportAction>): boolean {
const reportActionName = reportAction?.actionName;
return (
Expand Down Expand Up @@ -1203,6 +1207,7 @@ export {
isSentMoneyReportAction,
isSplitBillAction,
isTrackExpenseAction,
isPayAction,
isTaskAction,
doesReportHaveVisibleActions,
isThreadParentMessage,
Expand Down
Loading