[$250] Expense - First expense in the report is briefly highlighted after deleting expense

[jponikarchuk]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.1.41-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp
Email or phone of affected tester (no customers): #61243
Issue reported by: Applause Internal Team
Device used: Mac 15.3 / Chrome
App Component: Money Requests

Action Performed:

Precondition:

• Log in with Expensifail account.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit three expenses to the workspace chat.
4. Click on the expense preview.
5. Select an expense via checkbox.
6. Click on the dropdown.
7. Click Delete.

Expected Result:

The first expense in the report will not be highlighted after deleting expense.

Actual Result:

The first expense in the report is briefly highlighted after deleting expense.

Workaround:

Unknown

Platforms:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021920157419604664740
• Upwork Job ID: 1920157419604664740
• Last Price Increase: 2025-05-07

Issue Owner

Current Issue Owner: @rlinoz

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When deleting an expense, the first expense is highlighted as if it's new.

What is the root cause of that problem?

We calculate the new transaction based on this logic.

App/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

Lines 128 to 140 in f62ec5e

	const newTransactionID = useMemo(() => {
	if (!prevTransactions || transactions.length === prevTransactions.length) {
	return CONST.EMPTY_ARRAY as unknown as string[];
	}
	return transactions
	.filter((transaction) => !prevTransactions.some((prevTransaction) => prevTransaction.transactionID === transaction.transactionID))
	.reduce((latest, t) => {
	const inserted = t?.inserted ?? 0;
	const latestInserted = latest?.inserted ?? 0;
	return inserted > latestInserted ? t : latest;
	}, transactions.at(0))?.transactionID;
	}, [prevTransactions, transactions]);

First, we filter out the transaction list from the prev transaction list so we only left with the new transaction that doesn't exist on the prev transaction list. Then, we use reduce to get the transaction with the latest inserted value. The problem here is, we set the reduce initial value to the first transaction. When the filtered transaction list is empty (which happens when we delete an expense), then it will return the first transaction as the new transaction.

What changes do you think we should make in order to solve the problem?

We have 3 options.

First, we can ignore calculating the new transaction ID when the length of the transaction list is smaller than the prev transaction list.

App/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

