Merge ~caleb-ellis/maas:suppress-single-test into maas:master

[Caleb Ellis]

Allow dismissing tests from tests tab in machine details page

## Wireframe
https://app.zeplin.io/project/5bfff7810f5b0001ebd72a27/dashboard

## Done
- Added unsuppress_test method to node websocket handler
- Added a "Hide" column and checkbox to tests tab, which toggles suppressed parameter of a single **failed** test
- Added warning notification to tests tab if suppressed tests exist

## QA
- Log in as an admin
- Go to the hardware tests tab of a machine that has failed tests
- Hide a test and check that the row is slightly greyed out and a notification pops up
- Hide every failed test and check that the machine summary page no longer has red borders around the failed test suites. Note that the hardware tests tab icon will change to a green tick - in future this should be a warning icon but there isn't currently a testing status code for "some tests failed, but they were all suppressed"
- Log out and back in as a normal user
- Check that the hide test checkboxes are disabled and a tooltip shows up on hover

## Screenshots
### Default hardware tests page
https://user-images.githubusercontent.com/25733845/56738843-5d870b80-6765-11e9-8a37-531ee62efb70.png

### One test suppressed
https://user-images.githubusercontent.com/25733845/56738848-61b32900-6765-11e9-9255-3fa57818da5c.png

### All tests suppressed
https://user-images.githubusercontent.com/25733845/56738879-77c0e980-6765-11e9-999a-73eea6e11a10.png

### Updated summary page
https://user-images.githubusercontent.com/25733845/56738887-7c859d80-6765-11e9-9ce5-de3325b37795.png

### Updated listing page
https://user-images.githubusercontent.com/25733845/56738901-827b7e80-6765-11e9-91e5-af00dcac0eb5.png

EDIT: Updated to have an "All" label and made the Hide col a bit thinner:
https://user-images.githubusercontent.com/25733845/56890058-42234580-6a70-11e9-9219-23b6a8c4e72f.png

EDIT2: Updated so only admins can suppress/unsuppress tests. If user is a non-admin, the checkbox is disabled with a tooltip:
https://user-images.githubusercontent.com/25733845/56906850-fb971080-6a9a-11e9-91af-6f381d58b333.png

EDIT3: Added ScriptStatus to enum.js, and updated so only tests that have FAILED, FAILED_INSTALLING or TIMEDOUT can be suppressed.
https://user-images.githubusercontent.com/25733845/56949538-1a44e800-6b2b-11e9-9dfa-6042a6def50c.png

EDIT4: Changed instances of "hide" to "suppress":
https://user-images.githubusercontent.com/25733845/56979857-a4fd0580-6b72-11e9-9b49-d9742ab0c219.png

[Blake Rouse (blake-rouse)]

UI looks good, just some things that need to be fixed.

review: Needs Fixing

[Martin Storey (cassiocassio)]

nice.

- The column width for the HIDE column is kind of wide for a single checkbox

- Should/could we special-case "All tests hidden" in the notification?

(scope creep)

- when you get to "Hide all tests" in the overide failed tests dialog, can you expose the same hide-all functionality with a checkbox at the top the HIDE column?

[Lee Trager (ltrager)]

I agree with Blake that the column should be called Suppressed like it is on the API.

In master we have set_script_result_suppressed which is the inverse of set_script_result_unsuppressed that Cabel implemented. I think we should fix both in a separate bug. However the UI should be updated to also send the system_id.

review: Needs Fixing

[Martin Storey (cassiocassio)]

I aggree for historical features that have established use in CLI and code,
that the GUI should reflect the API, so users who already know one can
recognise the feature as the same.

In this case, it's a new feature, and we can call it what we want, to
reflect the functionality.

--
Separate question. Is "supress" a good name? I can live with it, but I've
never liked it. It sounds more technical than it really is.

"Hide" is simpler, more honest language that's immediately understandable,
unambiguous.

Some of our language, eg. "commission" and "deploy" follow industry
practice, so they're the right word, even though they're slightly
ridiculous. You commission officers and deploy troops. I wonder why all
this language is so military?

So perhaps that's why "Suppress" - it's what the First Order does to The
Resistance?

But please, what we really mean here is "Hide".

On Fri, Apr 26, 2019 at 7:42 AM Lee Trager <email address hidden> wrote:

