Skip to content

fix: missing Create expense options #71871

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
neil-marcellini merged 4 commits into from
Oct 13, 2025
Merged

fix: missing Create expense options #71871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cd001af
fix: missing Create expense options
nkdengineer Oct 6, 2025
7123259
Merge branch 'main' into fix/71635
nkdengineer Oct 13, 2025
750b7ae
add unit test
nkdengineer Oct 13, 2025
10aaedb
change test name
nkdengineer Oct 13, 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
5 changes: 4 additions & 1 deletion src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,10 @@ function getOptionData({
// Assign the actor account ID from the last action when it’s a REPORT_PREVIEW action.
// to ensures that lastActorDetails.accountID is correctly set in case it's empty string
if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && lastActorDetails) {
lastActorDetails.accountID = lastAction.actorAccountID;
lastActorDetails = {

@neil-marcellini neil-marcellini Oct 13, 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.

Not a blocker, but please add a comment explaining why it's important to create a copy of the object.

...lastActorDetails,
accountID: lastAction.actorAccountID,
};
Comment on lines 727 to +731

@hungvu193 hungvu193 Nov 14, 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.

We ended up removing this piece of code in #73220

More details:
#73220 (comment)

}

const lastActorDisplayName = getLastActorDisplayName(lastActorDetails);
Expand Down
41 changes: 41 additions & 0 deletions tests/ui/LHNItemsPresence.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetailsList, Report, ReportAction, ViolationName} from '@src/types/onyx';
import type {ReportCollectionDataSet} from '@src/types/onyx/Report';
import {chatReportR14932} from '../../__mocks__/reportData/reports';
import createRandomReportAction from '../utils/collections/reportActions';
import getOnyxValue from '../utils/getOnyxValue';
import * as LHNTestUtils from '../utils/LHNTestUtils';
import * as TestHelper from '../utils/TestHelper';
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
Expand Down Expand Up @@ -407,6 +409,45 @@ describe('SidebarLinksData', () => {
// Then the report should not disappear in the sidebar because we are in the focus mode
expect(getOptionRows()).toHaveLength(0);
});

it('should not change the current user personal detail when a report with last action is REPORTPREVIEW is displayed', async () => {

@neil-marcellini neil-marcellini Oct 13, 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.

Let's switch to when a report is displayed where the last action is REPORTPREVIEW. I think that's slightly more clear.

// Given the SidebarLinks are rendered.
LHNTestUtils.getDefaultRenderedSidebarLinks();

const report: Report = {
...createReport(undefined, [1, 2], undefined, undefined, undefined, true),
lastActorAccountID: 1,
lastMessageText: '123456',
};

const lastReportAction: ReportAction = {
...createRandomReportAction(2),
actionName: 'REPORTPREVIEW',
actorAccountID: 2,
Comment on lines +418 to +426

@neil-marcellini neil-marcellini Oct 13, 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.

NAB: Can we please create variables to give names to these IDs? I think that makes it a lot more clear. For example, something like setting the last actor account ID to submitterAccountID vs 1.

message: [],
originalMessage: undefined,
person: [
{
type: 'TEXT',
style: 'strong',
text: TEST_USER_LOGIN,
},
],
};

await initializeState({
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
});

await waitForBatchedUpdatesWithAct();

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, {[lastReportAction.reportActionID]: lastReportAction});

await waitForBatchedUpdatesWithAct();

const personalDetail = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST);

@neil-marcellini neil-marcellini Oct 13, 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.

NAB: You verified that this fails without your fix, right?

expect(personalDetail?.[TEST_USER_ACCOUNT_ID]?.accountID).toBe(TEST_USER_ACCOUNT_ID);
});
});

describe('Report that should NOT be included in the LHN', () => {
Expand Down
Loading