Lines 128 to 131 in f62ec5e

	const newTransactionID = useMemo(() => {
	if (!prevTransactions || transactions.length === prevTransactions.length) {
	return CONST.EMPTY_ARRAY as unknown as string[];
	}

transactions.length <= prevTransactions.length

OR

Return early if the filtered transaction list is empty.

const newTransactions = transactions
    .filter((transaction) => !prevTransactions.some((prevTransaction) => prevTransaction.transactionID === transaction.transactionID));

if (!newTransactions.length) {
    return '';
}

OR

We can use for loop to find the maximum value, so when it's empty, the loop simply won't run and we will return an empty value.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021920157419604664740

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

[thelullabyy]

🚨 Edited by proposal-police: This proposal was edited at 2025-05-08 05:59:51 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

The first expense in the report is briefly highlighted after deleting expense.

What is the root cause of that problem?

The bug occurs when a transaction is deleted (so the current list is shorter than the previous list). So the filtered list becomes empty. After that, we use the reduce operation with the initial value is the first transaction, so the reduce operation still returns the first transaction from the original list (transactions.at(0)). This incorrectly marks the first transaction as "new" even though it's not new at all

App/src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

Lines 128 to 140 in f62ec5e

	const newTransactionID = useMemo(() => {
	if (!prevTransactions || transactions.length === prevTransactions.length) {
	return CONST.EMPTY_ARRAY as unknown as string[];
	}
	return transactions
	.filter((transaction) => !prevTransactions.some((prevTransaction) => prevTransaction.transactionID === transaction.transactionID))
	.reduce((latest, t) => {
	const inserted = t?.inserted ?? 0;
	const latestInserted = latest?.inserted ?? 0;
	return inserted > latestInserted ? t : latest;
	}, transactions.at(0))?.transactionID;
	}, [prevTransactions, transactions]);

What changes do you think we should make in order to solve the problem?

Before coming up with a solution, I noticed that the previous logic doesn't perform well when using both (filter, reduce)

I suggest updating to a new logic with one loop combined with using Set structure to optimize the performance and also solve the bug in this issue

                const prevTransactionIDs = new Set(prevTransactions.map(t => t.transactionID));
                let newestTransaction = null;
                let maxInserted = -1;
                
                for (const transaction of transactions) {
                    if (!prevTransactionIDs.has(transaction.transactionID)) {
                        const inserted = Number(transaction?.inserted ?? 0);
                        if (inserted > maxInserted) {
                            maxInserted = inserted;
                            newestTransaction = transaction;
                        }
                    }
                }
                
                return newestTransaction?.transactionID;

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can wrap it into a function and add UTs for it to prevent this bug from happening again from new changes in the future

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[bradyrose]

Proposal [Updated after policy clarification]

🧩 Problem

When an expense is deleted, the newTransactionID calculation misfires. This happens because the reduce() function is called on an empty filtered array, using transactions.at(0) as the fallback value. As a result, the first transaction is mistakenly flagged as "new", even though it existed before.

💥 Root Cause

const newTransactionID = useMemo(() => {
    if (!prevTransactions || transactions.length === prevTransactions.length) {
        return CONST.EMPTY_ARRAY as unknown as string[];
    }

    return transactions
        .filter(transaction => !prevTransactions.some(prev => prev.transactionID === transaction.transactionID))
        .reduce((latest, t) => {
            const inserted = t?.inserted ?? 0;
            const latestInserted = latest?.inserted ?? 0;
            return inserted > latestInserted ? t : latest;
        }, transactions.at(0))?.transactionID;
}, [prevTransactions, transactions]);

This logic fails when the .filter() returns an empty array — the .reduce() still returns the initial value (transactions.at(0)), creating the false appearance of a new transaction.

---

✅ Proposal

I suggest two logically sound and efficient variants that fix this issue. Either could be used depending on whether clarity or performance is the preferred priority.

---

🔹 Variant A: Clear filter + reduce approach

This version is direct and readable. It explicitly guards against the empty case before reducing:

const newTransactionID = useMemo(() => {
    if (!prevTransactions) {
        return '';
    }

    const newTransactions = transactions.filter(
        t => !prevTransactions.some(prev => prev.transactionID === t.transactionID)
    );

    if (!newTransactions.length) {
        return '';
    }

    const newestTransaction = newTransactions.reduce((latest, t) =>
        (t?.inserted ?? 0) > (latest?.inserted ?? 0) ? t : latest
    );

    return newestTransaction?.transactionID ?? '';
}, [prevTransactions, transactions]);

---

🔹 Variant B: Optimized Set-based lookup (for performance)

This version uses a Set to reduce lookup time and a single for loop. It avoids both .filter() and .reduce():

const newTransactionID = useMemo(() => {
    if (!prevTransactions) {
        return '';
    }

    const prevIDs = new Set(prevTransactions.map(t => t.transactionID));
    let newest = null;
    let maxInserted = -1;

    for (const t of transactions) {
        if (!prevIDs.has(t.transactionID)) {
            const inserted = Number(t?.inserted ?? 0);
            if (inserted > maxInserted) {
                maxInserted = inserted;
                newest = t;
            }
        }
    }

    return newest?.transactionID ?? '';
}, [prevTransactions, transactions]);

---

🆚 Differences from Existing Proposal (thelullabyy)

• I offer both a clear version and a performance-oriented version.
• My Variant A emphasizes readability and onboarding clarity, which may help other contributors.
• I use explicit early returns and avoid defaulting to transactions.at(0).
• I give maintainers the flexibility to pick the approach best aligned with team values.

---

🔒 Regression Protection Suggestion

Wrap this logic in a helper function (e.g., getNewestTransactionID(prev, current)) and write unit tests covering:

• No change
• New transaction added
• Transaction deleted
• Empty transaction list
• Duplicate transaction IDs

---

Let me know if you'd prefer I proceed with one variant or include both in a pull request. Thanks!

[rushatgabhane]

Withdrawing this proposal to comply with Expensify’s one-proposal-at-a-time policy

there is no such policy. please post your proposal as long as yours is different/better from the existing ones

[rushatgabhane]

@bernhardoj's proposal LGTM 🎀 👀 🎀

#61552 (comment)

it fixes the issue using the correct RCA. Other proposals suggest loop optimization but we shouldn't do that unless it is a quantifiable problem

[melvin-bot]

Triggered auto assignment to @rlinoz, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

PR is ready

cc: @rushatgabhane