Skip to content

Fix: Expenses moved to Self DM, display a Join button on top of report #76043

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
pecanoro merged 19 commits into from
Dec 15, 2025
+38 −16
Merged

Fix: Expenses moved to Self DM, display a Join button on top of report #76043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f1bc53d
Expenses moved to Self DM, display a Join button on top of report
nkdengineer Nov 25, 2025
f094cea
Merge branch 'main' into fix/74897
nkdengineer Nov 27, 2025
1fa8e2b
implement isTrackExpenseReportNew function
nkdengineer Nov 27, 2025
a18adbc
remove parentReportAction getting from Onyx
nkdengineer Nov 28, 2025
0062dd7
Merge branch 'main' into fix/74897
nkdengineer Dec 3, 2025
63dfb15
mark isTrackExpenseReport deprecated
nkdengineer Dec 3, 2025
f1952de
fix eslint
nkdengineer Dec 3, 2025
eee3bce
fix eslint
nkdengineer Dec 3, 2025
96810cf
add dependency
nkdengineer Dec 3, 2025
0b901e8
disable deprecated lint
nkdengineer Dec 3, 2025
e4be804
disable lint
nkdengineer Dec 3, 2025
2e331c5
resolve conflict
nkdengineer Dec 4, 2025
a867b08
resolve conflict
nkdengineer Dec 8, 2025
25dcb4c
fix lint
nkdengineer Dec 8, 2025
dcc2f9b
remove unnecessary dependency
nkdengineer Dec 8, 2025
1cbdd80
fix lint
nkdengineer Dec 8, 2025
45d96c0
Merge branch 'main' into fix/74897
nkdengineer Dec 9, 2025
4d1b4e2
make parentReport as require parameter
nkdengineer Dec 12, 2025
48ae36c
resolve conflict
nkdengineer Dec 15, 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
30 changes: 26 additions & 4 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2511,8 +2511,8 @@ function isIOURequest(report: OnyxInputOrEntry<Report>): boolean {
}