> Review: Needs Fixing
>
> I agree with Blake that the column should be called Suppressed like it is
> on the API.
>
> In master we have set_script_result_suppressed which is the inverse of
> set_script_result_unsuppressed that Cabel implemented. I think we should
> fix both in a separate bug. However the UI should be updated to also send
> the system_id.
> --
> https://code.launchpad.net/~caleb-ellis/maas/+git/maas/+merge/366513
> Your team MAAS UI team is requested to review the proposed merge of
> ~caleb-ellis/maas:suppress-single-test into maas:master.
>

[Caleb Ellis (caleb-ellis)]

> I agree with Blake that the column should be called Suppressed like it is on
> the API.
>
> In master we have set_script_result_suppressed which is the inverse of
> set_script_result_unsuppressed that Cabel implemented. I think we should fix
> both in a separate bug. However the UI should be updated to also send the
> system_id.

Alright I've updated so the UI sends through the node system_id, and I'm okay to leave the permissions issue with you?

[Blake Rouse (blake-rouse)]

review: Needs Fixing

[Caleb Ellis (caleb-ellis)]

Updated so only admins can suppress or unsuppress tests, otherwise a permission exception is raised. In the UI, the checkboxes become disabled for non-admins.

[Lilyana Videnova (lilyanavidenova)]

Can we add a tooltip on those disabled checkboxes saying "Only admins can hide failed tests."

[Caleb Ellis (caleb-ellis)]

Done:

https://user-images.githubusercontent.com/25733845/56906850-fb971080-6a9a-11e9-91af-6f381d58b333.png

[Blake Rouse (blake-rouse)]

Thanks for the fixes, looks good.

review: Approve

[Lee Trager (ltrager)]

I just tested this branch and if I click hide nothing happens, same if I try the option in the menu list. I also noticed that the 'hide' check box is shown for aborted and pending, installing, and running statuses. It should only be shown if the test has failed, failed installing, or timed out.

I still think we need to be consistent between in naming between the API and UI. Users often do something in the UI and then try to automate that process. The API currently calls this suppress and does not mention 'hiding' at all. This will leave users confused as to how to 'hide' a ScriptResult with the API. I'd be open to changing the API to 'hide' if we agree that is a better name however we have already released a few beta's with this being called suppress so changing at this point would cause API breakage.

review: Needs Fixing

[Anthony Dillon (ya-bo-ng)]

Code looks good to me +1, so inline styles but nothing alarming.

review: Approve

[Caleb Ellis (caleb-ellis)]

