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.

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

Stop showing empty chats in #focus mode. Do not count them as unread in the global indicator. #30541

Closed
wants to merge 7 commits into from
Closed

Stop showing empty chats in #focus mode. Do not count them as unread in the global indicator. #30541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4d96845
Stop showing empty chats when in #focus mode
marcaaron Oct 27, 2023
5515b58
Also hide empty chats from the global unread count
marcaaron Oct 27, 2023
08f36a7
Run prettier
marcaaron Oct 27, 2023
5224c10
Fix conflict
marcaaron Nov 3, 2023
eaaeba4
Fix tests
marcaaron Nov 4, 2023
ea2deca
run prettier
marcaaron Nov 10, 2023
f34a90f
Merge branch 'main' into marcaaron-hideEmptyChatsInFocusMode
marcaaron Nov 10, 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
25 changes: 17 additions & 8 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3280,6 +3280,15 @@ function shouldHideReport(report, currentReportId) {
return parentReport.reportID !== report.reportID && !isChildReportHasComment;
}

/**
* @param {Object} report
* @returns {boolean}
*/
function isEmptyChat(report) {
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
return !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;
}

/**
* Takes several pieces of data from Onyx and evaluates if a report should be shown in the option list (either when searching
* for reports or the reports shown in the LHN).
Expand Down Expand Up @@ -3335,8 +3344,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
if (report.hasDraft || requiresAttentionFromCurrentUser(report)) {
return true;
}
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
const isEmptyChat = !report.lastMessageText && !report.lastMessageTranslationKey && !lastVisibleMessage.lastMessageText && !lastVisibleMessage.lastMessageTranslationKey;

const canHideReport = shouldHideReport(report, currentReportId);

// Include reports if they are pinned
Expand All @@ -3345,7 +3353,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
}

// Hide only chat threads that haven't been commented on (other threads are actionable)
if (isChatThread(report) && canHideReport && isEmptyChat) {
if (isChatThread(report) && canHideReport && isEmptyChat(report)) {
return false;
}

Expand All @@ -3355,6 +3363,11 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
return true;
}

// Hide chats between users that haven't been commented on from the LNH
if (excludeEmptyChats && isEmptyChat(report) && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && canHideReport) {
return false;
}

// 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) {
return isUnread(report);
Expand All @@ -3365,11 +3378,6 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
return true;
}

// Hide chats between two users that haven't been commented on from the LNH
if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && canHideReport) {
return false;
}

return true;
}

Expand Down Expand Up @@ -4338,6 +4346,7 @@ export {
shouldUseFullTitleToDisplay,
parseReportRouteParams,
getReimbursementQueuedActionMessage,
isEmptyChat,
getPersonalDetailsForAccountID,
getRoom,
};
5 changes: 4 additions & 1 deletion src/libs/UnreadIndicatorUpdater/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (reportsFromOnyx) => {
const unreadReports = _.filter(reportsFromOnyx, (report) => ReportUtils.isUnread(report) && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN);
const unreadReports = _.filter(
reportsFromOnyx,
(report) => ReportUtils.isUnread(report) && !ReportUtils.isEmptyChat(report) && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
);
updateUnread(_.size(unreadReports));
},
});
75 changes: 22 additions & 53 deletions tests/unit/SidebarFilterTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ describe('Sidebar', () => {
const report3 = LHNTestUtils.getFakeReport([5, 6]);
LHNTestUtils.getDefaultRenderedSidebarLinks(report1.reportID);

const reportAction1 = LHNTestUtils.getFakeReportAction();
const reportAction2 = LHNTestUtils.getFakeReportAction();
const reportAction3 = LHNTestUtils.getFakeReportAction();

return (
waitForBatchedUpdates()
// When Onyx is updated to contain that data and the sidebar re-renders
Expand All @@ -385,6 +389,9 @@ describe('Sidebar', () => {
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report1.reportID}`]: {[reportAction1.reportActionID]: reportAction1},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report2.reportID}`]: {[reportAction2.reportActionID]: reportAction1},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report3.reportID}`]: {[reportAction3.reportActionID]: reportAction1},
}),
)

Expand Down Expand Up @@ -490,6 +497,10 @@ describe('Sidebar', () => {
};
LHNTestUtils.getDefaultRenderedSidebarLinks();

const archivedReportAction = LHNTestUtils.getFakeReportAction();
const archivedPolicyRoomReportAction = LHNTestUtils.getFakeReportAction();
const archivedUserCreatedPolicyRoomReportAction = LHNTestUtils.getFakeReportAction();

const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
Expand All @@ -504,6 +515,11 @@ describe('Sidebar', () => {
[`${ONYXKEYS.COLLECTION.REPORT}${archivedReport.reportID}`]: archivedReport,
[`${ONYXKEYS.COLLECTION.REPORT}${archivedPolicyRoomReport.reportID}`]: archivedPolicyRoomReport,
[`${ONYXKEYS.COLLECTION.REPORT}${archivedUserCreatedPolicyRoomReport.reportID}`]: archivedUserCreatedPolicyRoomReport,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${archivedReport.reportID}`]: {[archivedReportAction.reportActionID]: archivedReportAction},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${archivedPolicyRoomReport.reportID}`]: {[archivedPolicyRoomReportAction.reportActionID]: archivedPolicyRoomReportAction},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${archivedUserCreatedPolicyRoomReport.reportID}`]: {
[archivedUserCreatedPolicyRoomReportAction.reportActionID]: archivedUserCreatedPolicyRoomReportAction,
},
}),
)

Expand Down Expand Up @@ -691,58 +707,6 @@ describe('Sidebar', () => {
});

describe('Archived chat', () => {
describe('in default (most recent) mode', () => {
it('is shown regardless if it has comments or not', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a behavior we are changing so I am removing this test. Empty chats should not be showing at all.

LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given an archived report with no comments
const report = {
...LHNTestUtils.getFakeReport(),
lastVisibleActionCreated: '2022-11-22 03:48:27.267',
statusNum: CONST.REPORT.STATUS.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForBatchedUpdates()
// When Onyx is updated to contain that data and the sidebar re-renders
.then(() =>
Onyx.multiSet({
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT,
[ONYXKEYS.BETAS]: betas,
[ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.IS_LOADING_REPORT_DATA]: false,
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
}),
)

// Then the report is rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
const optionRows = screen.queryAllByAccessibilityHint(hintText);
expect(optionRows).toHaveLength(1);
})

// When the report has comments
.then(() =>
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {
lastVisibleActionCreated: DateUtils.getDBTime(),
}),
)

// Then the report is rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
const optionRows = screen.queryAllByAccessibilityHint(hintText);
expect(optionRows).toHaveLength(1);
})
);
});
});

describe('in GSD (focus) mode', () => {
it('is shown when it is unread', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();
Expand All @@ -753,6 +717,7 @@ describe('Sidebar', () => {
statusNum: CONST.REPORT.STATUS.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
};
const reportAction = LHNTestUtils.getFakeReportAction();

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
Expand All @@ -778,9 +743,13 @@ describe('Sidebar', () => {
})

// When the report has a new comment and is now unread
.then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {[reportAction.reportActionID]: reportAction}))
.then(() => {
jest.advanceTimersByTime(10);
return Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {lastVisibleActionCreated: DateUtils.getDBTime()});
return Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {
lastVisibleActionCreated: DateUtils.getDBTime(),
lastMessageText: reportAction.message[0].text,
});
})

// Then the report is rendered in the LHN
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/SidebarOrderTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,10 @@ describe('Sidebar', () => {
const report3 = LHNTestUtils.getFakeReport([5, 6], 1, true);
const report4 = LHNTestUtils.getFakeReport([7, 8], 0, true);

const reportAction1 = LHNTestUtils.getFakeReportAction();
const reportAction2 = LHNTestUtils.getFakeReportAction();
const reportAction3 = LHNTestUtils.getFakeReportAction();
const reportAction4 = LHNTestUtils.getFakeReportAction();
return (
waitForBatchedUpdates()
// Given the sidebar is rendered in #focus mode (hides read chats)
Expand All @@ -625,6 +629,10 @@ describe('Sidebar', () => {
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report1.reportID}`]: {[reportAction1.reportActionID]: reportAction1},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report2.reportID}`]: {[reportAction2.reportActionID]: reportAction2},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report3.reportID}`]: {[reportAction3.reportActionID]: reportAction3},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report4.reportID}`]: {[reportAction4.reportActionID]: reportAction4},
}),
)

Expand Down Expand Up @@ -664,6 +672,9 @@ describe('Sidebar', () => {
};
const report2 = LHNTestUtils.getFakeReport([3, 4], 2, true);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1, true);
const reportAction1 = LHNTestUtils.getFakeReportAction();
const reportAction2 = LHNTestUtils.getFakeReportAction();
const reportAction3 = LHNTestUtils.getFakeReportAction();

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
Expand All @@ -680,6 +691,9 @@ describe('Sidebar', () => {
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report1.reportID}`]: {[reportAction1.reportActionID]: reportAction1},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report2.reportID}`]: {[reportAction2.reportActionID]: reportAction2},
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report3.reportID}`]: {[reportAction3.reportActionID]: reportAction3},
}),
)

Expand Down
2 changes: 1 addition & 1 deletion tests/utils/LHNTestUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ function getFakeReportAction(actor = '[email protected]', millisecondsInThePast =
actor,
actorAccountID: 1,
reportActionID: `${++lastFakeReportActionID}`,
actionName: CONST.REPORT.ACTIONS.TYPE.CREATED,
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
shouldShow: true,
timestamp,
reportActionTimestamp: timestamp,
Expand Down
Loading