Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {useIsFocused} from '@react-navigation/native';
import {format} from 'date-fns';
import Str from 'expensify-common/lib/str';
import {isUndefined} from 'lodash';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useEffect, useMemo, useReducer, useRef, useState} from 'react';
Expand Down Expand Up @@ -490,6 +491,31 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
IOU.setMoneyRequestMerchant(transaction.transactionID, distanceMerchant, true);
}, [isDistanceRequestWithPendingRoute, hasRoute, distance, unit, rate, currency, translate, toLocaleDigit, isDistanceRequest, transaction]);

// Auto select the category if there is only one enabled category and it is required
useEffect(() => {
Comment thread
dukenv0307 marked this conversation as resolved.
const enabledCategories = _.filter(policyCategories, (category) => category.enabled);
if (iouCategory || !shouldShowCategories || enabledCategories.length !== 1 || !isCategoryRequired) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using isCategoryRequired here means we'll get inconsistent behaviour vs tags as isCategoryRequired relies on canUseViolations being true, whereas the checks for the tags are not reliant on that. Do you think it's better to just use lodashGet(policy, 'requiresCategory', false) directly here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjcoffee With tags, we not only have isTagRequired which is canUseViolations && lodashGet(policy, 'requiresTag', false) but also have the required field for each tag in multiple level tags. So which required field should we use in this case? I am using the first one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dukenv0307 Ah good point! For multi-level tags I think we need to use the required field on each tag. For single-level tags however, we need to use isTagRequired as on oldDot this option is only available when multi-level tags are turned off (People must tag expenses):

image

Regarding my previous comment, I think it probably makes sense to just add canUseViolations to the tags to keep things consistent. Tags and Categories don't display the Required text unless canUseViolations is true so it makes sense for us to tie in with that I think!

return;
}
IOU.setMoneyRequestCategory(transaction.transactionID, enabledCategories[0].name);
}, [iouCategory, shouldShowCategories, policyCategories, transaction, isCategoryRequired]);

// Auto select the tag if there is only one enabled tag and it is required
useEffect(() => {
let updatedTagsString = TransactionUtils.getTag(transaction);
policyTagLists.forEach((tagList, index) => {
const enabledTags = _.filter(tagList.tags, (tag) => tag.enabled);
const isTagListRequired = isUndefined(tagList.required) ? false : tagList.required && canUseViolations;
Comment thread
puneetlath marked this conversation as resolved.
if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
return;
}
updatedTagsString = IOUUtils.insertTagIntoTransactionTagsString(updatedTagsString, enabledTags[0] ? enabledTags[0].name : '', index);
});
if (updatedTagsString !== TransactionUtils.getTag(transaction) && updatedTagsString) {
IOU.setMoneyRequestTag(transaction.transactionID, updatedTagsString);
}
}, [policyTagLists, transaction, policyTags, isTagRequired, canUseViolations]);

/**
* @param {Object} option
*/
Expand Down