Lee, did you run `make force-javascript` beforehand? I've updated so only tests with status FAILED, FAILED_INSTALLING or TIMEDOUT can be suppressed via the UI. As for the table header being "Hide" I'll leave it as is for now - this was agreed upon in the design meetings and is in the 9th iteration of the wireframe (https://app.zeplin.io/project/5bfff7810f5b0001ebd72a27). If it proves to be confusing for users it's only a simple text change.

https://user-images.githubusercontent.com/25733845/56949538-1a44e800-6b2b-11e9-9dfa-6042a6def50c.png

[Lee Trager (ltrager)]

My bad I did make javascript instead of make force-javascript. Thanks for the fixes, LGTM!

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b suppress-single-test lp:~caleb-ellis/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5574/consoleText

2b0be59... by Caleb Ellis on 2019-05-01

Fix linting

diff --git a/src/maasserver/static/js/angular/controllers/node_results.js b/src/maasserver/static/js/angular/controllers/node_results.js
index 66a7c0d..6ebccb9 100644
--- a/src/maasserver/static/js/angular/controllers/node_results.js
+++ b/src/maasserver/static/js/angular/controllers/node_results.js
@@ -1,4 +1,4 @@
-import NodeResultController from "./node_result";
+import { ScriptStatus } from "../enum";
 
 /* Copyright 2017-2018 Canonical Ltd.  This software is licensed under the
  * GNU Affero General Public License version 3 (see the file LICENSE).
@@ -222,6 +222,46 @@ function NodeResultsController(
     });
   };
 
+  $scope.hasSuppressedTests = () => {
+    return $scope.results.some(type => {
+      const entries = Object.entries(type.results);
+      return entries.some(entry => entry[1].some(result => result.suppressed));
+    });
+  };
+
+  $scope.isSuppressible = result =>
+    result.status === ScriptStatus.FAILED ||
+    result.status === ScriptStatus.FAILED_INSTALLING ||
+    result.status === ScriptStatus.TIMEDOUT;
+
+  $scope.getSuppressedCount = () => {
+    const suppressibleTests = $scope.results.reduce((acc, type) => {
+      const entries = Object.entries(type.results);
+      entries.forEach(entry => {
+        entry[1].forEach(result => {
+          if ($scope.isSuppressible(result)) {
+            acc.push(result);
+          }
+        });
+      });
+      return acc;
+    }, []);
+    const suppressedTests = suppressibleTests.filter(test => test.suppressed);
+
+    if (suppressibleTests.length === suppressedTests.length) {
+      return "All";
+    }
+    return suppressedTests.length;
+  };
+
+  $scope.toggleSuppressed = result => {
+    if (result.suppressed) {
+      $scope.nodesManager.unsuppressTests($scope.node, [result]);
+    } else {
+      $scope.nodesManager.suppressTests($scope.node, [result]);
+    }
+  };
+
   // Destroy the NodeResultsManager when the scope is destroyed. This is
   // so the client will not recieve any more notifications about results
   // from this node.
diff --git a/src/maasserver/static/js/angular/controllers/tests/test_node_results.js b/src/maasserver/static/js/angular/controllers/tests/test_node_results.js
index 1839b07..c0712f2 100644
--- a/src/maasserver/static/js/angular/controllers/tests/test_node_results.js
+++ b/src/maasserver/static/js/angular/controllers/tests/test_node_results.js
@@ -4,6 +4,21 @@
  * Unit tests for NodeResultsController
  */
 
+// 2019-04-30 Caleb - Syntax error `import { ScriptStatus }from "../../enum"`;
+// TODO - Fix es module imports in test files
+const ScriptStatus = {
+  PENDING: 0,
+  RUNNING: 1,
+  PASSED: 2,
+  FAILED: 3,
+  TIMEDOUT: 4,
+  ABORTED: 5,
+  DEGRADED: 6,
+  INSTALLING: 7,
+  FAILED_INSTALLING: 8,
+  SKIPPED: 9
+};
+
 describe("NodeResultsController", function() {
   // Load the MAAS module.
   beforeEach(module("MAAS"));
@@ -511,4 +526,137 @@ describe("NodeResultsController", function() {
       expect($scope.nodeResultsManager.get_history).not.toHaveBeenCalled();
     });
   });
+
+  describe("hasSuppressedTests", () => {
+    it("returns whether there are suppressed tests in results", () => {
+      makeController();
+      const suppressedResult = makeResult(1);
+      $scope.results = [
+        {
+          hardware_type: 1,
+          results: {
+            subtype: [
+              ...Array.from(Array(3)).map(() => makeResult(1)),
+              suppressedResult
+            ]
+          }
+        },
+        {
+          hardware_type: 2,
+          results: {
+            subtype: Array.from(Array(3)).map(() => makeResult(2))
+          }
+        }
+      ];
+
+      expect($scope.hasSuppressedTests()).toEqual(false);
+
+      suppressedResult.suppressed = true;
+      expect($scope.hasSuppressedTests()).toEqual(true);
+    });
+  });
+
+  describe("isSuppressible", () => {
+    it(`returns true if a result's status is
+      FAILED, FAILED_INSTALLING or TIMEDOUT`, () => {
+      makeController();
+      const results = [
+        makeResult(0, ScriptStatus.FAILED),
+        makeResult(0, ScriptStatus.FAILED_INSTALLING),
+        makeResult(0, ScriptStatus.TIMEDOUT),
+        makeResult(0, ScriptStatus.PASSED)
+      ];
+
+      expect($scope.isSuppressible(results[0])).toBe(true);
+      expect($scope.isSuppressible(results[1])).toBe(true);
+      expect($scope.isSuppressible(results[2])).toBe(true);
+      expect($scope.isSuppressible(results[3])).toBe(false);
+    });
+  });
+
+  describe("getSuppressedCount", () => {
+    it("returns number of suppressed tests in node test results", () => {
+      makeController();
+      const results1 = Array.from(Array(5)).map((e, i) => {
+        const result = makeResult(1, ScriptStatus.FAILED);
+        if (i % 2 === 0) {
+          result.suppressed = true;
+        }
+        return result;
+      });
+      const results2 = Array.from(Array(5)).map((e, i) => {
+        const result = makeResult(2, ScriptStatus.FAILED);
+        if (i % 2 === 0) {
+          result.suppressed = true;
+        }
+        return result;
+      });
+      $scope.results = [
+        {
+          hardware_type: 1,
+          results: {
+            subtype: results1
+          }
+        },
+        {
+          hardware_type: 2,
+          results: {
+            subtype: results2
+          }
+        }
+      ];
+
+      expect($scope.getSuppressedCount()).toEqual(6);
+    });
+
+    it("returns 'All' if all suppressible tests are suppressed", () => {
+      makeController();
+      const results = Array.from(Array(5)).map((e, i) => {
+        const result = makeResult(2, ScriptStatus.FAILED);
+        result.suppressed = true;
+        return result;
+      });
+      $scope.results = [
+        {
+          hardware_type: 1,
+          results: {
+            subtype: results
+          }
+        }
+      ];
+
+      expect($scope.getSuppressedCount()).toEqual("All");
+    });
+  });
+
+  describe("toggleSuppressed", () => {
+    it("calls suppressTests manager method if test not suppressed", () => {
+      makeController();
+      const result = makeResult();
+      $scope.node = node;
+      spyOn($scope.nodesManager, "suppressTests");
+
+      $scope.toggleSuppressed(result);
+
+      expect($scope.nodesManager.suppressTests).toHaveBeenCalledWith(
+        $scope.node,
+        [result]
+      );
+    });
+
+    it("calls unsuppressTests manager method if test suppressed", () => {
+      makeController();
+      const result = makeResult();
+      result.suppressed = true;
+      $scope.node = node;
+      spyOn($scope.nodesManager, "unsuppressTests");
+
+      $scope.toggleSuppressed(result);
+
+      expect($scope.nodesManager.unsuppressTests).toHaveBeenCalledWith(
+        $scope.node,
+        [result]
+      );
+    });
+  });
 });
