-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
Feat: Update how GBR is determined for IOU/expense reports #30655
Changes from 26 commits
cb3f3d0
ddba0ec
4fca049
7523bd6
b8b6752
f6b79e6
f4cb009
63f6bd8
647efb8
de9e121
2bb9228
bfb06ae
a9c9ef2
0ce0891
dd4a7bb
c79c9f1
c497922
8e1ad6d
1195a83
3e938c6
056b7d5
943bc80
d6eff10
fb9b40d
cc74887
b35d1da
efcefdb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1324,61 +1324,51 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { | |
} | ||
|
||
/** | ||
* 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) | ||
* Checks if a report is an open task report assigned to current user. | ||
* | ||
* @param {Object} report (chatReport or iouReport) | ||
* @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 = {}) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this one could also be an "option" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idk though maybe we can do a follow up to address as long as all of these other methods are referencing fields that will exist on either a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like for the most part all the things we need are there 🤷 |
||
return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); | ||
} | ||
|
||
/** | ||
* 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 isWaitingForIOUActionFromCurrentUser(report) { | ||
if (!report) { | ||
function requiresAttentionFromCurrentUser(option, parentReportAction = {}) { | ||
if (!option) { | ||
return false; | ||
} | ||
|
||
if (isArchivedRoom(getReport(report.parentReportID))) { | ||
if (isArchivedRoom(getReport(option.parentReportID))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @puneetlath did you think about the case of archived options too? see #30824 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah, I'm not sure why we did it this way. Thanks for handling! |
||
return false; | ||
} | ||
|
||
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 (option.isUnreadWithMention) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Crap, no it will not. Updated to check. Alternate would be to add the |
||
return true; | ||
} | ||
|
||
// 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) { | ||
if (isWaitingForAssigneeToCompleteTask(option, parentReportAction)) { | ||
return true; | ||
} | ||
|
||
// Money request waiting for current user to Pay (from expense or iou report) | ||
if (report.hasOutstandingIOU && report.ownerAccountID && (report.ownerAccountID !== currentUserAccountID || currentUserAccountID === report.managerID)) { | ||
// Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user | ||
if (option.hasOutstandingChildRequest) { | ||
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 | ||
* | ||
|
@@ -3296,8 +3286,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, having an outstanding IOU, or being assigned to an open task. | ||
if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report) || isWaitingForTaskCompleteFromAssignee(report)) { | ||
// 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)) { | ||
return true; | ||
} | ||
const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); | ||
|
@@ -4155,7 +4145,7 @@ export { | |
isCurrentUserTheOnlyParticipant, | ||
hasAutomatedExpensifyAccountIDs, | ||
hasExpensifyGuidesEmails, | ||
isWaitingForIOUActionFromCurrentUser, | ||
requiresAttentionFromCurrentUser, | ||
isIOUOwnedByCurrentUser, | ||
getMoneyRequestReimbursableTotal, | ||
getMoneyRequestSpendBreakdown, | ||
|
@@ -4275,7 +4265,7 @@ export { | |
hasNonReimbursableTransactions, | ||
hasMissingSmartscanFields, | ||
getIOUReportActionDisplayMessage, | ||
isWaitingForTaskCompleteFromAssignee, | ||
isWaitingForAssigneeToCompleteTask, | ||
isGroupChat, | ||
isReportDraft, | ||
shouldUseFullTitleToDisplay, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we are using either the
report
(in this case, "option") or theparentReportAction
to determine if the task is canceled. Why do we only pass theparentReportAction
here?