/**
* A Track Expense Report is a thread where the parent the parentReportAction is a transaction, and
* parentReportAction has type of track.
* @deprecated Use isTrackExpenseReportNew function instead
*
*/
function isTrackExpenseReport(report: OnyxInputOrEntry<Report>): boolean {
if (isThread(report)) {
Expand All @@ -2523,6 +2523,17 @@ function isTrackExpenseReport(report: OnyxInputOrEntry<Report>): boolean {
return false;
}

/**
* A Track Expense Report is a thread where the parent the parentReportAction is a transaction, and
* parentReportAction has type of track.
*/
function isTrackExpenseReportNew(report: OnyxInputOrEntry<Report>, parentReport: OnyxInputOrEntry<Report>, parentReportAction: OnyxInputOrEntry<ReportAction>): boolean {

@Krishna2323 Krishna2323 Nov 28, 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.

@pecanoro Is this fine to do? The current isTrackExpenseReport uses values from the Onyx.connect() method, and we want to avoid using those functions. Updating isTrackExpenseReport to use values passed from the UI can be complex and time-consuming. Let me know what you think about this.

Slack thread: https://expensify.slack.com/archives/C02NK2DQWUX/p1759913248186279

cc: @DylanDylann

@DylanDylann DylanDylann Nov 28, 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.

@nkdengineer @Krishna2323

  1. If that we need to mark isTrackExpenseReport as deprecated
  2. So how do you ensure that your change in isTrackExpenseReport function don't cause regression?

@nkdengineer nkdengineer Nov 28, 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.

So how do you ensure that your change in isTrackExpenseReport function don't cause regression?

What do you mean?

@DylanDylann DylanDylann Nov 28, 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.

Because you created a new function, we will need to migrate to this new function in other places later. But in the new function, you updated the logic. This update can cause regression in other places but we can't see that here because you only use isTrackExpenseReportNew in one place

@nkdengineer nkdengineer Dec 1, 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.

This update can cause regression in other places but we can't see that here because you only use isTrackExpenseReportNew in one place

It's just a better way to check the selfDM, so I don't think it can cause any regression.

@Krishna2323 Krishna2323 Dec 2, 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.

Thanks @DylanDylann, I think that's a better approach and has very low chances of causing regression.

@Krishna2323 Krishna2323 Dec 2, 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.

@nkdengineer could you please mark the old function as deprecated so it’s clear that we shouldn’t use it anymore?

@pecanoro pecanoro Dec 8, 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.

What is the plan to remove the other function? Do we need to create a new issue so someone can handle it?

@nkdengineer nkdengineer Dec 9, 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.

@pecanoro We can create a new issue for this. Happy to handle this if needed.

@nkdengineer nkdengineer Dec 16, 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.

What is the plan to remove the other function? Do we need to create a new issue so someone can handle it?

@pecanoro Do we want to create a new issue for this?

if (isThread(report)) {
return !isEmptyObject(parentReportAction) && isSelfDM(parentReport) && isTrackExpenseAction(parentReportAction);
}
return false;
}

/**
* Checks if a report is an IOU or expense request.
*/
Expand Down Expand Up @@ -2631,6 +2642,7 @@ function isOneTransactionThread(report: OnyxEntry<Report>, parentReport: OnyxEnt
* Checks if given report is a transaction thread
*/
function isReportTransactionThread(report: OnyxEntry<Report>) {
// eslint-disable-next-line @typescript-eslint/no-deprecated
return isMoneyRequest(report) || isTrackExpenseReport(report);
}

Expand Down Expand Up @@ -4889,6 +4901,7 @@ function canEditReportAction(reportAction: OnyxInputOrEntry<ReportAction>): bool
}

function canModifyHoldStatus(report: Report, reportAction: ReportAction): boolean {
// eslint-disable-next-line @typescript-eslint/no-deprecated
if (!isMoneyRequestReport(report) || isTrackExpenseReport(report)) {
return false;
}
Expand Down Expand Up @@ -4918,6 +4931,7 @@ function canHoldUnholdReportAction(
const isApproved = isReportApproved({report});
const isRequestIOU = isIOUReport(report);
const isHoldActionCreator = isActionCreator(holdReportAction);
// eslint-disable-next-line @typescript-eslint/no-deprecated
const isTrackExpenseMoneyReport = isTrackExpenseReport(report);
const isActionOwner = isActionCreator(reportAction);
const isApprover = isMoneyRequestReport(report) && report.managerID !== null && currentUserPersonalDetails?.accountID === report?.managerID;
Expand Down Expand Up @@ -11119,7 +11133,13 @@ function isAdminOwnerApproverOrReportOwner(report: OnyxEntry<Report>, policy: On
/**
* Whether the user can join a report
*/
function canJoinChat(report: OnyxEntry<Report>, parentReportAction: OnyxInputOrEntry<ReportAction>, policy: OnyxEntry<Policy>, isReportArchived = false): boolean {
function canJoinChat(
report: OnyxEntry<Report>,
parentReportAction: OnyxInputOrEntry<ReportAction>,
policy: OnyxEntry<Policy>,
parentReport: OnyxEntry<Report>,
isReportArchived = false,
): boolean {
// We disabled thread functions for whisper action
// So we should not show join option for existing thread on whisper message that has already been left, or manually leave it
if (isWhisperAction(parentReportAction)) {
Expand All @@ -11131,7 +11151,7 @@ function canJoinChat(report: OnyxEntry<Report>, parentReportAction: OnyxInputOrE
return false;
}

const isExpenseChat = isMoneyRequestReport(report) || isMoneyRequest(report) || isInvoiceReport(report) || isTrackExpenseReport(report);
const isExpenseChat = isMoneyRequestReport(report) || isMoneyRequest(report) || isInvoiceReport(report) || isTrackExpenseReportNew(report, parentReport, parentReportAction);
// Anyone viewing these chat types is already a participant and therefore cannot join
if (isRootGroupChat(report, isReportArchived) || isSelfDM(report) || isInvoiceRoom(report) || isSystemChat(report) || isExpenseChat) {
return false;
Expand Down Expand Up @@ -13262,6 +13282,7 @@ export {
isSystemChat,
isTaskReport,
isThread,
// eslint-disable-next-line @typescript-eslint/no-deprecated
isTrackExpenseReport,
isUnread,
isUnreadWithMention,
Expand Down Expand Up @@ -13403,6 +13424,7 @@ export {
shouldBlockSubmitDueToStrictPolicyRules,
isWorkspaceChat,
isOneTransactionReport,
isTrackExpenseReportNew,
shouldHideSingleReportField,
};

Expand Down
6 changes: 3 additions & 3 deletions src/pages/ReportDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ import {
isSystemChat as isSystemChatUtil,
isTaskReport as isTaskReportUtil,
isThread as isThreadUtil,
isTrackExpenseReport as isTrackExpenseReportUtil,
isTrackExpenseReportNew as isTrackExpenseReportUtil,
isUserCreatedPolicyRoom as isUserCreatedPolicyRoomUtil,
isWorkspaceChat as isWorkspaceChatUtil,
isWorkspaceMemberLeavingWorkspaceRoom as isWorkspaceMemberLeavingWorkspaceRoomUtil,
Expand Down Expand Up @@ -198,7 +198,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
const isInvoiceRoom = useMemo(() => isInvoiceRoomUtil(report), [report]);
const isTaskReport = useMemo(() => isTaskReportUtil(report), [report]);
const isSelfDM = useMemo(() => isSelfDMUtil(report), [report]);
const isTrackExpenseReport = useMemo(() => isTrackExpenseReportUtil(report), [report]);
const isTrackExpenseReport = useMemo(() => isTrackExpenseReportUtil(report, parentReport, parentReportAction), [report, parentReport, parentReportAction]);
const isCanceledTaskReport = isCanceledTaskReportUtil(report, parentReportAction);
const isParentReportArchived = useReportIsArchived(parentReport?.reportID);
const isTaskModifiable = canModifyTask(report, currentUserPersonalDetails?.accountID, isParentReportArchived);
Expand Down Expand Up @@ -669,7 +669,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
expensifyIcons.Camera,
]);

const canJoin = canJoinChat(report, parentReportAction, policy, !!reportNameValuePairs?.private_isArchived);
const canJoin = canJoinChat(report, parentReportAction, policy, parentReport, !!reportNameValuePairs?.private_isArchived);

const promotedActions = useMemo(() => {
const result: PromotedAction[] = [];
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/HeaderView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function HeaderView({report, parentReportAction, onNavigationMenuButtonClicked,

const join = callFunctionIfActionIsAllowed(() => joinRoom(report));

const canJoin = canJoinChat(report, parentReportAction, policy, isReportArchived);
const canJoin = canJoinChat(report, parentReportAction, policy, parentReport, isReportArchived);

const joinButton = (
<Button
Expand Down
16 changes: 8 additions & 8 deletions tests/unit/ReportUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6830,7 +6830,7 @@ describe('ReportUtils', () => {
},
};

expect(canJoinChat(report, parentReportAction, undefined)).toBe(false);
expect(canJoinChat(report, parentReportAction, undefined, undefined)).toBe(false);
});

it('should return false if the report is not hidden for the current user', async () => {
Expand All @@ -6842,7 +6842,7 @@ describe('ReportUtils', () => {

await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID});

expect(canJoinChat(report, undefined, undefined)).toBe(false);
expect(canJoinChat(report, undefined, undefined, undefined)).toBe(false);
});

it('should return false if the report is one of these types: group chat, selfDM, invoice room, system chat, expense chat', () => {
Expand All @@ -6851,15 +6851,15 @@ describe('ReportUtils', () => {
type: CONST.REPORT.TYPE.CHAT,
};

expect(canJoinChat(report, undefined, undefined)).toBe(false);
expect(canJoinChat(report, undefined, undefined, undefined)).toBe(false);
});

it('should return false if the report is archived', () => {
const report: Report = {
...createRandomReport(1, undefined),
};

expect(canJoinChat(report, undefined, undefined, true)).toBe(false);
expect(canJoinChat(report, undefined, undefined, undefined, true)).toBe(false);
});

it('should return true if the report is chat thread', async () => {
Expand All @@ -6878,7 +6878,7 @@ describe('ReportUtils', () => {

await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID});

expect(canJoinChat(report, undefined, undefined)).toBe(true);
expect(canJoinChat(report, undefined, undefined, undefined)).toBe(true);
});

it('should respect workspace membership for restricted visibility rooms', async () => {
Expand Down Expand Up @@ -6926,9 +6926,9 @@ describe('ReportUtils', () => {
visibility: CONST.REPORT.VISIBILITY.PUBLIC,
};

expect(canJoinChat(restrictedReport, undefined, policyWithoutCurrentUser)).toBe(false);
expect(canJoinChat(restrictedReport, undefined, policyWithCurrentUser)).toBe(true);
expect(canJoinChat(publicReport, undefined, policyWithoutCurrentUser)).toBe(true);
expect(canJoinChat(restrictedReport, undefined, policyWithoutCurrentUser, undefined)).toBe(false);
expect(canJoinChat(restrictedReport, undefined, policyWithCurrentUser, undefined)).toBe(true);
expect(canJoinChat(publicReport, undefined, policyWithoutCurrentUser, undefined)).toBe(true);
});
});

Expand Down
Loading