Adds new ReportUtils for Violations #31448
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,10 +8,12 @@ import _ from 'underscore'; | |||||||
| import participantPropTypes from '@components/participantPropTypes'; | ||||||||
| import transactionPropTypes from '@components/transactionPropTypes'; | ||||||||
| import withCurrentReportID, {withCurrentReportIDDefaultProps, withCurrentReportIDPropTypes} from '@components/withCurrentReportID'; | ||||||||
| import usePermissions from '@hooks/usePermissions'; | ||||||||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||||||||
| import compose from '@libs/compose'; | ||||||||
| import * as OptionsListUtils from '@libs/OptionsListUtils'; | ||||||||
| import * as ReportUtils from '@libs/ReportUtils'; | ||||||||
| import {transactionViolationsPropType} from '@libs/Violations/propTypes'; | ||||||||
| import reportActionPropTypes from '@pages/home/report/reportActionPropTypes'; | ||||||||
| import reportPropTypes from '@pages/reportPropTypes'; | ||||||||
| import stylePropTypes from '@styles/stylePropTypes'; | ||||||||
|
|
@@ -63,8 +65,13 @@ const propTypes = { | |||||||
|
|
||||||||
| /** The transaction from the parent report action */ | ||||||||
| transactions: PropTypes.objectOf(transactionPropTypes), | ||||||||
|
|
||||||||
| /** List of draft comments */ | ||||||||
| draftComments: PropTypes.objectOf(PropTypes.string), | ||||||||
|
|
||||||||
| /** The list of transaction violations */ | ||||||||
| transactionViolations: transactionViolationsPropType, | ||||||||
|
|
||||||||
| ...withCurrentReportIDPropTypes, | ||||||||
|
There was a problem hiding this comment. As new lines are added above, let's add here too
Suggested change
|
||||||||
| }; | ||||||||
|
|
||||||||
|
|
@@ -78,6 +85,7 @@ const defaultProps = { | |||||||
| personalDetails: {}, | ||||||||
| transactions: {}, | ||||||||
| draftComments: {}, | ||||||||
| transactionViolations: {}, | ||||||||
| ...withCurrentReportIDDefaultProps, | ||||||||
| }; | ||||||||
|
|
||||||||
|
|
@@ -98,8 +106,10 @@ function LHNOptionsList({ | |||||||
| transactions, | ||||||||
| draftComments, | ||||||||
| currentReportID, | ||||||||
| transactionViolations, | ||||||||
| }) { | ||||||||
| const styles = useThemeStyles(); | ||||||||
| const {canUseViolations} = usePermissions(); | ||||||||
| /** | ||||||||
| * Function which renders a row in the list | ||||||||
| * | ||||||||
|
|
@@ -137,10 +147,26 @@ function LHNOptionsList({ | |||||||
| onSelectRow={onSelectRow} | ||||||||
| preferredLocale={preferredLocale} | ||||||||
| comment={itemComment} | ||||||||
| transactionViolations={transactionViolations} | ||||||||
| canUseViolations={canUseViolations} | ||||||||
| /> | ||||||||
| ); | ||||||||
| }, | ||||||||
| [ | ||||||||
| currentReportID, | ||||||||
| draftComments, | ||||||||
| onSelectRow, | ||||||||
| optionMode, | ||||||||
| personalDetails, | ||||||||
| policy, | ||||||||
| preferredLocale, | ||||||||
| reportActions, | ||||||||
| reports, | ||||||||
| shouldDisableFocusOptions, | ||||||||
| transactions, | ||||||||
| transactionViolations, | ||||||||
| canUseViolations, | ||||||||
| ], | ||||||||
| ); | ||||||||
|
|
||||||||
| return ( | ||||||||
|
|
@@ -189,5 +215,8 @@ export default compose( | |||||||
| draftComments: { | ||||||||
| key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, | ||||||||
| }, | ||||||||
| transactionViolations: { | ||||||||
| key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, | ||||||||
|
There was a problem hiding this comment. Out of curiosity, is there a reason we're taking all the transaction violations, and not setting this to a function that takes a transactionID and returns the transaction violations for a specific transaction? There was a problem hiding this comment. Are we able to determine the transactionId within the There was a problem hiding this comment. @cead22 -- this I'll have to look into a bit more as this is my first contact with There was a problem hiding this comment. Will this work? export default compose(
withCurrentReportID,
withOnyx({
// ..
transactionViolations: {
key: ({currentReportId}) => `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${currentReportId}`,
},
}),
)(LHNOptionsList);There was a problem hiding this comment. why need to filter out with currentReportId here? This is OptionsList, not OptionRow |
||||||||
| }, | ||||||||
| }), | ||||||||
| )(LHNOptionsList); | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /** | ||
| * @returns A mock of the usePermissions hook. | ||
| */ | ||
| const usePermissions = () => ({ | ||
| canUseViolations: true, | ||
| }); | ||
| export default usePermissions; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1292,6 +1292,7 @@ function getOptions( | |
| recentlyUsedTags = [], | ||
| canInviteUser = true, | ||
| includeSelectedOptions = false, | ||
| transactionViolations = {}, | ||
| includePolicyTaxRates, | ||
| policyTaxRates, | ||
| }, | ||
|
|
@@ -1357,7 +1358,22 @@ function getOptions( | |
| const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); | ||
|
|
||
| // Filter out all the reports that shouldn't be displayed | ||
| const filteredReports = _.filter(reports, (report) => { | ||
| const {parentReportID, parentReportActionID} = report || {}; | ||
|
There was a problem hiding this comment. Not sure -- it happened once while I was building this but that may have been an artifact of some bad state. Reports here is an object, so it's possible that one of the properties of that object could be null. Especially in javascript. So it's mostly for safety, as it will cause the app to crash if you try and destructure There was a problem hiding this comment. I get the safety argument, but is it possible for report not to be defined, since it comes from There was a problem hiding this comment. If we don't know if it's possible, I don't think we should add the fallback, at least not without some logging to understand why it happens. If we know it's possible, then let's add a comment explaining when that can happen There was a problem hiding this comment. Ah here is where it is null: #31448 (comment) Not sure why -- i guess this is called when creating the list for new chats. There was a problem hiding this comment. Let's add a comment explaining why it's necessary to have the fallback |
||
| const canGetParentReport = parentReportID && parentReportActionID && allReportActions; | ||
| const parentReportAction = canGetParentReport ? lodashGet(allReportActions, [parentReportID, parentReportActionID], {}) : {}; | ||
| const doesReportHaveViolations = betas.includes(CONST.BETAS.VIOLATIONS) && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction); | ||
|
|
||
| return ReportUtils.shouldReportBeInOptionList({ | ||
| report, | ||
| currentReportId: Navigation.getTopmostReportId(), | ||
| betas, | ||
| policies, | ||
| doesReportHaveViolations, | ||
| isInGSDMode: false, | ||
| excludeEmptyChats: false, | ||
| }); | ||
| }); | ||
|
|
||
| // Sorting the reports works like this: | ||
| // - Order everything by the last message timestamp (descending) | ||
|
|
||
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
Sorry, something went wrong.