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: Combined report is more grayed #49633

Merged
merged 10 commits into from
Oct 2, 2024
Merged

fix: Combined report is more grayed #49633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3b81f84
fix: Combined report is more grayed
nkdengineer Sep 24, 2024
b474873
clean code
nkdengineer Sep 24, 2024
761cd4e
fix lint
nkdengineer Sep 24, 2024
c1b9645
fix lint
nkdengineer Sep 24, 2024
5683250
Update src/components/ReportActionItem/MoneyReportView.tsx
nkdengineer Sep 26, 2024
f8f37d1
Update src/components/ReportActionItem/MoneyRequestView.tsx
nkdengineer Sep 26, 2024
419d5c1
remove useless line
nkdengineer Sep 26, 2024
36ccd88
fix conflict
nkdengineer Sep 26, 2024
c9dd459
fix: ts check
nkdengineer Sep 26, 2024
c12613c
add comment
nkdengineer Oct 2, 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
8 changes: 6 additions & 2 deletions src/components/ReportActionItem/MoneyReportView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import * as reportActions from '@src/libs/actions/Report';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, PolicyReportField, Report} from '@src/types/onyx';
import type {PendingAction} from '@src/types/onyx/OnyxCommon';

type MoneyReportViewProps = {
/** The report currently being looked at */
Expand All @@ -41,9 +42,11 @@ type MoneyReportViewProps = {

/** Flag to show, hide the thread divider line */
shouldHideThreadDividerLine: boolean;

pendingAction?: PendingAction;
};

function MoneyReportView({report, policy, isCombinedReport = false, shouldShowTotal = true, shouldHideThreadDividerLine}: MoneyReportViewProps) {
function MoneyReportView({report, policy, isCombinedReport = false, shouldShowTotal = true, shouldHideThreadDividerLine, pendingAction}: MoneyReportViewProps) {
const theme = useTheme();
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
Expand Down Expand Up @@ -118,7 +121,8 @@ function MoneyReportView({report, policy, isCombinedReport = false, shouldShowTo

return (
<OfflineWithFeedback
pendingAction={report.pendingFields?.[fieldKey]}
// Need to return undefined when we have pendingAction to avoid the duplicate pending action
pendingAction={pendingAction ? undefined : report.pendingFields?.[fieldKey]}
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@MariaHCD MariaHCD Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can we add a comment to explain why we return undefined if there is a pendingAction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to return the pending field action here as undefined if the pendingAction is not undefined to avoid the duplicate pending action because if pendingAction exists, we already wrapped OfflineWithFeedback outside the MoneyRequestView

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, can we add this context as a comment in the code? I don't think it would be obvious to the next person who reads this code.

errors={report.errorFields?.[fieldKey]}
errorRowStyles={styles.ph5}
key={`menuItem-${fieldKey}`}
Expand Down
45 changes: 25 additions & 20 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(updatedTransaction ?? transaction);
}
const pendingAction = transaction?.pendingAction;
const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction;
// Need to return undefined when we have pendingAction to avoid the duplicate pending action
const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => (pendingAction ? undefined : transaction?.pendingFields?.[fieldPath]);
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason as above


const getErrorForField = useCallback(
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], policyHasDependentTags = false, tagValue?: string) => {
Expand Down Expand Up @@ -472,10 +473,12 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
{shouldShowAnimatedBackground && <AnimatedEmptyStateBackground />}
<>
{shouldShowReceiptAudit && (
<ReceiptAudit
notes={receiptViolations}
shouldShowAuditResult={!!shouldShowAuditMessage}
/>
<OfflineWithFeedback pendingAction={getPendingFieldAction('receipt')}>
<ReceiptAudit
notes={receiptViolations}
shouldShowAuditResult={!!shouldShowAuditMessage}
/>
</OfflineWithFeedback>
)}
{(hasReceipt || errors) && (
<OfflineWithFeedback
Expand Down Expand Up @@ -525,21 +528,23 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
</OfflineWithFeedback>
)}
{shouldShowReceiptEmptyState && (
<ReceiptEmptyState
hasError={hasErrors}
disabled={!canEditReceipt}
onPress={() =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
CONST.IOU.ACTION.EDIT,
iouType,
transaction?.transactionID ?? '-1',
report?.reportID ?? '-1',
Navigation.getReportRHPActiveRoute(),
),
)
}
/>
<OfflineWithFeedback pendingAction={getPendingFieldAction('receipt')}>
<ReceiptEmptyState
hasError={hasErrors}
disabled={!canEditReceipt}
onPress={() =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
CONST.IOU.ACTION.EDIT,
iouType,
transaction?.transactionID ?? '-1',
report?.reportID ?? '-1',
Navigation.getReportRHPActiveRoute(),
),
)
}
/>
</OfflineWithFeedback>
)}
{!shouldShowReceiptEmptyState && !hasReceipt && <View style={{marginVertical: 6}} />}
{shouldShowAuditMessage && <ReceiptAuditMessages notes={receiptImageViolations} />}
Expand Down
44 changes: 41 additions & 3 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7639,24 +7639,43 @@ function detachReceipt(transactionID: string) {
{
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: newTransaction,
value: {
...newTransaction,
pendingFields: {
receipt: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
},
},
},
];

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
pendingFields: {
receipt: null,
},
},
},
];
const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
...(transaction ?? null),
errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.receiptDeleteFailureError'),
pendingFields: {
receipt: null,
},
},
},
];