diff --git a/src/maasserver/static/js/angular/directives/machines_table.js b/src/maasserver/static/js/angular/directives/machines_table.js
index eed1f01..56666d2 100644
--- a/src/maasserver/static/js/angular/directives/machines_table.js
+++ b/src/maasserver/static/js/angular/directives/machines_table.js
@@ -6,7 +6,7 @@
  * Renders the machines listing.
  */
 
-import NodeStatus from "../enum";
+import { NodeStatus } from "../enum";
 
 /* @ngInject */
 function maasMachinesTable(
diff --git a/src/maasserver/static/js/angular/enum.js b/src/maasserver/static/js/angular/enum.js
index d791cb6..4502c76 100644
--- a/src/maasserver/static/js/angular/enum.js
+++ b/src/maasserver/static/js/angular/enum.js
@@ -1,4 +1,4 @@
-const NodeStatus = {
+export const NodeStatus = {
   // The node has been created and has a system ID assigned to it.
   NEW: 0,
   // Testing and other commissioning steps are taking place.
@@ -50,4 +50,15 @@ const NodeStatus = {
   FAILED_TESTING: 22
 };
 
-export default NodeStatus;
+export const ScriptStatus = {
+  PENDING: 0,
+  RUNNING: 1,
+  PASSED: 2,
+  FAILED: 3,
+  TIMEDOUT: 4,
+  ABORTED: 5,
+  DEGRADED: 6,
+  INSTALLING: 7,
+  FAILED_INSTALLING: 8,
+  SKIPPED: 9
+};
diff --git a/src/maasserver/static/js/angular/factories/nodes.js b/src/maasserver/static/js/angular/factories/nodes.js
index 5e812b7..a098c7a 100644
--- a/src/maasserver/static/js/angular/factories/nodes.js
+++ b/src/maasserver/static/js/angular/factories/nodes.js
@@ -382,6 +382,26 @@ function NodesManager(RegionConnection, Manager, KVMDeployOSBlacklist) {
     return true;
   };
 
+  NodesManager.prototype.suppressTests = function(node, scripts) {
+    return RegionConnection.callMethod(
+      this._handler + ".set_script_result_suppressed",
+      {
+        system_id: node.system_id,
+        script_result_ids: scripts.map(script => script.id)
+      }
+    );
+  };
+
+  NodesManager.prototype.unsuppressTests = function(node, scripts) {
+    return RegionConnection.callMethod(
+      this._handler + ".set_script_result_unsuppressed",
+      {
+        system_id: node.system_id,
+        script_result_ids: scripts.map(script => script.id)
+      }
+    );
+  };
+
   return NodesManager;
 }
 
diff --git a/src/maasserver/static/js/angular/factories/tests/test_nodes.js b/src/maasserver/static/js/angular/factories/tests/test_nodes.js
index c74a9e4..b0b067d 100644
--- a/src/maasserver/static/js/angular/factories/tests/test_nodes.js
+++ b/src/maasserver/static/js/angular/factories/tests/test_nodes.js
@@ -797,4 +797,46 @@ describe("NodesManager", function() {
       });
     });
   });
