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.

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

[NoQA] Add policyID to getOrderedReportIDs and sortReportsByLastRead #33302

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,7 @@ const CONST = {
CUSTOM_UNIT_RATE_BASE_OFFSET: 100,
OWNER_EMAIL_FAKE: '_FAKE_',
OWNER_ACCOUNT_ID_FAKE: 0,
ID_FAKE: '_FAKE_',
},

CUSTOM_UNITS: {
Expand Down
13 changes: 12 additions & 1 deletion src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,19 @@ const getLastAccessedReportID = (
isFirstTimeNewExpensifyUser: OnyxEntry<boolean>,
openOnAdminRoom: boolean,
reportMetadata: OnyxCollection<ReportMetadata>,
policyID?: string,
policyMemberAccountIDs?: number[],
): string | undefined => {
const lastReport = ReportUtils.findLastAccessedReport(reports, ignoreDefaultRooms, policies, !!isFirstTimeNewExpensifyUser, openOnAdminRoom, reportMetadata);
const lastReport = ReportUtils.findLastAccessedReport(
reports,
ignoreDefaultRooms,
policies,
!!isFirstTimeNewExpensifyUser,
openOnAdminRoom,
reportMetadata,
policyID,
policyMemberAccountIDs,
);
return lastReport?.reportID;
};

Expand Down
66 changes: 56 additions & 10 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -616,17 +616,22 @@ function isDraftExpenseReport(report: OnyxEntry<Report> | EmptyObject): boolean
}

/**
* Given a collection of reports returns them sorted by last read
* Checks if the supplied report has a common policy member with the array passed in params.
*/
function sortReportsByLastRead(reports: OnyxCollection<Report>, reportMetadata: OnyxCollection<ReportMetadata>): Array<OnyxEntry<Report>> {
return Object.values(reports ?? {})
.filter((report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime))
.sort((a, b) => {
const aTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '');
const bTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${b?.reportID}`]?.lastVisitTime ?? b?.lastReadTime ?? '');
function hasParticipantInArray(report: Report, policyMemberAccountIDs: number[]) {
if (!report.participantAccountIDs) {
return false;
}

return aTime.valueOf() - bTime.valueOf();
});
const policyMemberAccountIDsSet = new Set(policyMemberAccountIDs);

for (const reportParticipant of report.participantAccountIDs) {
if (policyMemberAccountIDsSet.has(reportParticipant)) {
return true;
}
}

return false;
}

/**
Expand Down Expand Up @@ -826,6 +831,37 @@ function isConciergeChatReport(report: OnyxEntry<Report>): boolean {
return report?.participantAccountIDs?.length === 1 && Number(report.participantAccountIDs?.[0]) === CONST.ACCOUNT_ID.CONCIERGE && !isChatThread(report);
}

/**
* Checks if the supplied report belongs to workspace based on the provided params. If the report's policyID is _FAKE_ or has no value, it means this report is a DM.
* In this case report and workspace members must be compared to determine whether the report belongs to the workspace.
*/
function doesReportBelongToWorkspace(report: Report, policyID: string, policyMemberAccountIDs: number[]) {
return (
isConciergeChatReport(report) || (report.policyID === CONST.POLICY.ID_FAKE || !report.policyID ? hasParticipantInArray(report, policyMemberAccountIDs) : report.policyID === policyID)
);
}

/**
* Given an array of reports, return them filtered by a policyID and policyMemberAccountIDs.
*/
function filterReportsByPolicyIdAndMemberAccountIDs(reports: Report[], policyID = '', policyMemberAccountIDs: number[] = []) {
return reports.filter((report) => !!report && doesReportBelongToWorkspace(report, policyID, policyMemberAccountIDs));
}

/**
* Given an array of reports, return them sorted by the last read timestamp.
*/
function sortReportsByLastRead(reports: Report[], reportMetadata: OnyxCollection<ReportMetadata>): Array<OnyxEntry<Report>> {
return reports
.filter((report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime))
.sort((a, b) => {
const aTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '');
const bTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${b?.reportID}`]?.lastVisitTime ?? b?.lastReadTime ?? '');

return aTime.valueOf() - bTime.valueOf();
});
}

