Skip to content

Commit

Permalink
Revert "Merge pull request #30655 from Expensify/feat-29595-Update-GB…
Browse files Browse the repository at this point in the history
…R-is-determined"

This reverts commit 3634718, reversing
changes made to 342a2c4.
  • Loading branch information
AndrewGable committed Nov 3, 2023
1 parent 8545ef4 commit d37bb2c
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 102 deletions.
3 changes: 2 additions & 1 deletion src/components/LHNOptionsList/OptionRowLHN.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ function OptionRowLHN(props) {

const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT;
const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.requiresAttentionFromCurrentUser(optionItem, optionItem.parentReportAction);
const shouldShowGreenDotIndicator =
!hasBrickError && (optionItem.isUnreadWithMention || optionItem.isWaitingForTaskCompleteFromAssignee || ReportUtils.isWaitingForIOUActionFromCurrentUser(optionItem));

/**
* Show the ReportActionContextMenu modal popover.
Expand Down
98 changes: 56 additions & 42 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1375,70 +1375,69 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) {
}

/**
* Checks if a report is an open task report assigned to current user.
* Determines if a report has an IOU that is waiting for an action from the current user (either Pay or Add a credit bank account)
*
* @param {Object} report
* @param {Object} [parentReportAction] - The parent report action of the report (Used to check if the task has been canceled)
* @returns {Boolean}
*/
function isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) {
return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction);
}

