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
9 changes: 8 additions & 1 deletion src/libs/IOUUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,16 @@ function insertTagIntoTransactionTagsString(transactionTags: string, tag: string
return tag;
}

const tagArray = getTagArrayFromName(transactionTags);
const tagArray = transactionTags ? getTagArrayFromName(transactionTags) : [];
tagArray[tagIndex] = tag;

// Fill any sparse slots created when tagIndex > tagArray.length
for (let i = 0; i < tagArray.length; i++) {
if (tagArray.at(i) === undefined) {
tagArray[i] = '';
}
}

while (tagArray.length > 0 && !tagArray.at(-1)) {
tagArray.pop();
}
Expand Down
11 changes: 10 additions & 1 deletion src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ function getTagViolationsForSingleLevelTags(
*/
function getTagViolationsForDependentTags(policyTagList: PolicyTagLists, transactionViolations: TransactionViolation[], tagName: string) {
const tagViolations = [...transactionViolations];
const policyTagKeys = getSortedTagKeys(policyTagList);

if (!tagName) {
for (const tagList of Object.values(policyTagList)) {
Expand All @@ -127,7 +128,15 @@ function getTagViolationsForDependentTags(policyTagList: PolicyTagLists, transac
}
} else {
const tags = TransactionUtils.getTagArrayFromName(tagName);
if (Object.keys(policyTagList).length !== tags.length || tags.includes('')) {
// Only flag ALL_TAG_LEVELS_REQUIRED if a required tag level is empty or missing.
// Previously this used `tags.includes('')` and a length check which flagged any
// empty/missing level regardless of whether it was required, causing false violations
// when only some levels were filled (e.g. "Engineering" with non-required second level).
const hasEmptyRequiredLevel = policyTagKeys.some((key, index) => {
const tagValue = tags.at(index) ?? '';
return tagValue === '' && (policyTagList[key]?.required ?? true);
});
Comment thread
wildan-m marked this conversation as resolved.
if (hasEmptyRequiredLevel) {
Comment thread
Julesssss marked this conversation as resolved.
tagViolations.push({
name: CONST.VIOLATIONS.ALL_TAG_LEVELS_REQUIRED,
type: CONST.VIOLATION_TYPES.VIOLATION,
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/IOUUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,18 @@ describe('IOUUtils', () => {
test('Return multiple tags when hasMultipleTagLists is true', () => {
expect(IOUUtils.insertTagIntoTransactionTagsString('East:NY:California', 'NewTag', 1, true)).toBe('East:NewTag:California');
});

test('Should not produce a leading colon when transactionTags is empty and tagIndex > 0', () => {
expect(IOUUtils.insertTagIntoTransactionTagsString('', 'Alpha', 1, true)).toBe(':Alpha');
});

test('Should produce correct result when transactionTags is empty and tagIndex is 0', () => {
expect(IOUUtils.insertTagIntoTransactionTagsString('', 'Alpha', 0, true)).toBe('Alpha');
});

test('Should fill sparse slots when tagIndex exceeds current array length', () => {
expect(IOUUtils.insertTagIntoTransactionTagsString('First', 'Third', 2, true)).toBe('First::Third');
});
});
});

Expand Down
24 changes: 24 additions & 0 deletions tests/unit/ViolationUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,30 @@ describe('getViolationsOnyxData', () => {

expect(result.value).toEqual([]);
});
it('should not return allTagLevelsRequired when only non-required dependent tag levels are empty', () => {
// Make Department non-required
policyTags.Department.required = false;
// Transaction has only Region and Project filled, Department (non-required) is empty
transaction.tag = 'Africa::Project1';

// hasDependentTags = true to exercise getTagViolationsForDependentTags
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, true, false);

expect(result.value).not.toContainEqual(expect.objectContaining({name: CONST.VIOLATIONS.ALL_TAG_LEVELS_REQUIRED}));
});

it('should return allTagLevelsRequired when a required dependent tag level is empty', () => {
// Make Project non-required, keep Region and Department required
policyTags.Project.required = false;
// Transaction has only Region filled, Department (required) is empty
transaction.tag = 'Africa';

// hasDependentTags = true to exercise getTagViolationsForDependentTags
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, true, false);

expect(result.value).toContainEqual(expect.objectContaining({name: CONST.VIOLATIONS.ALL_TAG_LEVELS_REQUIRED}));
});

it('should return missingTag when all dependent tags are enabled in the policy but are not set in the transaction', () => {
const missingDepartmentTag = {...missingTagViolation, data: {tagName: 'Department'}};
const missingRegionTag = {...missingTagViolation, data: {tagName: 'Region'}};
Expand Down
Loading