Skip to content
Open
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
7 changes: 6 additions & 1 deletion src/libs/TagsOptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,12 @@ function getTagVisibility({
const policyTagLists = getTagLists(policyTags);

return policyTagLists.map(({tags, required}, index) => {
const isTagRequired = required || !!policy?.requiresTag;
// For independent multi-level tags each level has its own `required` flag, so mirror the per-level tag
// validation (see getTagViolationForIndependentTags) instead of OR-ing in the workspace-wide `requiresTag`,
// which would incorrectly label every level as "Required". Dependent multi-level tags are excluded: their
// validation (getTagViolationsForDependentTags) blocks submission on every level once `requiresTag` is on,
// regardless of each level's own `required` flag, so they must keep OR-ing the workspace-wide `requiresTag`.
Comment on lines +246 to +250

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.

Could we make this more succinct? It doesn't really feel worthy of a five line comment to me.

const isTagRequired = isMultilevelTags && !hasDependentTags ? (required ?? true) : required || !!policy?.requiresTag;
let shouldShow = false;

if (shouldShowTags) {
Expand Down
76 changes: 76 additions & 0 deletions tests/unit/TagsOptionsListUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,82 @@ describe('TagsOptionsListUtils', () => {

expect(result).toEqual([{isTagRequired: true, shouldShow: true}]);
});

it('should only mark the per-level required tags for independent multi-level tags even when policy.requiresTag is true', () => {
const policyWithRequiresTag = {...mockPolicy, requiresTag: true};
const multiLevelTags: PolicyTagLists = {
tagList1: {
name: 'Level A',
required: true,
tags: {tagA: {name: 'A', enabled: true}},
orderWeight: 0,
},
tagList2: {
name: 'Level B',
required: false,
tags: {tagB: {name: 'B', enabled: true}},
orderWeight: 1,
},
tagList3: {
name: 'Level C',
required: false,
tags: {tagC: {name: 'C', enabled: true}},
orderWeight: 2,
},
};

const result = getTagVisibility({
shouldShowTags: true,
policy: policyWithRequiresTag,
policyTags: multiLevelTags,
transaction: mockTransaction,
});

expect(result).toEqual([
{isTagRequired: true, shouldShow: true},
{isTagRequired: false, shouldShow: true},
{isTagRequired: false, shouldShow: true},
]);
});

it('should keep marking every level required for dependent multi-level tags when policy.requiresTag is true even if a level required is false', () => {
const policyWithRequiresTag = {...mockPolicy, requiresTag: true, hasMultipleTagLists: true};
const dependentMultiLevelTags: PolicyTagLists = {
tagList1: {
name: 'Level A',
required: false,
tags: {tagA: {name: 'A', enabled: true, rules: {parentTagsFilter: ''}}},
orderWeight: 0,
},
tagList2: {
name: 'Level B',
required: false,
tags: {tagB: {name: 'B', enabled: true, rules: {parentTagsFilter: 'A'}}},
orderWeight: 1,
},
tagList3: {
name: 'Level C',
required: false,
tags: {tagC: {name: 'C', enabled: true, rules: {parentTagsFilter: 'A:B'}}},
orderWeight: 2,
},
};

const result = getTagVisibility({
shouldShowTags: true,
policy: policyWithRequiresTag,
policyTags: dependentMultiLevelTags,
transaction: {...mockTransaction, tag: 'A:B:C'},
});

// Dependent tags block submission on every level once requiresTag is on, so the badge must
// stay "Required" on every level regardless of each level's own `required` flag.
expect(result).toEqual([
{isTagRequired: true, shouldShow: true},
{isTagRequired: true, shouldShow: true},
{isTagRequired: true, shouldShow: true},
]);
});
});

describe('getEnabledTags', () => {
Expand Down
Loading