diff --git a/ui/src/utils/rules.js b/ui/src/utils/rules.js index c3f2c054b..af5aaafbf 100644 --- a/ui/src/utils/rules.js +++ b/ui/src/utils/rules.js @@ -18,12 +18,21 @@ const ruleMatchesChannel = (rule, channel) => { // if a scheduled change does not exist at all // we never want this to match, otherwise all rules // without scheduled changes will always match any filter - const scChannelMatches = rule.scheduledChange - ? rule.scheduledChange.channel === null || - rule.scheduledChange.channel === '' || - rule.scheduledChange.channel === channel || - matchesGlob(rule.scheduledChange.channel, channel) - : false; + // if a scheduled change is a deletion + // we never want scheduled deletions to match any filter + // because the scheduled change channel on deletions is null + // and the rule channel is a truthy value that might not equal 'channel' + let scChannelMatches = false; + + if (rule.scheduledChange) { + if (rule.scheduledChange.change_type !== 'delete') { + scChannelMatches = + rule.scheduledChange.channel === null || + rule.scheduledChange.channel === '' || + rule.scheduledChange.channel === channel || + matchesGlob(rule.scheduledChange.channel, channel); + } + } return ruleChannelMatches || scChannelMatches; }; diff --git a/ui/test/ListRules_test.jsx b/ui/test/ListRules_test.jsx index 16898abde..ba4d11944 100644 --- a/ui/test/ListRules_test.jsx +++ b/ui/test/ListRules_test.jsx @@ -25,7 +25,7 @@ describe('channel matching', () => { expect(result).toBeTruthy(); }); - test('should match when rule is null', () => { + test('should match when rule is null and scheduled change matches', () => { const result = ruleMatchesChannel( { scheduledChange: { @@ -59,9 +59,22 @@ describe('channel matching', () => { expect(results).toEqual(expect.not.arrayContaining([false])); }); + test('should match when rule is null and scheduled change has null channel', () => { + const result = ruleMatchesChannel( + { + scheduledChange: { + channel: null, + }, + }, + 'nightly' + ); + + expect(result).toBeTruthy(); + }); test('should match when rule and scheduled change have null channel', () => { const result = ruleMatchesChannel( { + channel: null, scheduledChange: { channel: null, }, @@ -71,7 +84,7 @@ describe('channel matching', () => { expect(result).toBeTruthy(); }); - test('should not match anything when rule is null', () => { + test('should not match anything when rule is null and scheduled change is a different channel', () => { const result = ruleMatchesChannel( { scheduledChange: { @@ -118,6 +131,20 @@ describe('channel matching', () => { 'release' ); + expect(result).toBeFalsy(); + }); + test('should not match when rule has a different channel and scheduled change type is delete with channel null', () => { + const result = ruleMatchesChannel( + { + channel: 'aurora', + scheduledChange: { + change_type: 'delete', + channel: null, + }, + }, + 'release' + ); + expect(result).toBeFalsy(); }); });