From 4b6ca555533e9778a026d495d3b84d6c083f0867 Mon Sep 17 00:00:00 2001
From: Michelle Mounde <mitchmounde@gmail.com>
Date: Thu, 2 Nov 2023 22:54:57 +0300
Subject: [PATCH] Stop showing scheduled deletions on unrelated channels
 (#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
---
 ui/src/utils/rules.js      | 21 +++++++++++++++------
 ui/test/ListRules_test.jsx | 31 +++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/ui/src/utils/rules.js b/ui/src/utils/rules.js
index c3f2c054be..af5aaafbfd 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 16898abded..ba4d119442 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();
   });
 });