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

Fix/30868: Incorrect group displayed #31424

Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7804843
fix incorrect group displayed
DylanDylann Nov 16, 2023
4f5a617
fix merge main
DylanDylann Nov 21, 2023
2252f7c
fix merge main
DylanDylann Nov 23, 2023
cb0782f
fix does not update timestamp with empty report
DylanDylann Nov 27, 2023
c32c0c9
fix merge conflict
DylanDylann Nov 28, 2023
7988973
fix lint
DylanDylann Nov 28, 2023
87f5f0e
fix add comment
DylanDylann Nov 30, 2023
cadc716
fix add comment
DylanDylann Nov 30, 2023
dd11d73
Merge branch 'Expensify:main' into fix/30868-difference-group-chat-ap…
DylanDylann Dec 1, 2023
87c8aae
fix merge main
DylanDylann Dec 1, 2023
410e3d4
fix add comments to logic
DylanDylann Dec 1, 2023
5a89b41
fix additional space
DylanDylann Dec 1, 2023
3355279
fix try to fix performance issue
DylanDylann Dec 1, 2023
a41c14c
Merge branch 'main' into fix/30868-difference-group-chat-appear-when-…
DylanDylann Dec 2, 2023
1ed435f
fix merge main
DylanDylann Dec 5, 2023
6ac934c
fix try to fix performance regression
DylanDylann Dec 5, 2023
9ae4d5b
fix remove space
DylanDylann Dec 5, 2023
8423ed6
fix update when going back
DylanDylann Dec 5, 2023
7cd02ab
fix jest test 1
DylanDylann Dec 5, 2023
4da0520
fix jest test 1
DylanDylann Dec 5, 2023
07f8ea5
fix disable update timestamp when back
DylanDylann Dec 5, 2023
37f11e5
fix performance regression
DylanDylann Dec 5, 2023
34dc79f
fix reassure error
DylanDylann Dec 6, 2023
32400c4
fix reassure regression
DylanDylann Dec 6, 2023
df2ab5e
fix merge main
DylanDylann Dec 12, 2023
83d3864
fix merge main
DylanDylann Dec 15, 2023
b80d70e
fix merge main
DylanDylann Dec 15, 2023
e64c615
fix remove console log
DylanDylann Dec 15, 2023
1f8552c
fix remove comment
DylanDylann Dec 15, 2023
17a67d7
fix lint
DylanDylann Dec 15, 2023
fe8dfde
Merge branch 'main' into fix/30868-difference-group-chat-appear-when-…
DylanDylann Dec 20, 2023
06dfd19
fix filter function
DylanDylann Dec 21, 2023
3959145
fix sort function
DylanDylann Dec 21, 2023
9feb1d2
fix lint issue
DylanDylann Dec 21, 2023
3240264
fix comment
DylanDylann Dec 26, 2023
d9dd286
fix add comment to prop
DylanDylann Dec 29, 2023
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
18 changes: 12 additions & 6 deletions src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import usePermissions from '@hooks/usePermissions';
import * as ReportUtils from '@libs/ReportUtils';
import * as App from '@userActions/App';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, Report} from '@src/types/onyx';
import type {Policy, Report, ReportMetadata} from '@src/types/onyx';
import type {ReportScreenWrapperProps} from './ReportScreenWrapper';