const parameters: DetachReceiptParams = {transactionID};

API.write(WRITE_COMMANDS.DETACH_RECEIPT, parameters, {optimisticData, failureData});
API.write(WRITE_COMMANDS.DETACH_RECEIPT, parameters, {optimisticData, successData, failureData});
}

function replaceReceipt(transactionID: string, file: File, source: string) {
Expand All @@ -7674,9 +7693,25 @@ function replaceReceipt(transactionID: string, file: File, source: string) {
value: {
receipt: receiptOptimistic,
filename: file.name,
pendingFields: {
receipt: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},
},
},
];

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
pendingFields: {
receipt: null,
},
},
},
];

nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -7685,6 +7720,9 @@ function replaceReceipt(transactionID: string, file: File, source: string) {
receipt: !isEmptyObject(oldReceipt) ? oldReceipt : null,
filename: transaction?.filename,
errors: getReceiptError(receiptOptimistic, file.name),
pendingFields: {
receipt: null,
},
},
},
];
Expand All @@ -7694,7 +7732,7 @@ function replaceReceipt(transactionID: string, file: File, source: string) {
receipt: file,
};

API.write(WRITE_COMMANDS.REPLACE_RECEIPT, parameters, {optimisticData, failureData});
API.write(WRITE_COMMANDS.REPLACE_RECEIPT, parameters, {optimisticData, successData, failureData});
}

/**
Expand Down
22 changes: 13 additions & 9 deletions src/pages/home/report/ReportActionItemContentCreated.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,17 @@ function ReportActionItemContentCreated({contextValue, parentReportAction, trans
}

return (
<ShowContextMenuContext.Provider value={contextMenuValue}>
<View>
<MoneyRequestView
report={report}
shouldShowAnimatedBackground
/>
{renderThreadDivider}
</View>
</ShowContextMenuContext.Provider>
<OfflineWithFeedback pendingAction={action.pendingAction}>
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
<ShowContextMenuContext.Provider value={contextMenuValue}>
<View>
<MoneyRequestView
report={report}
shouldShowAnimatedBackground
/>
{renderThreadDivider}
</View>
</ShowContextMenuContext.Provider>
</OfflineWithFeedback>
);
}

Expand Down Expand Up @@ -157,6 +159,7 @@ function ReportActionItemContentCreated({contextValue, parentReportAction, trans
report={report}
policy={policy}
isCombinedReport
pendingAction={action.pendingAction}
shouldShowTotal={transaction ? transactionCurrency !== report.currency : false}
shouldHideThreadDividerLine={shouldHideThreadDividerLine}
/>
Expand All @@ -174,6 +177,7 @@ function ReportActionItemContentCreated({contextValue, parentReportAction, trans
<MoneyReportView
report={report}
policy={policy}
pendingAction={action.pendingAction}
shouldHideThreadDividerLine={shouldHideThreadDividerLine}
/>
)}
Expand Down
Loading