+
+  describe("suppressTests", () => {
+    it("calls machine.set_script_result_suppressed for script list", done => {
+      const machine = makemachine();
+      const scripts = [{ id: makeName("id") }, { id: makeName("id") }];
+
+      webSocket.returnData.push(makeFakeResponse("updated"));
+
+      MachinesManager.suppressTests(machine, scripts).then(() => {
+        const sentObject = angular.fromJson(webSocket.sentData[0]);
+        expect(sentObject.method).toBe("machine.set_script_result_suppressed");
+        expect(sentObject.params.system_id).toBe(machine.system_id);
+        expect(sentObject.params.script_result_ids).toEqual([
+          scripts[0].id,
+          scripts[1].id
+        ]);
+        done();
+      });
+    });
+  });
+
+  describe("unsuppressTests", () => {
+    it("calls machine.set_script_result_unsuppressed for script list", done => {
+      const machine = makemachine();
+      const scripts = [{ id: makeName("id") }, { id: makeName("id") }];
+
+      webSocket.returnData.push(makeFakeResponse("updated"));
+
+      MachinesManager.unsuppressTests(machine, scripts).then(() => {
+        const sentObject = angular.fromJson(webSocket.sentData[0]);
+        expect(sentObject.method).toBe(
+          "machine.set_script_result_unsuppressed"
+        );
+        expect(sentObject.params.system_id).toBe(machine.system_id);
+        expect(sentObject.params.script_result_ids).toEqual([
+          scripts[0].id,
+          scripts[1].id
+        ]);
+        done();
+      });
+    });
+  });
 });
diff --git a/src/maasserver/static/partials/script-results-list.html b/src/maasserver/static/partials/script-results-list.html
index 0246116..f968d48 100644
--- a/src/maasserver/static/partials/script-results-list.html
+++ b/src/maasserver/static/partials/script-results-list.html
@@ -4,6 +4,16 @@
             <p class="u-text--loading"><i class="p-icon--spinner u-animation--spin"></i>&nbsp;&nbsp;Loading...</p>
         </div>
     </div>
+    <div class="row" data-ng-if="resultsLoaded && hasSuppressedTests()">
+        <div class="p-notification--caution">
+            <p class="p-notification__response">
+                <span class="p-notification__status">
+                    {$ getSuppressedCount() $} test failure{$ getSuppressedCount() === 1 ? "" : "s" $} suppressed:
+                </span>
+                <span>suppressed results are not shown on the <em>Machine list</em>, but are always shown here.</span>
+            </p>
+        </div>
+    </div>
     <div class="row">
         <div data-ng-repeat="hardware_type in results">
             <div data-ng-if="resultsLoaded && (hardware_type.results | json) != '{}'">
@@ -13,6 +23,7 @@
                     <table class="p-table-expanding p-table--controllers-commissioning">
                         <thead>
                             <tr>
+                                <th style="width: 80px">Suppress</th>
                                 <th class="col-3">Name</th>
                                 <th class="col-2">Tags</th>
                                 <th class="col-1">Runtime</th>
@@ -22,7 +33,27 @@
                             </tr>
                         </thead>
                         <tbody>
-                            <tr data-ng-repeat="result in results" data-ng-class="{'is-active': result.showing_results || result.showing_history}">
+                            <tr
+                                data-ng-repeat="result in results"
+                                data-ng-class="{
+                                    'is-active': result.showing_results || result.showing_history,
+                                    'is-suppressed': result.suppressed
+                                }"
+                            >
+                                <td style="width: 80px; overflow: visible" aria-label="Suppress"">
+                                    <div class="p-tooltip">
+                                        <input
+                                            id="{$ result.id $}"
+                                            type="checkbox"
+                                            data-ng-if="isSuppressible(result)"
+                                            data-ng-checked="result.suppressed"
+                                            data-ng-click="toggleSuppressed(result)"
+                                            data-ng-disabled="!isSuperUser()"
+                                        >
+                                        <label style="height: 2rem" for="{$ result.id $}" data-ng-if="result.exit_status !== 0"></label>
+                                        <span style="left: -1rem" data-ng-if="!isSuperUser()" class="p-tooltip__message">Only admins can suppress failed tests.</span>
+                                    </div>
+                                </td>
                                 <td class="col-3" data-ng-click="result.showing_results = !result.showing_results" aria-label="Name" title="{$ result.name $}">
                                     <span data-maas-script-status="script-status" data-script-status="result.status"></span>
                                     {$ result.name $}
