Skip to content

Commit

Permalink
Merge pull request #40336 from tienifr/fix/38368
Browse files Browse the repository at this point in the history
fix empty chat displayed in focus mode
  • Loading branch information
flodnv authored May 10, 2024
2 parents d7216ea + 9509578 commit ce1fd06
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1752,7 +1752,7 @@ function getOptions(
betas,
policies,
doesReportHaveViolations,
isInGSDMode: false,
isInFocusMode: false,
excludeEmptyChats: false,
includeSelfDM,
});
Expand Down
24 changes: 18 additions & 6 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4885,11 +4885,23 @@ function buildOptimisticMoneyRequestEntities(
return [createdActionForChat, createdActionForIOUReport, iouAction, transactionThread, createdActionForTransactionThread];
}

// Check if the report is empty, meaning it has no visible messages (i.e. only a "created" report action).
function isEmptyReport(report: OnyxEntry<Report>): boolean {
if (!report) {
return true;
}
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;
}

function isUnread(report: OnyxEntry<Report>): boolean {
if (!report) {
return false;
}

if (isEmptyReport(report)) {
return false;
}
// lastVisibleActionCreated and lastReadTime are both datetime strings and can be compared directly
const lastVisibleActionCreated = report.lastVisibleActionCreated ?? '';
const lastReadTime = report.lastReadTime ?? '';
Expand Down Expand Up @@ -5022,7 +5034,7 @@ function hasViolations(reportID: string, transactionViolations: OnyxCollection<T
function shouldReportBeInOptionList({
report,
currentReportId,
isInGSDMode,
isInFocusMode,
betas,
policies,
excludeEmptyChats,
Expand All @@ -5031,14 +5043,14 @@ function shouldReportBeInOptionList({
}: {
report: OnyxEntry<Report>;
currentReportId: string;
isInGSDMode: boolean;
isInFocusMode: boolean;
betas: OnyxEntry<Beta[]>;
policies: OnyxCollection<Policy>;
excludeEmptyChats: boolean;
doesReportHaveViolations: boolean;
includeSelfDM?: boolean;
}) {
const isInDefaultMode = !isInGSDMode;
const isInDefaultMode = !isInFocusMode;
// Exclude reports that have no data because there wouldn't be anything to show in the option item.
// This can happen if data is currently loading from the server or a report is in various stages of being created.
// This can also happen for anyone accessing a public room or archived room for which they don't have access to the underlying policy.
Expand Down Expand Up @@ -5089,8 +5101,8 @@ function shouldReportBeInOptionList({
if (hasDraftComment || requiresAttentionFromCurrentUser(report)) {
return true;
}
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
const isEmptyChat = !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

const isEmptyChat = isEmptyReport(report);
const canHideReport = shouldHideReport(report, currentReportId);

// Include reports if they are pinned
Expand All @@ -5117,7 +5129,7 @@ function shouldReportBeInOptionList({
}

// All unread chats (even archived ones) in GSD mode will be shown. This is because GSD mode is specifically for focusing the user on the most relevant chats, primarily, the unread ones
if (isInGSDMode) {
if (isInFocusMode) {
return isUnread(report) && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE;
}

Expand Down
6 changes: 3 additions & 3 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ function getOrderedReportIDs(
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
): string[] {
const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;
const isInFocusMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInFocusMode;
const allReportsDictValues = Object.values(allReports ?? {});

// Filter out all the reports that shouldn't be displayed
Expand Down Expand Up @@ -103,7 +103,7 @@ function getOrderedReportIDs(
return ReportUtils.shouldReportBeInOptionList({
report,
currentReportId: currentReportId ?? '',
isInGSDMode,
isInFocusMode,
betas,
policies: policies as OnyxCollection<Policy>,
excludeEmptyChats: true,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/UnreadIndicatorUpdater/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default function getUnreadReportsForUnreadIndicator(reports: OnyxCollecti
betas: [],
policies: {},
doesReportHaveViolations: false,
isInGSDMode: false,
isInFocusMode: false,
excludeEmptyChats: false,
}) &&
/**
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2505,7 +2505,7 @@ function navigateToMostRecentReport(currentReport: OnyxEntry<Report>) {
ReportUtils.shouldReportBeInOptionList({
report: sortedReport,
currentReportId: '',
isInGSDMode: false,
isInFocusMode: false,
betas: [],
policies: {},
excludeEmptyChats: true,
Expand Down
4 changes: 2 additions & 2 deletions tests/perf-test/ReportUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ describe('ReportUtils', () => {
test('[ReportUtils] shouldReportBeInOptionList on 1k participant', async () => {
const report = {...createRandomReport(1), participantAccountIDs, type: CONST.REPORT.TYPE.CHAT};
const currentReportId = '2';
const isInGSDMode = true;
const isInFocusMode = true;
const betas = [CONST.BETAS.DEFAULT_ROOMS];
const policies = getMockedPolicies();

await waitForBatchedUpdates();
await measureFunction(() =>
ReportUtils.shouldReportBeInOptionList({report, currentReportId, isInGSDMode, betas, policies, doesReportHaveViolations: false, excludeEmptyChats: false}),
ReportUtils.shouldReportBeInOptionList({report, currentReportId, isInFocusMode, betas, policies, doesReportHaveViolations: false, excludeEmptyChats: false}),
);
});

Expand Down
2 changes: 1 addition & 1 deletion tests/perf-test/SidebarLinks.perf-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const getMockedReportsMap = (length = 100) => {
const reportID = index + 1;
const participants = [1, 2];
const reportKey = `${ONYXKEYS.COLLECTION.REPORT}${reportID}`;
const report = LHNTestUtils.getFakeReport(participants, 1, true);
const report = {...LHNTestUtils.getFakeReport(participants, 1, true), lastMessageText: 'hey'};

return [reportKey, report];
}),
Expand Down
16 changes: 10 additions & 6 deletions tests/unit/SidebarOrderTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,10 +862,10 @@ describe('Sidebar', () => {
it('alphabetizes chats', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

const report1 = LHNTestUtils.getFakeReport([1, 2], 3, true);
const report2 = LHNTestUtils.getFakeReport([3, 4], 2, true);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1, true);
const report4 = LHNTestUtils.getFakeReport([7, 8], 0, true);
const report1 = {...LHNTestUtils.getFakeReport([1, 2], 3, true), lastMessageText: 'test'};
const report2 = {...LHNTestUtils.getFakeReport([3, 4], 2, true), lastMessageText: 'test'};
const report3 = {...LHNTestUtils.getFakeReport([5, 6], 1, true), lastMessageText: 'test'};
const report4 = {...LHNTestUtils.getFakeReport([7, 8], 0, true), lastMessageText: 'test'};

const reportCollectionDataSet: ReportCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
Expand Down Expand Up @@ -919,9 +919,13 @@ describe('Sidebar', () => {
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
lastMessageText: 'test',
};
const report2 = LHNTestUtils.getFakeReport([3, 4], 2, true);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1, true);
const report2 = {
...LHNTestUtils.getFakeReport([3, 4], 2, true),
lastMessageText: 'test',
};
const report3 = {...LHNTestUtils.getFakeReport([5, 6], 1, true), lastMessageText: 'test'};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS];
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/SidebarTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe('Sidebar', () => {
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
lastMessageText: 'test',
};

const action = {
Expand Down Expand Up @@ -93,6 +94,7 @@ describe('Sidebar', () => {
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
lastMessageText: 'test',
};
const action = {
...LHNTestUtils.getFakeReportAction('email1@test.com', 3),
Expand Down
32 changes: 27 additions & 5 deletions tests/unit/UnreadIndicatorUpdaterTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,23 @@ describe('UnreadIndicatorUpdaterTest', () => {
describe('should return correct number of unread reports', () => {
it('given last read time < last visible action created', () => {
const reportsToBeUsed = {
1: {reportID: '1', reportName: 'test', type: CONST.REPORT.TYPE.EXPENSE, lastReadTime: '2023-07-08 07:15:44.030', lastVisibleActionCreated: '2023-08-08 07:15:44.030'},
2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'},
3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK},
1: {
reportID: '1',
reportName: 'test',
type: CONST.REPORT.TYPE.EXPENSE,
lastReadTime: '2023-07-08 07:15:44.030',
lastVisibleActionCreated: '2023-08-08 07:15:44.030',
lastMessageText: 'test',
},
2: {
reportID: '2',
reportName: 'test',
type: CONST.REPORT.TYPE.TASK,
lastReadTime: '2023-02-05 09:12:05.000',
lastVisibleActionCreated: '2023-02-06 07:15:44.030',
lastMessageText: 'test',
},
3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastMessageText: 'test'},
};
expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '3').length).toBe(2);
});
Expand All @@ -31,9 +45,17 @@ describe('UnreadIndicatorUpdaterTest', () => {
notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
lastReadTime: '2023-07-08 07:15:44.030',
lastVisibleActionCreated: '2023-08-08 07:15:44.030',
lastMessageText: 'test',
},
2: {
reportID: '2',
reportName: 'test',
type: CONST.REPORT.TYPE.TASK,
lastReadTime: '2023-02-05 09:12:05.000',
lastVisibleActionCreated: '2023-02-06 07:15:44.030',
lastMessageText: 'test',
},
2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'},
3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK},
3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastMessageText: 'test'},
};
expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '3').length).toBe(1);
});
Expand Down

0 comments on commit ce1fd06

Please sign in to comment.