Skip to content

Commit

Permalink
Merge pull request Expensify#19321 from Expensify/paulogasparsv-exclu…
Browse files Browse the repository at this point in the history 
…de-empty-chats-from-lnh

Exclude empty chats (with no ADDCOMMENT actions or message drafts) from LNH
  • Loading branch information
@luacmartins
luacmartins authored Aug 13, 2023
2 parents ed2c287 + ef0b220 commit a03a21d
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 13 deletions.
12 changes: 10 additions & 2 deletions src/libs/ReportUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2341,9 +2341,10 @@ function canAccessReport(report, policies, betas, allReportActions) {
* @param {String[]} betas
* @param {Object} policies
* @param {Object} allReportActions
* @param {Boolean} excludeEmptyChats
* @returns {boolean}
*/
function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouReports, betas, policies, allReportActions) {
function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouReports, betas, policies, allReportActions, excludeEmptyChats = false) {
const isInDefaultMode = !isInGSDMode;

// Exclude reports that have no data because there wouldn't be anything to show in the option item.
Expand Down Expand Up @@ -2374,8 +2375,10 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouRep
return true;
}

const isEmptyChat = !ReportActionsUtils.getLastVisibleMessage(report.reportID).lastMessageText;

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

Expand Down Expand Up @@ -2405,6 +2408,11 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouRep
return false;
}

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

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/SidebarUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p

// Filter out all the reports that shouldn't be displayed
const reportsToDisplay = _.filter(allReportsDict, (report) =>
ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, allReportsDict, betas, policies, allReportActions),
ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, allReportsDict, betas, policies, allReportActions, true),
);

if (_.isEmpty(reportsToDisplay)) {
Expand Down
58 changes: 57 additions & 1 deletion tests/unit/SidebarFilterTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import wrapOnyxWithWaitForPromisesToResolve from '../utils/wrapOnyxWithWaitForPr
import CONST from '../../src/CONST';
import DateUtils from '../../src/libs/DateUtils';
import * as Localize from '../../src/libs/Localize';
import * as Report from '../../src/libs/actions/Report';

// Be sure to include the mocked permissions library or else the beta tests won't work
jest.mock('../../src/libs/Permissions');
Expand Down Expand Up @@ -72,7 +73,59 @@ describe('Sidebar', () => {
);
});

it('includes or excludes policy expense chats depending on the beta', () => {
it('excludes an empty chat report', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given a new report
const report = LHNTestUtils.getFakeReport(['emptychat+1@test.com', 'emptychat+2@test.com'], 0);

return (
waitForPromisesToResolve()
// When Onyx is updated to contain that report
.then(() =>
Onyx.multiSet({
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
}),
)

// Then no reports are rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames).toHaveLength(0);
})
);
});

it('includes an empty chat report if it has a draft', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given a new report
const report = {
...LHNTestUtils.getFakeReport([1, 2], 0),
hasDraft: true,
};

return (
waitForPromisesToResolve()
// When Onyx is updated to contain that report
.then(() =>
Onyx.multiSet({
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
[ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails,
}),
)

// Then no reports are rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames).toHaveLength(1);
})
);
});

it('includes or excludes policy expensechats depending on the beta', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given a policy expense report
Expand All @@ -94,6 +147,9 @@ describe('Sidebar', () => {
}),
)

// When the report has at least one ADDCOMMENT action to be rendered in the LNH
.then(() => Report.addComment(report.reportID, 'Hi, this is a comment'))

// Then no reports are rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
Expand Down
47 changes: 38 additions & 9 deletions tests/unit/SidebarOrderTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as LHNTestUtils from '../utils/LHNTestUtils';
import CONST from '../../src/CONST';
import DateUtils from '../../src/libs/DateUtils';
import * as Localize from '../../src/libs/Localize';
import * as Report from '../../src/libs/actions/Report';

// Be sure to include the mocked Permissions and Expensicons libraries or else the beta tests won't work
jest.mock('../../src/libs/Permissions');
Expand Down Expand Up @@ -109,15 +110,14 @@ describe('Sidebar', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given three unread reports in the recently updated order of 3, 2, 1
const report1 = {
...LHNTestUtils.getFakeReport([1, 2], 3),
};
const report2 = {
...LHNTestUtils.getFakeReport([3, 4], 2),
};
const report3 = {
...LHNTestUtils.getFakeReport([5, 6], 1),
};
const report1 = LHNTestUtils.getFakeReport([1, 2], 3);
const report2 = LHNTestUtils.getFakeReport([3, 4], 2);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1);

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

return (
waitForPromisesToResolve()
Expand All @@ -137,6 +137,7 @@ describe('Sidebar', () => {
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);

expect(displayNames).toHaveLength(3);
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Three, Four');
Expand All @@ -155,8 +156,15 @@ describe('Sidebar', () => {
};
const report2 = LHNTestUtils.getFakeReport([3, 4], 2);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1);

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

const currentReportId = report1.reportID;
LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId);

return (
waitForPromisesToResolve()
// When Onyx is updated with the data and the sidebar re-renders
Expand Down Expand Up @@ -195,6 +203,11 @@ describe('Sidebar', () => {
const report2 = LHNTestUtils.getFakeReport([3, 4], 2);
const report3 = LHNTestUtils.getFakeReport([5, 6], 1);

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

return (
waitForPromisesToResolve()
// When Onyx is updated with the data and the sidebar re-renders
Expand Down Expand Up @@ -239,6 +252,12 @@ describe('Sidebar', () => {
hasDraft: true,
};
const report3 = LHNTestUtils.getFakeReport([5, 6], 1);

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

const currentReportId = report2.reportID;
LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId);

Expand Down Expand Up @@ -550,6 +569,11 @@ describe('Sidebar', () => {
const report2 = LHNTestUtils.getFakeReport([3, 4]);
const report3 = LHNTestUtils.getFakeReport([5, 6]);

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
Expand Down Expand Up @@ -780,6 +804,11 @@ describe('Sidebar', () => {
lastVisibleActionCreated,
};

// Each report has at least one ADDCOMMENT action so should be rendered in the LNH
Report.addComment(report1.reportID, 'Hi, this is a comment');
Report.addComment(report2.reportID, 'Hi, this is a comment');
Report.addComment(report3.reportID, 'Hi, this is a comment');

LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return (
waitForPromisesToResolve()
Expand Down

0 comments on commit a03a21d

Please sign in to comment.