/**
* @param {Object} report
* @returns {Boolean}
* @param {Object} report (chatReport or iouReport)
* @returns {boolean}
*/
function isUnreadWithMention(report) {
function isWaitingForIOUActionFromCurrentUser(report) {
if (!report) {
return false;
}

// lastMentionedTime and lastReadTime are both datetime strings and can be compared directly
const lastMentionedTime = report.lastMentionedTime || '';
const lastReadTime = report.lastReadTime || '';
return lastReadTime < lastMentionedTime;
}

/**
* Determines if the option requires action from the current user. This can happen when it:
- is unread and the user was mentioned in one of the unread comments
- is for an outstanding task waiting on the user
- has an outstanding child money request that is waiting for an action from the current user (e.g. pay, approve, add bank account)
*
* @param {Object} option (report or optionItem)
* @param {Object} parentReportAction (the report action the current report is a thread of)
* @returns {boolean}
*/
function requiresAttentionFromCurrentUser(option, parentReportAction = {}) {
if (!option) {
if (isArchivedRoom(getReport(report.parentReportID))) {
return false;
}

if (isArchivedRoom(option)) {
if (isArchivedRoom(report)) {
return false;
}

if (isArchivedRoom(getReport(option.parentReportID))) {
if (isArchivedRoom(getReport(report.parentReportID))) {
return false;
}

if (option.isUnreadWithMention || isUnreadWithMention(option)) {
return true;
const policy = getPolicy(report.policyID);
if (policy.type === CONST.POLICY.TYPE.CORPORATE) {
// If the report is already settled, there's no action required from any user.
if (isSettled(report.reportID)) {
return false;
}

// Report is pending approval and the current user is the manager
if (isReportManager(report) && !isReportApproved(report)) {
return true;
}

// Current user is an admin and the report has been approved but not settled yet
return policy.role === CONST.POLICY.ROLE.ADMIN && isReportApproved(report);
}

if (isWaitingForAssigneeToCompleteTask(option, parentReportAction)) {
// Money request waiting for current user to add their credit bank account
// hasOutstandingIOU will be false if the user paid, but isWaitingOnBankAccount will be true if user don't have a wallet or bank account setup
if (!report.hasOutstandingIOU && report.isWaitingOnBankAccount && report.ownerAccountID === currentUserAccountID) {
return true;
}

// Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user
if (option.hasOutstandingChildRequest) {
// Money request waiting for current user to Pay (from expense or iou report)
if (report.hasOutstandingIOU && report.ownerAccountID && (report.ownerAccountID !== currentUserAccountID || currentUserAccountID === report.managerID)) {
return true;
}

return false;
}

/**
* Checks if a report is an open task report assigned to current user.
*
* @param {Object} report
* @param {Object} parentReportAction - The parent report action of the report (Used to check if the task has been canceled)
* @returns {Boolean}
*/
function isWaitingForTaskCompleteFromAssignee(report, parentReportAction = {}) {
return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction);
}

/**
* Returns number of transactions that are nonReimbursable
*
Expand Down Expand Up @@ -3177,6 +3176,21 @@ function isUnread(report) {
return lastReadTime < lastVisibleActionCreated;
}

/**
* @param {Object} report
* @returns {Boolean}
*/
function isUnreadWithMention(report) {
if (!report) {
return false;
}

// lastMentionedTime and lastReadTime are both datetime strings and can be compared directly
const lastMentionedTime = report.lastMentionedTime || '';
const lastReadTime = report.lastReadTime || '';
return lastReadTime < lastMentionedTime;
}

/**
* @param {Object} report
* @param {Object} allReportsDict
Expand Down Expand Up @@ -3320,8 +3334,8 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
return true;
}

// Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing.
if (report.hasDraft || requiresAttentionFromCurrentUser(report)) {
// Include reports that are relevant to the user in any view mode. Criteria include having a draft, having an outstanding IOU, or being assigned to an open task.
if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report) || isWaitingForTaskCompleteFromAssignee(report)) {
return true;
}
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID);
Expand Down Expand Up @@ -4188,7 +4202,7 @@ export {
isCurrentUserTheOnlyParticipant,
hasAutomatedExpensifyAccountIDs,
hasExpensifyGuidesEmails,
requiresAttentionFromCurrentUser,
isWaitingForIOUActionFromCurrentUser,
isIOUOwnedByCurrentUser,
getMoneyRequestReimbursableTotal,
getMoneyRequestSpendBreakdown,
Expand Down Expand Up @@ -4308,7 +4322,7 @@ export {
hasNonReimbursableTransactions,
hasMissingSmartscanFields,
getIOUReportActionDisplayMessage,
isWaitingForAssigneeToCompleteTask,
isWaitingForTaskCompleteFromAssignee,
isGroupChat,
isReportDraft,
shouldUseFullTitleToDisplay,
Expand Down
39 changes: 23 additions & 16 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,23 +168,26 @@ function getOrderedReportIDs(
report.iouReportAmount = ReportUtils.getMoneyRequestReimbursableTotal(report, allReports);
});

// The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned/GBR - Always sorted by reportDisplayName
// 2. Drafts - Always sorted by reportDisplayName
// 3. Non-archived reports and settled IOUs
// The LHN is split into five distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned - Always sorted by reportDisplayName
// 2. Outstanding IOUs - Always sorted by iouReportAmount with the largest amounts at the top of the group
// 3. Drafts - Always sorted by reportDisplayName
// 4. Non-archived reports and settled IOUs
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 4. Archived reports
// 5. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
const pinnedAndGBRReports: Report[] = [];
const pinnedReports: Report[] = [];
const outstandingIOUReports: Report[] = [];
const draftReports: Report[] = [];
const nonArchivedReports: Report[] = [];
const archivedReports: Report[] = [];
reportsToDisplay.forEach((report) => {
const isPinned = report.isPinned ?? false;
if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report)) {
pinnedAndGBRReports.push(report);
if (report.isPinned) {
pinnedReports.push(report);
} else if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report)) {
outstandingIOUReports.push(report);
} else if (report.hasDraft) {
draftReports.push(report);
} else if (ReportUtils.isArchivedRoom(report)) {
Expand All @@ -195,7 +198,12 @@ function getOrderedReportIDs(
});

// Sort each group of reports accordingly
pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0));
pinnedReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0));
outstandingIOUReports.sort((a, b) => {
const compareAmounts = a?.iouReportAmount && b?.iouReportAmount ? b.iouReportAmount - a.iouReportAmount : 0;
const compareDisplayNames = a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0;
return compareAmounts || compareDisplayNames;
});
draftReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0));

if (isInDefaultMode) {
Expand All @@ -213,7 +221,7 @@ function getOrderedReportIDs(

// Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
const LHNReports = [...pinnedReports, ...outstandingIOUReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
setWithLimit(reportIDsCache, cachedReportsKey, LHNReports);
return LHNReports;
}
Expand Down Expand Up @@ -246,7 +254,6 @@ type OptionData = {
searchText?: string | null;
isPinned?: boolean | null;
hasOutstandingIOU?: boolean | null;
hasOutstandingChildRequest?: boolean | null;
iouReportID?: string | null;
isIOUReportOwner?: boolean | null;
iouReportAmount?: number | null;
Expand All @@ -260,8 +267,8 @@ type OptionData = {
isAllowedToComment?: boolean | null;
isThread?: boolean | null;
isTaskReport?: boolean | null;
isWaitingForTaskCompleteFromAssignee?: boolean | null;
parentReportID?: string | null;
parentReportAction?: ReportAction;
notificationPreference?: string | number | null;
displayNamesWithTooltips?: DisplayNamesWithTooltip[] | null;
chatType?: ValueOf<typeof CONST.REPORT.CHAT_TYPE> | null;
Expand Down Expand Up @@ -331,7 +338,6 @@ function getOptionData(
searchText: null,
isPinned: false,
hasOutstandingIOU: false,
hasOutstandingChildRequest: false,
iouReportID: null,
isIOUReportOwner: null,
iouReportAmount: 0,
Expand All @@ -351,7 +357,9 @@ function getOptionData(
result.isThread = ReportUtils.isChatThread(report);
result.isChatRoom = ReportUtils.isChatRoom(report);
result.isTaskReport = ReportUtils.isTaskReport(report);
result.parentReportAction = parentReportAction;
if (result.isTaskReport) {
result.isWaitingForTaskCompleteFromAssignee = ReportUtils.isWaitingForTaskCompleteFromAssignee(report, parentReportAction);
}
result.isArchivedRoom = ReportUtils.isArchivedRoom(report);
result.isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
result.isExpenseRequest = ReportUtils.isExpenseRequest(report);
Expand All @@ -374,7 +382,6 @@ function getOptionData(
result.keyForList = String(report.reportID);
result.tooltipText = ReportUtils.getReportParticipantsTitle(report.participantAccountIDs ?? []);
result.hasOutstandingIOU = report.hasOutstandingIOU;
result.hasOutstandingChildRequest = report.hasOutstandingChildRequest;
result.parentReportID = report.parentReportID ?? null;
result.isWaitingOnBankAccount = report.isWaitingOnBankAccount;
result.notificationPreference = report.notificationPreference ?? null;
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/sidebar/SidebarLinksData.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ const chatReportSelector = (report) =>
total: report.total,
nonReimbursableTotal: report.nonReimbursableTotal,
hasOutstandingIOU: report.hasOutstandingIOU,
hasOutstandingChildRequest: report.hasOutstandingChildRequest,
isWaitingOnBankAccount: report.isWaitingOnBankAccount,
statusNum: report.statusNum,
stateNum: report.stateNum,
Expand Down
3 changes: 0 additions & 3 deletions src/types/onyx/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ type Report = {
/** Whether there is an outstanding amount in IOU */
hasOutstandingIOU?: boolean;

/** Whether the report has a child that is an outstanding money request that is awaiting action from the current user */
hasOutstandingChildRequest?: boolean;

/** List of icons for report participants */
icons?: OnyxCommon.Icon[];

Expand Down
38 changes: 10 additions & 28 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,17 @@ describe('ReportUtils', () => {
});
});

describe('requiresAttentionFromCurrentUser', () => {
describe('isWaitingForIOUActionFromCurrentUser', () => {
it('returns false when there is no report', () => {
expect(ReportUtils.requiresAttentionFromCurrentUser()).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser()).toBe(false);
});
it('returns false when the matched IOU report does not have an owner accountID', () => {
const report = {
...LHNTestUtils.getFakeReport(),
ownerAccountID: undefined,
hasOutstandingIOU: true,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
it('returns false when the linked iou report has an oustanding IOU', () => {
const report = {
Expand All @@ -268,17 +268,17 @@ describe('ReportUtils', () => {
ownerAccountID: 99,
hasOutstandingIOU: true,
}).then(() => {
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns false when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => {
it('returns true when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => {
const report = {
...LHNTestUtils.getFakeReport(),
hasOutstandingIOU: false,
ownerAccountID: currentUserAccountID,
isWaitingOnBankAccount: true,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
it('returns false when the report has outstanding IOU and is not waiting for a bank account and the logged user is the report owner', () => {
const report = {
Expand All @@ -287,7 +287,7 @@ describe('ReportUtils', () => {
ownerAccountID: currentUserAccountID,
isWaitingOnBankAccount: false,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
it('returns false when the report has no oustanding IOU but is waiting for a bank account and the logged user is not the report owner', () => {
const report = {
Expand All @@ -296,34 +296,16 @@ describe('ReportUtils', () => {
ownerAccountID: 97,
isWaitingOnBankAccount: true,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
it('returns true when the report has an unread mention', () => {
const report = {
...LHNTestUtils.getFakeReport(),
isUnreadWithMention: true,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true);
});
it('returns true when the report is an outstanding task', () => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.TASK,
managerID: currentUserAccountID,
stateNum: CONST.REPORT.STATE_NUM.OPEN,
statusNum: CONST.REPORT.STATUS.OPEN,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true);
});
it('returns true when the report has oustanding child request', () => {
it('returns true when the report has oustanding IOU', () => {
const report = {
...LHNTestUtils.getFakeReport(),
ownerAccountID: 99,
hasOutstandingIOU: true,
hasOutstandingChildRequest: true,
isWaitingOnBankAccount: false,
};
expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
});

Expand Down
Loading

0 comments on commit d37bb2c

Please sign in to comment.