Skip to content

Commit

Permalink
fix conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
marcaaron committed Oct 4, 2022
1 parent 4cbffa5 commit 5dac271
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 66 deletions.
26 changes: 14 additions & 12 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as ReportUtils from './ReportUtils';
import * as Localize from './Localize';
import Permissions from './Permissions';
import * as CollectionUtils from './CollectionUtils';
import Navigation from './Navigation/Navigation';

/**
* OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can
Expand All @@ -22,12 +23,6 @@ Onyx.connect({
callback: val => currentUserLogin = val && val.email,
});

let currentlyViewedReportID;
Onyx.connect({
key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
callback: val => currentlyViewedReportID = val,
});

let priorityMode;
Onyx.connect({
key: ONYXKEYS.NVP_PRIORITY_MODE,
Expand Down Expand Up @@ -428,12 +423,11 @@ function isCurrentUser(userDetails) {
*
* @param {Object} reports
* @param {Object} personalDetails
* @param {String} activeReportID
* @param {Object} options
* @returns {Object}
* @private
*/
function getOptions(reports, personalDetails, activeReportID, {
function getOptions(reports, personalDetails, {
reportActions = {},
betas = [],
selectedOptions = [],
Expand All @@ -457,7 +451,15 @@ function getOptions(reports, personalDetails, activeReportID, {
const reportMapForLogins = {};

// Filter out all the reports that shouldn't be displayed
const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies));
const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(
report,
Navigation.getReportIDFromRoute(),
isInGSDMode,
currentUserLogin,
iouReports,
betas,
policies,
));

// Sorting the reports works like this:
// - Order everything by the last message timestamp (descending)
Expand Down Expand Up @@ -639,7 +641,7 @@ function getSearchOptions(
searchValue = '',
betas,
) {
return getOptions(reports, personalDetails, 0, {
return getOptions(reports, personalDetails, {
betas,
searchValue: searchValue.trim(),
includeRecentReports: true,
Expand Down Expand Up @@ -706,7 +708,7 @@ function getNewChatOptions(
selectedOptions = [],
excludeLogins = [],
) {
return getOptions(reports, personalDetails, 0, {
return getOptions(reports, personalDetails, {
betas,
searchValue: searchValue.trim(),
selectedOptions,
Expand All @@ -733,7 +735,7 @@ function getMemberInviteOptions(
searchValue = '',
excludeLogins = [],
) {
return getOptions([], personalDetails, 0, {
return getOptions([], personalDetails, {
betas,
searchValue: searchValue.trim(),
excludeDefaultRooms: true,
Expand Down
6 changes: 3 additions & 3 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -890,15 +890,15 @@ function hasOutstandingIOU(report, currentUserLogin, iouReports) {
* filter out the majority of reports before filtering out very specific minority of reports.
*
* @param {Object} report
* @param {String} currentlyViewedReportID
* @param {String} reportIDFromRoute
* @param {Boolean} isInGSDMode
* @param {String} currentUserLogin
* @param {Object} iouReports
* @param {String[]} betas
* @param {Object} policies
* @returns {boolean}
*/
function shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies) {
function shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies) {
// 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.
if (!report || !report.reportID || !report.participants || _.isEmpty(report.participants)) {
Expand All @@ -908,7 +908,7 @@ function shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode
// Include the currently viewed report. If we excluded the currently viewed report, then there
// would be no way to highlight it in the options list and it would be confusing to users because they lose
// a sense of context.
if (report.reportID === currentlyViewedReportID) {
if (report.reportID === reportIDFromRoute) {
return true;
}

Expand Down
15 changes: 5 additions & 10 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as CollectionUtils from './CollectionUtils';
// keys that are connected to SidebarLinks withOnyx(). If there was a key missing from SidebarLinks and it's data was updated
// for that key, then there would be no re-render and the options wouldn't reflect the new data because SidebarUtils.getOrderedReportIDs() wouldn't be triggered.
// There are a couple of keys here which are OK to have stale data. iouReports for example, doesn't need to exist in withOnyx() because
// when IOUs change, it also triggers a change on the reports collection. Having redudant subscriptions causes more re-renders which should be avoided.
// when IOUs change, it also triggers a change on the reports collection. Having redundant subscriptions causes more re-renders which should be avoided.
// Session also can remain stale because the only way for the current user to change is to sign out and sign in, which would clear out all the Onyx
// data anyway and cause SidebarLinks to rerender.

Expand All @@ -29,12 +29,6 @@ Onyx.connect({
callback: val => personalDetails = val,
});

let currentlyViewedReportID;
Onyx.connect({
key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
callback: val => currentlyViewedReportID = val,
});

let priorityMode;
Onyx.connect({
key: ONYXKEYS.NVP_PRIORITY_MODE,
Expand Down Expand Up @@ -88,9 +82,10 @@ Onyx.connect({
});

/**
* @param {String} reportIDFromRoute
* @returns {String[]} An array of reportIDs sorted in the proper order
*/
function getOrderedReportIDs() {
function getOrderedReportIDs(reportIDFromRoute) {
let recentReportOptions = [];
const pinnedReportOptions = [];
const iouDebtReportOptions = [];
Expand All @@ -100,7 +95,7 @@ function getOrderedReportIDs() {
const isInDefaultMode = !isInGSDMode;

// Filter out all the reports that shouldn't be displayed
const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies));
const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies));

// Get all the display names for our reports in an easy to access property so we don't have to keep
// re-running the logic
Expand Down Expand Up @@ -142,7 +137,7 @@ function getOrderedReportIDs() {

// If the active report has a draft, we do not put it in the group of draft reports because we want it to maintain it's current position. Otherwise the report's position
// jumps around in the LHN and it's kind of confusing to the user to see the LHN reorder when they start typing a comment on a report.
} else if (report.hasDraft && report.reportID !== currentlyViewedReportID) {
} else if (report.hasDraft && report.reportID !== reportIDFromRoute) {
draftReportOptions.push(report);
} else {
recentReportOptions.push(report);
Expand Down
13 changes: 6 additions & 7 deletions tests/unit/LHNFilterTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ jest.mock('../../src/libs/Permissions');

const ONYXKEYS = {
PERSONAL_DETAILS: 'personalDetails',
CURRENTLY_VIEWED_REPORTID: 'currentlyViewedReportID',
NVP_PRIORITY_MODE: 'nvp_priorityMode',
SESSION: 'session',
BETAS: 'betas',
Expand Down Expand Up @@ -306,15 +305,14 @@ describe('Sidebar', () => {
...LHNTestUtils.getAdvancedFakeReport(...boolArr),
policyID: policy.policyID,
};
const sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks();
const sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks(report1.reportID.toString());

return waitForPromisesToResolve()

// When Onyx is updated to contain that data and the sidebar re-renders
.then(() => Onyx.multiSet({
[ONYXKEYS.BETAS]: betas,
[ONYXKEYS.PERSONAL_DETAILS]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.CURRENTLY_VIEWED_REPORTID]: report1.reportID.toString(),
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.POLICY}${policy.policyID}`]: policy,
Expand All @@ -340,8 +338,6 @@ describe('Sidebar', () => {

describe('in #focus mode', () => {
it('hides unread chats', () => {
const sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given the sidebar is rendered in #focus mode (hides read chats)
// with report 1 and 2 having unread actions
const report1 = {
Expand All @@ -353,14 +349,14 @@ describe('Sidebar', () => {
lastReadSequenceNumber: LHNTestUtils.TEST_MAX_SEQUENCE_NUMBER - 1,
};
const report3 = LHNTestUtils.getFakeReport(['[email protected]', '[email protected]']);
let sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks(report1.reportID.toString());

return waitForPromisesToResolve()

// When Onyx is updated to contain that data and the sidebar re-renders
.then(() => Onyx.multiSet({
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD,
[ONYXKEYS.PERSONAL_DETAILS]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.CURRENTLY_VIEWED_REPORTID]: report1.reportID.toString(),
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
Expand Down Expand Up @@ -391,7 +387,10 @@ describe('Sidebar', () => {
})

// When report 2 becomes the active report
.then(() => Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, report2.reportID.toString()))
.then(() => {
sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks(report2.reportID.toString());
return waitForPromisesToResolve();
})

// Then report 1 should now disappear
.then(() => {
Expand Down
Loading

0 comments on commit 5dac271

Please sign in to comment.