-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Held requests] Clean up the hold/unhold logic #45151
Changes from all commits
948e1f7
e43ffe0
1384fe1
94ae4d3
ade4976
c8dce56
c4289b4
1259a4a
dbf2421
1238c05
7dba3b9
6730a11
e5c06b8
689662e
4e91940
9c97c2c
522447f
3521502
9bbd7a7
07c607d
70691da
eee69cb
03e5b25
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 |
---|---|---|
|
@@ -14,6 +14,7 @@ import useEnvironment from '@hooks/useEnvironment'; | |
import useKeyboardShortcut from '@hooks/useKeyboardShortcut'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import useNetwork from '@hooks/useNetwork'; | ||
import usePaginatedReportActions from '@hooks/usePaginatedReportActions'; | ||
import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
import useStyleUtils from '@hooks/useStyleUtils'; | ||
import * as ReportActionsUtils from '@libs/ReportActionsUtils'; | ||
|
@@ -141,13 +142,64 @@ function BaseReportActionContextMenu({ | |
|
||
const [download] = useOnyx(`${ONYXKEYS.COLLECTION.DOWNLOAD}${sourceID}`); | ||
|
||
const childReport = ReportUtils.getReport(reportAction?.childReportID ?? '-1'); | ||
const parentReportAction = ReportActionsUtils.getReportAction(childReport?.parentReportID ?? '', childReport?.parentReportActionID ?? ''); | ||
const {reportActions: paginatedReportActions} = usePaginatedReportActions(childReport?.reportID ?? '-1'); | ||
|
||
const transactionThreadReportID = useMemo( | ||
() => ReportActionsUtils.getOneTransactionThreadReportID(childReport?.reportID ?? '-1', paginatedReportActions ?? [], isOffline), | ||
[childReport?.reportID, paginatedReportActions, isOffline], | ||
); | ||
|
||
const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`); | ||
|
||
const isMoneyRequestReport = useMemo(() => ReportUtils.isMoneyRequestReport(childReport), [childReport]); | ||
const isInvoiceReport = useMemo(() => ReportUtils.isInvoiceReport(childReport), [childReport]); | ||
|
||
const requestParentReportAction = useMemo(() => { | ||
if (isMoneyRequestReport || isInvoiceReport) { | ||
if (!paginatedReportActions || !transactionThreadReport?.parentReportActionID) { | ||
return undefined; | ||
} | ||
return paginatedReportActions.find((action) => action.reportActionID === transactionThreadReport.parentReportActionID); | ||
} | ||
return parentReportAction; | ||
}, [parentReportAction, isMoneyRequestReport, isInvoiceReport, paginatedReportActions, transactionThreadReport?.parentReportActionID]); | ||
|
||
const moneyRequestAction = transactionThreadReportID ? requestParentReportAction : parentReportAction; | ||
|
||
const [parentReportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${childReport?.parentReportID ?? '-1'}`); | ||
const parentReport = ReportUtils.getReport(childReport?.parentReportID ?? '-1'); | ||
|
||
const isMoneyRequest = useMemo(() => ReportUtils.isMoneyRequest(childReport), [childReport]); | ||
const isTrackExpenseReport = ReportUtils.isTrackExpenseReport(childReport); | ||
const isSingleTransactionView = isMoneyRequest || isTrackExpenseReport; | ||
const isMoneyRequestOrReport = isMoneyRequestReport || isInvoiceReport || isSingleTransactionView; | ||
|
||
const areHoldRequirementsMet = isMoneyRequestOrReport && !ReportUtils.isArchivedRoom(transactionThreadReportID ? childReport : parentReport, parentReportNameValuePairs); | ||
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. This caused an issue where the hold options are shown for invoice reports. More info here. #47570 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. Invoice reports were added after this change so technically it didn't cause the issue. |
||
|
||
const originalReportID = useMemo(() => ReportUtils.getOriginalReportID(reportID, reportAction), [reportID, reportAction]); | ||
|
||
const shouldEnableArrowNavigation = !isMini && (isVisible || shouldKeepOpen); | ||
let filteredContextMenuActions = ContextMenuActions.filter( | ||
(contextAction) => | ||
!disabledActions.includes(contextAction) && | ||
contextAction.shouldShow(type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat, isUnreadChat, !!isOffline, isMini, isProduction), | ||
contextAction.shouldShow( | ||
type, | ||
reportAction, | ||
isArchivedRoom, | ||
betas, | ||
anchor, | ||
isChronosReport, | ||
reportID, | ||
isPinnedChat, | ||
isUnreadChat, | ||
!!isOffline, | ||
isMini, | ||
isProduction, | ||
moneyRequestAction, | ||
areHoldRequirementsMet, | ||
), | ||
); | ||
|
||
if (isMini) { | ||
|
@@ -254,6 +306,7 @@ function BaseReportActionContextMenu({ | |
interceptAnonymousUser, | ||
openOverflowMenu, | ||
setIsEmojiPickerActive, | ||
moneyRequestAction, | ||
}; | ||
|
||
if ('renderContent' in contextAction) { | ||
|
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.
Don't we have utils to get all these already?
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.
If you know of said utils then please point them out to me, but I don't think we already have one that would handle this sort of logic. This was recently written for just
ReportDetailsPage
which unified multiple pages with hold functionalities into one component.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.
Let me see.