@@ -42,11 +73,12 @@
                                             <button class="p-button--base is-small p-contextual-menu__toggle" data-ng-click="toggleMenu()">
                                                 <i class="p-icon--contextual-menu u-no-margin--right">Actions</i>
                                             </button>
-                                            <div class="p-contextual-menu__dropdown" role="menu" data-ng-show="isToggled">
+                                            <div class="p-contextual-menu__dropdown" role="menu" data-ng-show="isToggled" style="width: auto">
                                                 <button class="p-contextual-menu__link" aria-label="View metrics" data-ng-if="!result.showing_results && result.results.length !== 0" data-ng-click="toggleMenu(); result.showing_history = false; result.showing_results = true">View metrics</button>
                                                 <button class="p-contextual-menu__link" aria-label="Hide metrics" data-ng-if="result.showing_results" data-ng-click="toggleMenu(); result.showing_history = false; result.showing_results = false">Hide metrics</button>
                                                 <button class="p-contextual-menu__link" aria-label="View previous {$ result.result_section $}" data-ng-if="!result.showing_history" data-ng-click="toggleMenu(); loadHistory(result)">View previous {$ result.result_section $}</button>
                                                 <button class="p-contextual-menu__link" aria-label="Hide previous {$ result.result_section $}" data-ng-if="result.showing_history" data-ng-click="toggleMenu(); result.showing_results = false; result.showing_history = false">Hide previous {$ result.result_section $}</button>
+                                                <button class="p-contextual-menu__link" aria-label="Toggle this test in machine list" data-ng-if="isSuperUser() && isSuppressible(result)" data-ng-click="toggleSuppressed(result)">{$ result.suppressed ? "Show" : "Suppress" $} test in machine list</button>
                                             </div>
                                         </div>
                                     </div>
diff --git a/src/maasserver/static/scss/_base_tables.scss b/src/maasserver/static/scss/_base_tables.scss
index 657439e..710e416 100644
--- a/src/maasserver/static/scss/_base_tables.scss
+++ b/src/maasserver/static/scss/_base_tables.scss
@@ -61,6 +61,16 @@
       background-color: $color-x-light;
     }
 
+    &.is-suppressed {
+      > td:nth-child(2),
+      > td:nth-child(3),
+      > td:nth-child(4),
+      > td:nth-child(5),
+      > td:nth-child(6) {
+        opacity: 0.5;
+      }
+    }
+
     thead & {
       border-bottom-color: $color-mid-light;
     }
diff --git a/src/maasserver/websockets/handlers/controller.py b/src/maasserver/websockets/handlers/controller.py
index 9acd874..609c5e9 100644
--- a/src/maasserver/websockets/handlers/controller.py
+++ b/src/maasserver/websockets/handlers/controller.py
@@ -73,6 +73,7 @@ class ControllerHandler(MachineHandler):
             'get_summary_xml',
             'get_summary_yaml',
             'set_script_result_suppressed',
+            'set_script_result_unsuppressed',
             'get_suppressible_script_results',
         ]
         form = ControllerForm
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index a6469d8..ebba122 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -196,6 +196,7 @@ class MachineHandler(NodeHandler):
             'get_summary_xml',
             'get_summary_yaml',
             'set_script_result_suppressed',
+            'set_script_result_unsuppressed',
             'get_suppressible_script_results',
         ]
         form = AdminMachineWithMACAddressesForm
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index c344577..bf93dc8 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -34,6 +34,7 @@ from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.tag import Tag
 from maasserver.models.virtualblockdevice import VirtualBlockDevice
 from maasserver.node_action import compile_node_actions
