From e52e0747ad1b2b10dd76c67f34cfc57712e46caf Mon Sep 17 00:00:00 2001
From: RachelElysia <71795832+RachelElysia@users.noreply.github.com>
Date: Fri, 4 Jun 2021 15:13:59 -0400
Subject: [PATCH] Query Edit/Run: Conditional select targets dropdown (#923)

* Modify targets endpoint to  use queryId
* Conditionally render query page including queryId
* Includes conditionally renders target dropdown

Co-authored by: Sarah Gillespie @gillespi314
Test mods co-authored by: Gabriel Hernandez @ghernandez345
---
 .../SelectTargetsDropdown.jsx                 |  9 ++-
 .../SelectTargetsDropdown.tests.jsx           | 24 +-------
 .../QueryPageSelectTargets.jsx                |  3 +
 frontend/kolide/entities/targets.js           |  4 +-
 frontend/kolide/entities/targets.tests.js     | 11 ++--
 .../pages/queries/QueryPage/QueryPage.jsx     | 59 +++++++++++++------
 frontend/pages/queries/QueryPage/helpers.js   | 17 +++++-
 frontend/test/mocks/target_mocks.js           |  3 +-
 frontend/test/target_mock.js                  |  1 +
 9 files changed, 80 insertions(+), 51 deletions(-)

diff --git a/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.jsx b/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.jsx
index ae33397d8e..ca12077450 100644
--- a/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.jsx
+++ b/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.jsx
@@ -20,6 +20,7 @@ class SelectTargetsDropdown extends Component {
     onSelect: PropTypes.func.isRequired,
     selectedTargets: PropTypes.arrayOf(targetInterface),
     targetsCount: PropTypes.number,
+    queryId: PropTypes.number,
   };
 
   static defaultProps = {
@@ -118,7 +119,11 @@ class SelectTargetsDropdown extends Component {
     this.setState({ moreInfoTarget: null });
   };
 
-  fetchTargets = (query = "", selectedTargets = this.props.selectedTargets) => {
+  fetchTargets = (
+    query = "",
+    queryId = this.props.queryId,
+    selectedTargets = this.props.selectedTargets
+  ) => {
     const { onFetchTargets } = this.props;
 
     if (!this.mounted) {
@@ -128,7 +133,7 @@ class SelectTargetsDropdown extends Component {
     this.setState({ isLoadingTargets: true, query });
 
     return Kolide.targets
-      .loadAll(query, formatSelectedTargetsForApi(selectedTargets))
+      .loadAll(query, queryId, formatSelectedTargetsForApi(selectedTargets))
       .then((response) => {
         const { targets } = response;
         const isEmpty = targets.length === 0;
diff --git a/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.tests.jsx b/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.tests.jsx
index 9f20eed573..3bb153308f 100644
--- a/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.tests.jsx
+++ b/frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsDropdown.tests.jsx
@@ -16,6 +16,7 @@ describe("SelectTargetsDropdown - component", () => {
     onSelect: noop,
     selectedTargets: [],
     targetsCount: 0,
+    queryId: 1,
   };
   afterEach(() => nock.cleanAll());
 
@@ -111,34 +112,13 @@ describe("SelectTargetsDropdown - component", () => {
     const defaultSelectedTargets = { hosts: [], labels: [] };
     const defaultParams = {
       query: "",
+      query_id: 1,
       selected: defaultSelectedTargets,
     };
     const expectedApiClientResponseWithTargets = {
       targets: [{ ...Test.Stubs.labelStub, target_type: "labels" }],
     };
 
-    it("calls the api", () => {
-      Test.Mocks.targetMock(defaultParams, apiResponseWithTargets);
-      const Component = shallow(<SelectTargetsDropdown {...defaultProps} />);
-      const node = Component.instance();
-
-      nock.cleanAll();
-      const request = Test.Mocks.targetMock(
-        defaultParams,
-        apiResponseWithTargets
-      );
-
-      expect.assertions(1);
-      return node
-        .fetchTargets()
-        .then(() => {
-          expect(request.isDone()).toEqual(true);
-        })
-        .catch((error) => {
-          expect(error).toBe(undefined);
-        });
-    });
-
     it("calls the onFetchTargets prop", () => {
       const onFetchTargets = jest.fn();
       const props = { ...defaultProps, onFetchTargets };
diff --git a/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx b/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx
index d4077d969f..da36115e59 100644
--- a/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx
+++ b/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx
@@ -21,6 +21,7 @@ class QueryPageSelectTargets extends Component {
     targetsCount: PropTypes.number,
     queryTimerMilliseconds: PropTypes.number,
     disableRun: PropTypes.bool,
+    queryId: PropTypes.number,
   };
 
   render() {
@@ -36,6 +37,7 @@ class QueryPageSelectTargets extends Component {
       queryIsRunning,
       queryTimerMilliseconds,
       disableRun,
+      queryId,
     } = this.props;
 
     return (
@@ -47,6 +49,7 @@ class QueryPageSelectTargets extends Component {
           selectedTargets={selectedTargets}
           targetsCount={targetsCount}
           label="Select targets"
+          queryId={queryId}
         />
         <QueryProgressDetails
           campaign={campaign}
diff --git a/frontend/kolide/entities/targets.js b/frontend/kolide/entities/targets.js
index a434a3276c..8b9641a22e 100644
--- a/frontend/kolide/entities/targets.js
+++ b/frontend/kolide/entities/targets.js
@@ -8,13 +8,13 @@ const defaultSelected = {
 
 export default (client) => {
   return {
-    loadAll: (query, selected = defaultSelected) => {
+    loadAll: (query = "", queryId = null, selected = defaultSelected) => {
       const { TARGETS } = endpoints;
 
       return client
         .authenticatedPost(
           client._endpoint(TARGETS),
-          JSON.stringify({ query, selected })
+          JSON.stringify({ query, query_id: queryId, selected })
         )
         .then((response) => {
           const { targets } = response;
diff --git a/frontend/kolide/entities/targets.tests.js b/frontend/kolide/entities/targets.tests.js
index 0a50d33b20..cdb0ed70d2 100644
--- a/frontend/kolide/entities/targets.tests.js
+++ b/frontend/kolide/entities/targets.tests.js
@@ -19,12 +19,15 @@ describe("Kolide - API client (targets)", () => {
       const hosts = [];
       const labels = [];
       const query = "mac";
-      const request = targetMocks.loadAll.valid(bearerToken, query);
+      const queryId = 1;
+      const request = targetMocks.loadAll.valid(bearerToken, query, queryId);
 
       Kolide.setBearerToken(bearerToken);
-      return Kolide.targets.loadAll(query, { hosts, labels }).then(() => {
-        expect(request.isDone()).toEqual(true);
-      });
+      return Kolide.targets
+        .loadAll(query, queryId, { hosts, labels })
+        .then(() => {
+          expect(request.isDone()).toEqual(true);
+        });
     });
   });
 });
diff --git a/frontend/pages/queries/QueryPage/QueryPage.jsx b/frontend/pages/queries/QueryPage/QueryPage.jsx
index 5f3ccc5ebd..5d05fac242 100644
--- a/frontend/pages/queries/QueryPage/QueryPage.jsx
+++ b/frontend/pages/queries/QueryPage/QueryPage.jsx
@@ -70,7 +70,6 @@ export class QueryPage extends Component {
     requestHost: PropTypes.bool,
     hostId: PropTypes.string,
     currentUser: userInterface,
-    queryID: PropTypes.number,
   };
 
   static defaultProps = {
@@ -608,6 +607,7 @@ export class QueryPage extends Component {
       liveQueryError,
     } = this.state;
     const { selectedTargets } = this.props;
+    const queryId = this.props.query.id;
 
     return (
       <QueryPageSelectTargets
@@ -622,6 +622,7 @@ export class QueryPage extends Component {
         targetsCount={targetsCount}
         queryTimerMilliseconds={runQueryMilliseconds}
         disableRun={liveQueryError !== undefined}
+        queryId={queryId}
       />
     );
   };
@@ -647,8 +648,10 @@ export class QueryPage extends Component {
       selectedOsqueryTable,
       title,
       currentUser,
-      queryID,
     } = this.props;
+    const { hasSavePermissions, showDropdown } = helpers;
+
+    const queryId = this.props.query.id;
 
     if (loadingQueries) {
       return false;
@@ -684,12 +687,34 @@ export class QueryPage extends Component {
       );
     };
 
-    // If team maintainer, can create and run new query, but not save
-    const hasSavePermissions =
-      permissionUtils.isGlobalAdmin(currentUser) ||
-      permissionUtils.isGlobalMaintainer(currentUser);
+    // Team maintainer: Create and run new query, but not save
+    if (permissionUtils.isAnyTeamMaintainer(currentUser)) {
+      // Team maintainer: Existing query
+      if (queryId) {
+        return (
+          <div className={`${baseClass}__content`}>
+            <div className={`${baseClass}__observer-query-view body-wrap`}>
+              <div className={`${baseClass}__observer-query-details`}>
+                <Link
+                  to={PATHS.MANAGE_QUERIES}
+                  className={`${baseClass}__back-link`}
+                >
+                  <img src={BackChevron} alt="back chevron" id="back-chevron" />
+                  <span>Back to queries</span>
+                </Link>
+                <h1>{query.name}</h1>
+                <p>{query.description}</p>
+                {editDisabledSql()}
+              </div>
+              {renderLiveQueryWarning()}
+              {renderTargetsInput()}
+              {renderResultsTable()}
+            </div>
+          </div>
+        );
+      }
 
-    if (permissionUtils.isAnyTeamMaintainer(currentUser) && !queryID) {
+      // Team maintainer: New query
       return (
         <div className={`${baseClass} has-sidebar`}>
           <div className={`${baseClass}__content`}>
@@ -713,7 +738,7 @@ export class QueryPage extends Component {
                 serverErrors={errors}
                 selectedOsqueryTable={selectedOsqueryTable}
                 title={title}
-                hasSavePermissions={hasSavePermissions}
+                hasSavePermissions={hasSavePermissions(currentUser)}
               />
             </div>
             {renderLiveQueryWarning()}
@@ -729,15 +754,11 @@ export class QueryPage extends Component {
       );
     }
 
-    // TODO: Modify observerCanQueryHostCount to reflect all hosts user can query
-    // This will depend on 5/26 API changes, if it comes back as 0, we will not render the dropdown for observers
-    const observerCanQueryHostCount = 1;
-
+    // Global Observer or Team Maintainer or Team Observer: Restricted UI
     if (
       permissionUtils.isGlobalObserver(currentUser) ||
       !permissionUtils.isOnGlobalTeam(currentUser)
     ) {
-      // Restricted UI for Global Observer or Team Maintainer or Team Observer
       return (
         <div className={`${baseClass}__content`}>
           <div className={`${baseClass}__observer-query-view body-wrap`}>
@@ -753,7 +774,7 @@ export class QueryPage extends Component {
               <p>{query.description}</p>
               {editDisabledSql()}
             </div>
-            {observerCanQueryHostCount > 0 && (
+            {showDropdown(query, currentUser) && (
               <div>
                 {renderLiveQueryWarning()}
                 {renderTargetsInput()}
@@ -765,7 +786,7 @@ export class QueryPage extends Component {
       );
     }
 
-    // UI for Global Admin and Global Maintainer
+    // Global Admin or Global Maintainer: Full functionality
     return (
       <div className={`${baseClass} has-sidebar`}>
         <div className={`${baseClass}__content`}>
@@ -789,7 +810,7 @@ export class QueryPage extends Component {
               serverErrors={errors}
               selectedOsqueryTable={selectedOsqueryTable}
               title={title}
-              hasSavePermissions={hasSavePermissions}
+              hasSavePermissions={hasSavePermissions(currentUser)}
             />
           </div>
           {renderLiveQueryWarning()}
@@ -808,13 +829,13 @@ export class QueryPage extends Component {
 
 const mapStateToProps = (state, ownProps) => {
   const stateEntities = entityGetter(state);
-  const { id: queryID } = ownProps.params;
-  const query = entityGetter(state).get("queries").findBy({ id: queryID });
+  const { id: queryId } = ownProps.params;
+  const query = entityGetter(state).get("queries").findBy({ id: queryId });
   const { selectedOsqueryTable } = state.components.QueryPages;
   const { errors, loading: loadingQueries } = state.entities.queries;
   const { selectedTargets } = state.components.QueryPages;
   const { host_ids: hostIDs, host_uuids: hostUUIDs } = ownProps.location.query;
-  const title = queryID ? "Edit & run query" : "Custom query";
+  const title = queryId ? "Edit & run query" : "Custom query";
   let selectedHosts = [];
 
   if (((hostIDs && hostIDs.length) || (hostUUIDs && hostUUIDs.length)) > 0) {
diff --git a/frontend/pages/queries/QueryPage/helpers.js b/frontend/pages/queries/QueryPage/helpers.js
index 7d49afc149..4591d0c91b 100644
--- a/frontend/pages/queries/QueryPage/helpers.js
+++ b/frontend/pages/queries/QueryPage/helpers.js
@@ -2,6 +2,7 @@ import { push } from "react-router-redux";
 import PATHS from "router/paths";
 import { differenceWith, isEqual, uniqWith } from "lodash";
 
+import permissionUtils from "utilities/permissions";
 import hostActions from "redux/nodes/entities/hosts/actions";
 import { setSelectedTargets } from "redux/nodes/components/QueryPages/actions";
 
@@ -44,4 +45,18 @@ export const fetchHost = (dispatch, hostID) => {
   });
 };
 
-export default { selectHosts, fetchHost };
+export const showDropdown = (query, currentUser) => {
+  if (query.observer_can_run) {
+    return true;
+  }
+  return !permissionUtils.isOnlyObserver(currentUser);
+};
+
+export const hasSavePermissions = (currentUser) => {
+  return (
+    permissionUtils.isGlobalAdmin(currentUser) ||
+    permissionUtils.isGlobalMaintainer(currentUser)
+  );
+};
+
+export default { selectHosts, fetchHost, showDropdown, hasSavePermissions };
diff --git a/frontend/test/mocks/target_mocks.js b/frontend/test/mocks/target_mocks.js
index 9717c16c81..a5f2dd570a 100644
--- a/frontend/test/mocks/target_mocks.js
+++ b/frontend/test/mocks/target_mocks.js
@@ -2,13 +2,14 @@ import createRequestMock from "test/mocks/create_request_mock";
 
 export default {
   loadAll: {
-    valid: (bearerToken, query) => {
+    valid: (bearerToken, query, queryId) => {
       return createRequestMock({
         bearerToken,
         endpoint: "/api/v1/fleet/targets",
         method: "post",
         params: {
           query,
+          query_id: queryId,
           selected: {
             hosts: [],
             labels: [],
diff --git a/frontend/test/target_mock.js b/frontend/test/target_mock.js
index f56960cf05..165590122a 100644
--- a/frontend/test/target_mock.js
+++ b/frontend/test/target_mock.js
@@ -2,6 +2,7 @@ import nock from "nock";
 
 const defaultParams = {
   query: "",
+  query_id: 1,
   selected: {
     hosts: [],
     labels: [],