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

Only show reports without comments if they are non-archived policy rooms #11634

Merged
merged 6 commits into from
Oct 6, 2022
Merged

Only show reports without comments if they are non-archived policy rooms #11634

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8203e1a
Only show reports without comments if they are non-archived policy rooms
johnmlee101 Oct 6, 2022
327a84e
Fix some tests
johnmlee101 Oct 6, 2022
e8db2ab
Fix more tests
johnmlee101 Oct 6, 2022
e9ed223
Remove unnecessary comment from LHNFilterTest
johnmlee101 Oct 6, 2022
9a8e880
Merge branch 'main' into john-fix-no-comment-reports
johnmlee101 Oct 6, 2022
e58df5b
Fix tests with merged main
johnmlee101 Oct 6, 2022
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
4 changes: 2 additions & 2 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,9 +929,9 @@ function shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, curr
}

// Exclude reports that don't have any comments
// Archived rooms or user created policy rooms are OK to show when the don't have any comments
// User created policy rooms are OK to show when the don't have any comments, only if they aren't archived.
const hasNoComments = report.lastMessageTimestamp === 0;
if (hasNoComments && (isArchivedRoom(report) || isUserCreatedPolicyRoom(report))) {
if (hasNoComments && (isArchivedRoom(report) || !isUserCreatedPolicyRoom(report))) {
return false;
}

Expand Down
15 changes: 7 additions & 8 deletions tests/unit/LHNFilterTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,16 @@ describe('Sidebar', () => {

// Given these combinations of booleans which result in the report being filtered out (not shown).
const booleansWhichRemovesInactiveReport = [
// isUserCreatedPolicyRoom
JSON.stringify([false, false, true, false, false, false, false]),
JSON.stringify([false, false, false, false, false, false, false]),

// isUserCreatedPolicyRoom, isUnread
Copy link
Contributor Author

Choose a reason for hiding this comment

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

isUserCreatedPolicyRoom should always show, regardless of if there's no message if they aren't archived, so this fixes that

JSON.stringify([false, false, true, false, true, false, false]),
// isUnread
JSON.stringify([false, false, false, false, true, false, false]),

// isUserCreatedPolicyRoom, hasAddWorkspaceError
JSON.stringify([false, false, true, true, false, false, false]),
// hasAddWorkspaceError, isUnread
JSON.stringify([false, false, false, true, true, false, false]),

// isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread
JSON.stringify([false, false, true, true, true, false, false]),
// hasAddWorkspaceError
JSON.stringify([false, false, false, true, false, false, false]),

// isArchived
JSON.stringify([false, true, false, false, false, false, false]),
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/OptionsListUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ describe('OptionsListUtils', () => {
// Then the 2 personalDetails that don't have reports should be returned
expect(results.personalDetails.length).toBe(2);

// Then all of the reports should be shown, including the one that has no message on them.
expect(results.recentReports.length).toBe(_.size(REPORTS));
// Then all of the reports should be shown EXCEPT the archived room, including the one that has no message on them.
expect(results.recentReports.length).toBe(_.size(REPORTS) - 1);

// When we filter again but provide a searchValue
results = OptionsListUtils.getSearchOptions(REPORTS, PERSONAL_DETAILS, 'spider');
Expand Down