+from maasserver.permissions import NodePermission
 from maasserver.storage_layouts import get_applied_storage_layout_for_node
 from maasserver.third_party_drivers import get_third_party_driver
 from maasserver.utils.converters import (
@@ -44,6 +45,7 @@ from maasserver.utils.osystems import make_hwe_kernel_ui_text
 from maasserver.websockets.base import (
     dehydrate_datetime,
     HandlerError,
+    HandlerPermissionError,
 )
 from maasserver.websockets.handlers.event import dehydrate_event_type_level
 from maasserver.websockets.handlers.node_result import NodeResultHandler
@@ -897,10 +899,22 @@ class NodeHandler(TimestampedModelHandler):
 
     def set_script_result_suppressed(self, params):
         """Set suppressed for the ScriptResult ids."""
+        node = self.get_object(params)
+        if not self.user.has_perm(NodePermission.admin, node):
+            raise HandlerPermissionError()
         script_result_ids = params.get('script_result_ids')
         ScriptResult.objects.filter(id__in=script_result_ids).update(
             suppressed=True)
 
+    def set_script_result_unsuppressed(self, params):
+        """Set unsuppressed for the ScriptResult ids."""
+        node = self.get_object(params)
+        if not self.user.has_perm(NodePermission.admin, node):
+            raise HandlerPermissionError()
+        script_result_ids = params.get('script_result_ids')
+        ScriptResult.objects.filter(id__in=script_result_ids).update(
+            suppressed=False)
+
     def get_suppressible_script_results(self, params):
         """Return a dictionary with Nodes system_ids mapped to lists of
         ScriptResults that can still be suppressed."""
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 5257e2a..916a8b1 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -4286,7 +4286,7 @@ class TestMachineHandlerUpdateFilesystem(MAASServerTestCase):
         self.assertEqual(blockdevice.tags, [])
 
     def test__set_script_result_suppressed(self):
-        owner = factory.make_User()
+        owner = factory.make_admin()
         node = factory.make_Node(owner=owner)
         handler = MachineHandler(owner, {}, None)
         hardware_type = factory.pick_choice(HARDWARE_TYPE_CHOICES)
@@ -4310,6 +4310,72 @@ class TestMachineHandlerUpdateFilesystem(MAASServerTestCase):
             script_result = reload_object(script_result)
             self.assertTrue(script_result.suppressed)
 
+    def test__set_script_result_suppressed_raises_permission_error(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        handler = MachineHandler(owner, {}, None)
+        script_set = factory.make_ScriptSet(node=node)
+        script_results = []
+        for _ in range(3):
+            script_results.append(
+                factory.make_ScriptResult(script_set=script_set))
+        params = {
+            'system_id': node.system_id,
+            'script_result_ids': [
+                script_result.id for script_result in script_results]
+        }
+
+        self.assertRaises(
+            HandlerPermissionError,
+            handler.set_script_result_suppressed,
+            params)
+
+    def test__set_script_result_unsuppressed(self):
+        owner = factory.make_admin()
+        node = factory.make_Node(owner=owner)
+        handler = MachineHandler(owner, {}, None)
+        hardware_type = factory.pick_choice(HARDWARE_TYPE_CHOICES)
+        script_set = factory.make_ScriptSet(node=node)
+        script = factory.make_Script(hardware_type=hardware_type)
+        script_result = factory.make_ScriptResult(
+            script_set, script=script, suppressed=True)
+
+        script_results = []
+        for _ in range(3):
+            script_results.append(
+                factory.make_ScriptResult(script_set=script_set))
+
+        handler.set_script_result_unsuppressed({
+            'system_id': node.system_id,
+            'script_result_ids': [
+                script_result.id for script_result in script_results]
+        })
+        script_result = reload_object(script_result)
+        self.assertTrue(script_result.suppressed)
+        for script_result in script_results:
+            script_result = reload_object(script_result)
+            self.assertFalse(script_result.suppressed)
+
+    def test__set_script_result_unsuppressed_raises_permission_error(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        handler = MachineHandler(owner, {}, None)
+        script_set = factory.make_ScriptSet(node=node)
+        script_results = []
+        for _ in range(3):
+            script_results.append(
+                factory.make_ScriptResult(script_set=script_set))
+        params = {
+            'system_id': node.system_id,
+            'script_result_ids': [
+                script_result.id for script_result in script_results]
+        }
+
+        self.assertRaises(
+            HandlerPermissionError,
+            handler.set_script_result_unsuppressed,
+            params)
+
     def test__get_unsuppressed_script_results(self):
         owner = factory.make_User()
         handler = MachineHandler(owner, {}, None)