type ReportScreenIDSetterComponentProps = {
Expand All @@ -16,6 +16,8 @@ type ReportScreenIDSetterComponentProps = {

/** Whether user is a new user */
isFirstTimeNewExpensifyUser: OnyxEntry<boolean>;

reportsMetadata: OnyxCollection<ReportMetadata>;
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
};

type ReportScreenIDSetterProps = ReportScreenIDSetterComponentProps & ReportScreenWrapperProps;
Expand All @@ -29,15 +31,15 @@ const getLastAccessedReportID = (
policies: OnyxCollection<Policy>,
isFirstTimeNewExpensifyUser: OnyxEntry<boolean>,
openOnAdminRoom: boolean,
reportsMetadata: OnyxCollection<ReportMetadata>,
): string | undefined => {
const lastReport = ReportUtils.findLastAccessedReport(reports, ignoreDefaultRooms, policies, !!isFirstTimeNewExpensifyUser, openOnAdminRoom);
const lastReport = ReportUtils.findLastAccessedReport(reports, ignoreDefaultRooms, policies, !!isFirstTimeNewExpensifyUser, openOnAdminRoom, reportsMetadata);
return lastReport?.reportID;
};

// This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params
function ReportScreenIDSetter({route, reports, policies, navigation, isFirstTimeNewExpensifyUser = false}: ReportScreenIDSetterProps) {
function ReportScreenIDSetter({route, reports, policies, navigation, isFirstTimeNewExpensifyUser = false, reportsMetadata}: ReportScreenIDSetterProps) {
const {canUseDefaultRooms} = usePermissions();

useEffect(() => {
// Don't update if there is a reportID in the params already
if (route?.params?.reportID) {
Expand All @@ -46,7 +48,7 @@ function ReportScreenIDSetter({route, reports, policies, navigation, isFirstTime
}

// If there is no reportID in route, try to find last accessed and use it for setParams
const reportID = getLastAccessedReportID(reports, !canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, !!reports?.params?.openOnAdminRoom);
const reportID = getLastAccessedReportID(reports, !canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, !!reports?.params?.openOnAdminRoom, reportsMetadata);

// It's possible that reports aren't fully loaded yet
// in that case the reportID is undefined
Expand All @@ -55,7 +57,7 @@ function ReportScreenIDSetter({route, reports, policies, navigation, isFirstTime
} else {
App.confirmReadyToOpenApp();
}
}, [route, navigation, reports, canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser]);
}, [route, navigation, reports, canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, reportsMetadata]);

// The ReportScreen without the reportID set will display a skeleton
// until the reportID is loaded and set in the route param
Expand All @@ -77,4 +79,8 @@ export default withOnyx<ReportScreenIDSetterProps, ReportScreenIDSetterComponent
key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER,
initialValue: false,
},
reportsMetadata: {
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
allowStaleData: true,
},
})(ReportScreenIDSetter);
15 changes: 9 additions & 6 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import CONST from '@src/CONST';
import {ParentNavigationSummaryParams, TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import {Beta, Login, PersonalDetails, PersonalDetailsList, Policy, Report, ReportAction, Session, Transaction} from '@src/types/onyx';
import {Beta, Login, PersonalDetails, PersonalDetailsList, Policy, Report, ReportAction, ReportMetadata, Session, Transaction} from '@src/types/onyx';
import {Errors, Icon, PendingAction} from '@src/types/onyx/OnyxCommon';
import {IOUMessage, OriginalMessageActionName, OriginalMessageCreated} from '@src/types/onyx/OriginalMessage';
import {NotificationPreference} from '@src/types/onyx/Report';
Expand Down Expand Up @@ -550,12 +550,14 @@ function isDraftExpenseReport(report: OnyxEntry<Report>): boolean {
/**
* Given a collection of reports returns them sorted by last read
*/
function sortReportsByLastRead(reports: OnyxCollection<Report>): Array<OnyxEntry<Report>> {
function sortReportsByLastRead(reports: OnyxCollection<Report>, reportsMetadata: OnyxCollection<ReportMetadata>): Array<OnyxEntry<Report>> {
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
return Object.values(reports ?? {})
.filter((report) => !!report?.reportID && !!report?.lastReadTime)
.filter(
(report) => !!report?.reportID && ((reportsMetadata && !!reportsMetadata[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime) ?? !!report?.lastReadTime),
)
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
.sort((a, b) => {
const aTime = new Date(a?.lastReadTime ?? '');
const bTime = new Date(b?.lastReadTime ?? '');
const aTime = new Date((reportsMetadata && a && reportsMetadata[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a.reportID}`]?.lastVisitTime) ?? a?.lastReadTime ?? '');
const bTime = new Date((reportsMetadata && b && reportsMetadata[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${b.reportID}`]?.lastVisitTime) ?? b?.lastReadTime ?? '');
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved

return aTime.valueOf() - bTime.valueOf();
});
Expand Down Expand Up @@ -819,13 +821,14 @@ function findLastAccessedReport(
policies: OnyxCollection<Policy>,
isFirstTimeNewExpensifyUser: boolean,
openOnAdminRoom = false,
reportsMetadata: OnyxCollection<ReportMetadata> = {},
): 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);
let sortedReports = sortReportsByLastRead(reports, reportsMetadata);

let adminReport: OnyxEntry<Report> | undefined;
if (openOnAdminRoom) {
Expand Down
9 changes: 9 additions & 0 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ function openReport(
isLoadingInitialReportActions: true,
isLoadingOlderReportActions: false,
isLoadingNewerReportActions: false,
lastVisitTime: DateUtils.getDBTime(),
},
},
];
Expand Down Expand Up @@ -2514,6 +2515,13 @@ function searchInServer(searchInput: string) {
debouncedSearchInServer(searchInput);
}

function updateLastVisitTime(reportID: string) {
if (!ReportUtils.isValidReportIDFromPath(reportID)) {
return;
}
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {lastVisitTime: DateUtils.getDBTime()});
}

function clearNewRoomFormError() {
Onyx.set(ONYXKEYS.FORMS.NEW_ROOM_FORM, {
isLoading: false,
Expand Down Expand Up @@ -2582,5 +2590,6 @@ export {
openRoomMembersPage,
savePrivateNotesDraft,
getDraftPrivateNote,
updateLastVisitTime,
clearNewRoomFormError,
};
9 changes: 9 additions & 0 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {useIsFocused} from '@react-navigation/native';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
Expand Down Expand Up @@ -247,6 +248,14 @@ function ReportScreen({
return reportIDFromPath !== '' && report.reportID && !isTransitioning;
}, [route, report]);

const isFocused = useIsFocused();
useEffect(() => {
if (!report.reportID || !isFocused) {
return;
}
Report.updateLastVisitTime(report.reportID);
}, [report.reportID, isFocused]);

const fetchReportIfNeeded = useCallback(() => {
const reportIDFromPath = getReportID(route);

Expand Down
2 changes: 1 addition & 1 deletion src/types/onyx/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Report = {
/** The time of the last read of the report */
lastReadCreated?: string;

/** The last time the report was visited */
/** The time when user read the last message */
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
lastReadTime?: string;

/** The sequence number of the last report visit */
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/ReportMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ type ReportMetadata = {

/** Flag to check if the report actions data are loading */
isLoadingInitialReportActions?: boolean;

/** The time of the last visit of the report */
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
lastVisitTime?: string;
};

export default ReportMetadata;
Loading