Skip to content
Merged
Show file tree
Hide file tree
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
5 changes: 3 additions & 2 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ function MoneyRequestView({
(!isPerDiemRequest || canSubmitPerDiemExpenseFromWorkspace(policy) || (isExpenseUnreported && !!perDiemOriginalPolicy));

const policyTagLists = getTagLists(policyTagList);
const policyHasEnabledTags = hasEnabledTags(policyTagLists);

const category = transactionCategory ?? '';
const categoryForDisplay = isCategoryMissing(category) ? '' : category;
Expand All @@ -522,12 +523,12 @@ function MoneyRequestView({
(isExpenseUnreported && (!policyForMovingExpenses || hasEnabledOptions(policyCategories ?? {})));
// transactionTag can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const shouldShowTag = (isPolicyExpenseChat || isExpenseUnreported) && (transactionTag || (canEdit && hasEnabledTags(policyTagLists)));
const shouldShowTag = (isPolicyExpenseChat || isExpenseUnreported) && (transactionTag || (canEdit && policyHasEnabledTags));
// Surface a delete confirmation (like tax) when the value is stale and there's nothing valid to select, instead of
// navigating to edit. Categories need at least one, so they only hit this when disabled; tags can be fully emptied,
// so also cover "no enabled tags remain". Applies to multi-level tags too - deleting clears the whole tag value.
const shouldShowCategoryDisabledAlert = !policy?.areCategoriesEnabled && !!category;
const shouldShowTagDisabledAlert = (!policy?.areTagsEnabled || !hasEnabledTags(policyTagLists)) && !!transactionTag;
const shouldShowTagDisabledAlert = (!policy?.areTagsEnabled || !policyHasEnabledTags) && !!transactionTag;
const shouldShowBillable = (isPolicyExpenseChat || isExpenseUnreported) && (!!transactionBillable || isBillableEnabledOnPolicy(policy) || !!updatedTransaction?.billable);
const isCurrentTransactionReimbursableDifferentFromPolicyDefault =
policy?.defaultReimbursable !== undefined && !!(updatedTransaction?.reimbursable ?? transactionReimbursable) !== policy.defaultReimbursable;
Expand Down
10 changes: 9 additions & 1 deletion src/libs/OptionsListUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -888,11 +888,18 @@ function getSearchValueForPhoneOrEmail(searchTerm: string, countryCode: number)
return parsedPhoneNumber.possible ? (parsedPhoneNumber.number?.e164 ?? '') : searchTerm.toLowerCase();
}

/**
* Verifies that a single option (category or tag) is enabled and not pending deletion
*/
function isOptionEnabled(option?: PolicyTag | PolicyCategory): boolean {
return !!option?.enabled && option.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
}

/**
* Verifies that there is at least one enabled option
*/
function hasEnabledOptions(options: PolicyCategories | PolicyTag[]): boolean {
return Object.values(options).some((option: PolicyTag | PolicyCategory) => option.enabled && option.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
return Object.values(options).some(isOptionEnabled);
}

/**
Expand Down Expand Up @@ -3180,6 +3187,7 @@ export {
isDisablingOrDeletingLastEnabledCategory,
isDisablingOrDeletingLastEnabledTag,
isMakingLastRequiredTagListOptional,
isOptionEnabled,
isPersonalDetailsReady,
optionsOrderAndGroupBy,
optionsOrderBy,
Expand Down
27 changes: 21 additions & 6 deletions src/libs/TagsOptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {OnyxEntry} from 'react-native-onyx';
import type {Option} from './OptionsListUtils';

import {insertTagIntoTransactionTagsString} from './IOUUtils';
import {hasEnabledOptions} from './OptionsListUtils';
import {hasEnabledOptions, isOptionEnabled} from './OptionsListUtils';
import {
getCleanedTagName,
getGLCodeFromPolicyTag,
Expand Down Expand Up @@ -208,12 +208,27 @@ function getTagListSections({
* Verifies that there is at least one enabled tag
*/
function hasEnabledTags(policyTagList: Array<PolicyTagLists[keyof PolicyTagLists]>) {
const policyTagValueList = policyTagList
.filter((tag) => tag?.tags)
.map(({tags}) => Object.values(tags))
.flat();
for (const tagList of policyTagList) {

@aimane-chnaif aimane-chnaif Sep 22, 2026 •

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.

function hasEnabledTags(policyTagList: Array<PolicyTagLists[keyof PolicyTagLists]>) {
    return policyTagList.some((tagList) => Object.values(tagList?.tags ?? {}).some(isOptionEnabled));
}

Can we simplify code like this? Or doesn't help with performance optimization?

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.

Object.values() creates the full tag array for a list. So on a policy with a few thousand tags we allocate the whole array even when the very first tag answers the question. The loop allocates nothing and bails on the first hit, so the performance is better here

const tags = tagList?.tags;

return hasEnabledOptions(policyTagValueList);
if (!tags) {
continue;
}

for (const tagName in tags) {
if (!Object.hasOwn(tags, tagName)) {
continue;
}

const tag = tags[tagName];

Comment thread
TMisiukiewicz marked this conversation as resolved.
if (isOptionEnabled(tag)) {
return true;
}
}
}

return false;
}

/**
Expand Down
151 changes: 150 additions & 1 deletion tests/unit/TagsOptionsListUtilsTest.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {Section} from '@components/SelectionList/SelectionListWithSections/types';

import type {SelectedTagOption, TagOption} from '@libs/TagsOptionsListUtils';
import {getEnabledTags, getTagListSections, getTagVisibility, getUpdatedTransactionTag, sortTags} from '@libs/TagsOptionsListUtils';
import {getEnabledTags, getTagListSections, getTagVisibility, getUpdatedTransactionTag, hasEnabledTags, sortTags} from '@libs/TagsOptionsListUtils';

import CONST from '@src/CONST';
import IntlStore from '@src/languages/IntlStore';
Expand Down Expand Up @@ -1187,4 +1187,153 @@ describe('TagsOptionsListUtils', () => {
expect(result).toBe('Acme Corp');
});
});

describe('hasEnabledTags', () => {
Comment thread
TMisiukiewicz marked this conversation as resolved.
type TagListValue = PolicyTagLists[keyof PolicyTagLists];
const buildTagLists = (...tagLists: PolicyTagLists[]): TagListValue[] => {
const result: TagListValue[] = [];
for (const tagList of tagLists) {
result.push(...Object.values(tagList));
}
return result;
};

it('returns true when at least one tag is enabled', () => {
// Given a tag list where one tag is disabled and one is enabled
const tagLists = buildTagLists({
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {
disabled: {name: 'Disabled', enabled: false},
enabled: {name: 'Enabled', enabled: true},
},
},
});

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then true, the disabled tag must not mask the selectable one behind it
expect(result).toBe(true);
});

it('returns false when every tag is disabled', () => {
// Given a tag list an admin has fully disabled
const tagLists = buildTagLists({
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {
a: {name: 'A', enabled: false},
b: {name: 'B', enabled: false},
},
},
});

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then false, callers hide the tag row instead of opening an empty picker
expect(result).toBe(false);
});

it('returns false when the only enabled tag is pending deletion', () => {
// Given a still-enabled tag that is optimistically being deleted offline
const tagLists = buildTagLists({
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {
enabled: {name: 'Enabled', enabled: true, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
disabled: {name: 'Disabled', enabled: false},
},
},
});

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then false, a tag on its way out should not look selectable
expect(result).toBe(false);
});

it('returns true when an enabled tag is pending an action other than deletion', () => {
// Given an enabled tag pending an update, so it survives once the request settles
const tagLists = buildTagLists({
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {
enabled: {name: 'Enabled', enabled: true, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},
},
},
});

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then true, only a pending deletion disqualifies a tag
expect(result).toBe(true);
});

it('finds an enabled tag across multiple tag lists', () => {
// Given multi-level tags where only the second list has an enabled tag
const tagLists = buildTagLists(
{
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {a: {name: 'A', enabled: false}},
},
},
{
list2: {
name: 'List 2',
required: false,
orderWeight: 1,
tags: {b: {name: 'B', enabled: true}},
},
},
);

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then true, the search must not stop at the first list it finds nothing in
expect(result).toBe(true);
});

it('returns false for an empty list of tag lists', () => {
// Given a policy with no tag lists
// When checking whether the policy has a selectable tag
const result = hasEnabledTags([]);

// Then false rather than a throw, callers pass whatever Onyx currently holds
expect(result).toBe(false);
});

it('ignores a tag list that has no tags', () => {
// Given a tag list an admin just created and never populated
const tagLists = buildTagLists({
list1: {
name: 'List 1',
required: false,
orderWeight: 0,
tags: {},
},
});

// When checking whether the policy has a selectable tag
const result = hasEnabledTags(tagLists);

// Then false, an empty list offers nothing to select
expect(result).toBe(false);
});
});
});
Loading