Skip to content

Commit

Permalink
Stop showing scheduled deletions on unrelated channels (mozilla-relen…
Browse files Browse the repository at this point in the history
…g#3041)

* Modify ruleMatchesChannel to not return postive when rule channel doesn't match and rule scheduled changes channel is null

* Update scheduled change channel matching to check of rule channel exists

* Reword contradictory test descriptions to accurately match what is being tested

* Add test based on reworded test

* Add test for when rule channel doesn't match and scheduled change channel is null

* Modify scheduled change channel match for when rule is null and scheduled change channel is null

* Fix lint errors

* Modify ruleMatchesChannel to check for scheduled deletion
  • Loading branch information
michellemounde authored Nov 2, 2023
1 parent a4536ab commit 4b6ca55
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
21 changes: 15 additions & 6 deletions ui/src/utils/rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
31 changes: 29 additions & 2 deletions ui/test/ListRules_test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
},
Expand All @@ -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: {
Expand Down Expand Up @@ -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();
});
});

0 comments on commit 4b6ca55

Please sign in to comment.