Skip to content

Split expense flow (bug fixes) #63323

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

Merged
lakchote merged 5 commits into from
Jun 5, 2025
Merged

Split expense flow (bug fixes) #63323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
73360da
Fix 63309 and 63308
ZhenjaHorbach Jun 3, 2025
2a856c0
Fix 63364
ZhenjaHorbach Jun 3, 2025
4a6b3dd
Fix 63376
ZhenjaHorbach Jun 3, 2025
cc26e0a
Revert fix for 63376
ZhenjaHorbach Jun 4, 2025
b6e5226
Merge branch 'main' into split-expense-flow-fix-new-bugs
ZhenjaHorbach Jun 5, 2025
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
1 change: 1 addition & 0 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12014,6 +12014,7 @@ function saveSplitTransactions(draftTransaction: OnyxEntry<OnyxTypes.Transaction
policyParams,
transactionParams,
moneyRequestReportID: report?.reportID,
existingTransaction: originalTransaction,
existingTransactionID,
isSplitExpense: true,
});
Expand Down
26 changes: 18 additions & 8 deletions src/pages/iou/SplitExpenseEditPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import FixedFooter from '@components/FixedFooter';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import ScreenWrapper from '@components/ScreenWrapper';
import ScrollView from '@components/ScrollView';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import {removeSplitExpenseField, updateSplitExpenseField} from '@libs/actions/IOU';
Expand All @@ -15,9 +16,11 @@ import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import type {SplitExpenseParamList} from '@libs/Navigation/types';
import Parser from '@libs/Parser';
import {getPolicy} from '@libs/PolicyUtils';
import {getPolicy, getTagLists} from '@libs/PolicyUtils';
import type {TransactionDetails} from '@libs/ReportUtils';
import {getParsedComment, getReportOrDraftReport, getTransactionDetails} from '@libs/ReportUtils';
import {hasEnabledTags} from '@libs/TagsOptionsListUtils';
import {getTag} from '@libs/TransactionUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand All @@ -31,9 +34,13 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
const {translate} = useLocalize();

const {reportID, transactionID, splitExpenseTransactionID = '', backTo} = route.params;
const report = getReportOrDraftReport(reportID);

const [splitExpenseDraftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${CONST.IOU.OPTIMISTIC_TRANSACTION_ID}`, {canBeMissing: false});
const splitExpenseDraftTransactionDetails = useMemo<Partial<TransactionDetails>>(() => getTransactionDetails(splitExpenseDraftTransaction) ?? {}, [splitExpenseDraftTransaction]);
const policy = getPolicy(report?.policyID);
const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${report?.policyID}`, {canBeMissing: false});
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${report?.policyID}`, {canBeMissing: false});

const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {canBeMissing: false});
const transactionDetails = useMemo<Partial<TransactionDetails>>(() => getTransactionDetails(transaction) ?? {}, [transaction]);
Expand All @@ -45,8 +52,11 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
const currentAmount = transactionDetailsAmount >= 0 ? Math.abs(Number(splitExpenseDraftTransactionDetails?.amount)) : Number(splitExpenseDraftTransactionDetails?.amount);
const currentDescription = getParsedComment(Parser.htmlToMarkdown(splitExpenseDraftTransactionDetails?.comment ?? ''));

const report = getReportOrDraftReport(reportID);
const policy = getPolicy(report?.policyID);
const transactionTag = getTag(splitExpenseDraftTransaction);
const policyTagLists = useMemo(() => getTagLists(policyTags), [policyTags]);

const shouldShowTag = !!policy?.areTagsEnabled && !!(transactionTag || hasEnabledTags(policyTagLists));
const shouldShowCategory = !!policy?.areCategoriesEnabled && !!policyCategories;

return (
<ScreenWrapper testID={SplitExpenseEditPage.displayName}>
Expand All @@ -59,7 +69,7 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
})}
onBackButtonPress={() => Navigation.goBack(backTo)}
/>
<View style={[styles.flex1]}>
<ScrollView>

@eh2077 eh2077 Jun 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we can consider using SelectionList which is used by MoneyRequestConfirmationList. I also found there's a related guide https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#handling-scroll-issues-with-nested-lists-in-react-native

The current solution works fine and I also found similar use cases of <ScrollView> in the codebase, so just comment for discussion. Maybe we can try SelectionList when we have longer list of menu items.

@ZhenjaHorbach ZhenjaHorbach Jun 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, SelectionList is needed for long dynamic lists
When in our case I don't think it is appropriate

But anyway
I don't think we need to focus on that right now !

<MenuItemWithTopDescription
shouldShowRightIcon
shouldRenderAsHTML
Expand All @@ -81,7 +91,7 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
titleWrapperStyle={styles.flex1}
numberOfLinesTitle={2}
/>
{!!policy?.areCategoriesEnabled && (
{shouldShowCategory && (
<MenuItemWithTopDescription
shouldShowRightIcon
key={translate('common.category')}
Expand All @@ -103,12 +113,12 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
titleStyle={styles.flex1}
/>
)}
{!!policy?.areTagsEnabled && (
{shouldShowTag && (
<MenuItemWithTopDescription
shouldShowRightIcon
key={translate('workspace.common.tags')}
description={translate('workspace.common.tags')}
title={splitExpenseDraftTransactionDetails?.tag}
title={transactionTag}
numberOfLinesTitle={2}
onPress={() => {
Navigation.navigate(
Expand Down Expand Up @@ -146,7 +156,7 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) {
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
/>
</View>
</ScrollView>
<FixedFooter style={styles.mtAuto}>
{Number(splitExpensesList?.length) > 2 && (
<Button
Expand Down
Loading