/**
* Returns true if report is still being processed
*/
Expand Down Expand Up @@ -896,13 +932,22 @@ function findLastAccessedReport(
isFirstTimeNewExpensifyUser: boolean,
openOnAdminRoom = false,
reportMetadata: OnyxCollection<ReportMetadata> = {},
policyID?: string,
policyMemberAccountIDs: number[] = [],
): OnyxEntry<Report> {
// If it's the user's first time using New Expensify, then they could either have:
// - just a Concierge report, if so we'll return that
// - their Concierge report, and a separate report that must have deeplinked them to the app before they created their account.
// If it's the latter, we'll use the deeplinked report over the Concierge report,
// since the Concierge report would be incorrectly selected over the deep-linked report in the logic below.
let sortedReports = sortReportsByLastRead(reports, reportMetadata);

let reportsValues = Object.values(reports ?? {}) as Report[];

if (!!policyID || policyMemberAccountIDs.length > 0) {
reportsValues = filterReportsByPolicyIdAndMemberAccountIDs(reportsValues, policyID, policyMemberAccountIDs);
}

let sortedReports = sortReportsByLastRead(reportsValues, reportMetadata);

let adminReport: OnyxEntry<Report> | undefined;
if (openOnAdminRoom) {
Expand Down Expand Up @@ -4575,6 +4620,7 @@ export {
getReportFieldTitle,
shouldDisplayThreadReplies,
shouldDisableThread,
doesReportBelongToWorkspace,
getChildReportNotificationPreference,
};

Expand Down
18 changes: 16 additions & 2 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,22 @@ function getOrderedReportIDs(
policies: Record<string, Policy>,
priorityMode: ValueOf<typeof CONST.PRIORITY_MODE>,
allReportActions: OnyxCollection<ReportAction[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
): string[] {
// Generate a unique cache key based on the function arguments
const cachedReportsKey = JSON.stringify(
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
[currentReportId, allReports, betas, policies, priorityMode, allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${currentReportId}`]?.length || 1],
[
currentReportId,
allReports,
betas,
policies,
priorityMode,
allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${currentReportId}`]?.length ?? 1,
currentPolicyID,
policyMemberAccountIDs,
],
(key, value: unknown) => {
/**
* Exclude some properties not to overwhelm a cached key value with huge data,
Expand All @@ -151,7 +162,7 @@ function getOrderedReportIDs(
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);
// Filter out all the reports that shouldn't be displayed
const reportsToDisplay = allReportsDictValues.filter((report) => ReportUtils.shouldReportBeInOptionList(report, currentReportId ?? '', isInGSDMode, betas, policies, true));
let reportsToDisplay = allReportsDictValues.filter((report) => ReportUtils.shouldReportBeInOptionList(report, currentReportId ?? '', isInGSDMode, betas, policies, true));

if (reportsToDisplay.length === 0) {
// Display Concierge chat report when there is no report to be displayed
Expand All @@ -175,6 +186,9 @@ function getOrderedReportIDs(
const nonArchivedReports: Report[] = [];
const archivedReports: Report[] = [];

if (currentPolicyID || policyMemberAccountIDs.length > 0) {
reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, currentPolicyID, policyMemberAccountIDs));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi 👋 Coming from #35963

When the IOU report and details page are not highlighted in the LHN, the filter removes the current viewed report. So to fix it we decided to add a check where it's past the currently viewed report.

// There are a few properties that need to be calculated for the report which are used when sorting reports.
reportsToDisplay.forEach((report) => {
// Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params.
Expand Down
23 changes: 23 additions & 0 deletions src/libs/actions/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,28 @@ function openWorkspaceReimburseView(policyID: string) {
API.read('OpenWorkspaceReimburseView', params, {successData, failureData});
}

/**
* Returns the accountIDs of the members of the policy whose data is passed in the parameters
*/
function openWorkspace(policyID: string, clientMemberAccountIDs: number[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cuold you add docs here to explain what is returned in this case?

if (!policyID || !clientMemberAccountIDs) {
Log.warn('openWorkspace invalid params', {policyID, clientMemberAccountIDs});
return;
}

type OpenWorkspaceParams = {
policyID: string;
clientMemberAccountIDs: string;
};

const params: OpenWorkspaceParams = {
policyID,
clientMemberAccountIDs: JSON.stringify(clientMemberAccountIDs),
};

API.read('OpenWorkspace', params);
}

function openWorkspaceMembersPage(policyID: string, clientMemberEmails: string[]) {
if (!policyID || !clientMemberEmails) {
Log.warn('openWorkspaceMembersPage invalid params', {policyID, clientMemberEmails});
Expand Down Expand Up @@ -2063,6 +2085,7 @@ export {
createWorkspace,
openWorkspaceMembersPage,
openWorkspaceInvitePage,
openWorkspace,
removeWorkspace,
createWorkspaceFromIOUPayment,
setWorkspaceInviteMembersDraft,
Expand Down
18 changes: 9 additions & 9 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,15 +588,15 @@ describe('ReportUtils', () => {

describe('sortReportsByLastRead', () => {
it('should filter out report without reportID & lastReadTime and sort lastReadTime in ascending order', () => {
const reports = {
1: {reportID: 1, lastReadTime: '2023-07-08 07:15:44.030'},
2: {reportID: 2, lastReadTime: null},
3: {reportID: 3, lastReadTime: '2023-07-06 07:15:44.030'},
4: {reportID: 4, lastReadTime: '2023-07-07 07:15:44.030', type: CONST.REPORT.TYPE.IOU},
5: {lastReadTime: '2023-07-09 07:15:44.030'},
6: {reportID: 6},
7: {},
};
const reports = [
{reportID: 1, lastReadTime: '2023-07-08 07:15:44.030'},
{reportID: 2, lastReadTime: null},
{reportID: 3, lastReadTime: '2023-07-06 07:15:44.030'},
{reportID: 4, lastReadTime: '2023-07-07 07:15:44.030', type: CONST.REPORT.TYPE.IOU},
{lastReadTime: '2023-07-09 07:15:44.030'},
{reportID: 6},
{},
];
const sortedReports = [
{reportID: 3, lastReadTime: '2023-07-06 07:15:44.030'},
{reportID: 4, lastReadTime: '2023-07-07 07:15:44.030', type: CONST.REPORT.TYPE.IOU},
Expand Down
Loading