Fix loading indicator on select targets pages (#3408)

changes/issue-3299-fix-select-targets-loading

@@ -0,0 +1 @@
+* Fix loading indicator not appearing when cancelling and returning to target selection

cypress/integration/all/app/queryflow.spec.ts

@@ -12,23 +12,19 @@ describe(
     it("Create, check, edit, and delete a query successfully and create, edit, and delete a global scheduled query successfully", () => {
       cy.visit("/queries/manage");
 
-      // cy.findByRole("button", { name: /create new query/i }).click();
-      cy.wait(2000); // eslint-disable-line cypress/no-unnecessary-waiting
-      cy.get(".queries-list-wrapper__create-button").click();
+      cy.getAttached(".queries-list-wrapper__create-button").click();
 
-      // Using class selector because third party element doesn't work with Cypress Testing Selector Library
-      cy.get(".ace_scroller")
+      cy.getAttached(".ace_scroller")
         .click({ force: true })
         .type("{selectall}SELECT * FROM windows_crashes;");
 
       cy.findByRole("button", { name: /save/i }).click();
 
-      // save modal
-      cy.get(".query-form__query-save-modal-name")
+      cy.getAttached(".query-form__query-save-modal-name")
         .click()
         .type("Query all window crashes");
 
-      cy.get(".query-form__query-save-modal-description")
+      cy.getAttached(".query-form__query-save-modal-description")
         .click()
         .type("See all window crashes");
 
@@ -37,43 +33,33 @@ describe(
       cy.findByText(/query created/i).should("exist");
       cy.findByText(/back to queries/i).should("exist");
       cy.visit("/queries/manage");
+      cy.getAttached(".name__cell .button--text-link").first().click();
 
-      cy.wait(3000); // eslint-disable-line cypress/no-unnecessary-waiting
-      cy.findByText(/query all/i).click();
-
-      cy.wait(2000); // eslint-disable-line cypress/no-unnecessary-waiting
       cy.findByText(/run query/i).should("exist");
 
-      cy.get(".ace_scroller")
-        .click({ force: true })
+      cy.getAttached(".ace_scroller")
+        .click()
         .type("{selectall}SELECT datetime, username FROM windows_crashes;");
 
-      cy.findByRole("button", { name: /^Save$/ }).click();
+      cy.getAttached(".button--brand.query-form__save").click();
 
       cy.findByText(/query updated/i).should("be.visible");
 
-      // E2e Test for schedules moved to premium/admin
-
       cy.visit("/queries/manage");
 
       cy.findByText(/query all window crashes/i)
         .parent()
         .parent()
         .within(() => {
-          cy.get(".fleet-checkbox__input").check({ force: true });
+          cy.getAttached(".fleet-checkbox__input").check({ force: true });
         });
 
       cy.findByRole("button", { name: /delete/i }).click();
 
-      // Can't figure out how attach findByRole onto modal button
-      // Can't use findByText because delete button under modal
-      cy.get(".remove-query-modal")
-        .contains("button", /delete/i)
-        .click();
+      cy.getAttached(".button--alert.remove-query-modal__btn").click();
 
       cy.findByText(/successfully removed query/i).should("be.visible");
-
-      cy.findByText(/query all/i).should("not.exist");
+      cy.get(".name__cell .button--text-link").should("not.exist");
     });
   }
 );

cypress/support/commands.ts

@@ -305,3 +305,20 @@ Cypress.Commands.add("clearDownloads", () => {
     cy.exec(`rm -rf ${Cypress.config("downloadsFolder")}`, { env: { SHELL } });
   }
 });
+
+Cypress.Commands.add("getAttached", (selector) => {
+  const uniqueAlias = `element_${selector}`;
+
+  return cy
+    .waitUntil(
+      () =>
+        // eslint-disable-next-line cypress/no-unnecessary-waiting
+        cy
+          .get(selector)
+          .as(uniqueAlias)
+          .wait(1)
+          .then(($el) => Cypress.dom.isAttached($el)),
+      { timeout: 1000, interval: 10 }
+    )
+    .get(`@${uniqueAlias}`);
+});

frontend/pages/policies/PolicyPage/screens/SelectTargets.tsx

@@ -92,7 +92,7 @@ const SelectTargets = ({
   const [searchText, setSearchText] = useState<string>("");
   const [relatedHosts, setRelatedHosts] = useState<IHost[]>([]);
 
-  const { isLoading: isTargetsLoading, isError: isTargetsError } = useQuery(
+  const { isFetching: isTargetsFetching, isError: isTargetsError } = useQuery(
     // triggers query on change
     ["targetsFromSearch", searchText, [...selectedTargets]],
     () =>
@@ -252,7 +252,7 @@ const SelectTargets = ({
     </>
   );
 
-  if (isEmpty(searchText) && isTargetsLoading) {
+  if (isEmpty(searchText) && isTargetsFetching) {
     return (
       <div className={`${baseClass}__wrapper body-wrap`}>
         <h1>Select targets</h1>
@@ -308,7 +308,7 @@ const SelectTargets = ({
         tabIndex={inputTabIndex}
         searchText={searchText}
         relatedHosts={[...relatedHosts]}
-        isTargetsLoading={isTargetsLoading}
+        isTargetsLoading={isTargetsFetching}
         selectedTargets={[...selectedTargets]}
         hasFetchError={isTargetsError}
         setSearchText={setSearchText}

frontend/pages/queries/ManageQueriesPage/ManageQueriesPage.tsx

@@ -107,8 +107,6 @@ const ManageQueriesPage = (): JSX.Element => {
     "fleet queries by platform",
     () => fleetQueriesAPI.loadAll(),
     {
-      // refetchOnMount: false,
-      // refetchOnReconnect: false,
       refetchOnWindowFocus: false,
       select: (data: IFleetQueriesResponse) => data.queries,
     }
@@ -140,51 +138,33 @@ const ManageQueriesPage = (): JSX.Element => {
     setSelectedQueryIds(selectedTableQueryIds);
   };
 
-  const onRemoveQuerySubmit = useCallback(() => {
+  const onRemoveQuerySubmit = useCallback(async () => {
     const queryOrQueries = selectedQueryIds.length === 1 ? "query" : "queries";
 
-    const promises = selectedQueryIds.map((id: number) => {
-      fleetQueriesAPI.destroy(id);
-      return null;
-    });
+    const removeQueries = selectedQueryIds.map((id) =>
+      fleetQueriesAPI.destroy(id)
+    );
 
-    return Promise.all(promises)
-      .then(() => {
-        dispatch(
-          renderFlash("success", `Successfully removed ${queryOrQueries}.`)
-        );
-        toggleRemoveQueryModal();
-      })
-      .catch((response) => {
-        if (
-          response?.errors?.filter((error: Record<string, string>) =>
-            error.reason?.includes(
-              "the operation violates a foreign key constraint"
-            )
-          ).length
-        ) {
-          dispatch(
-            renderFlash(
-              "error",
-              `Could not delete query because this query is used as a policy. First remove the policy and then try deleting the query again.`
-            )
-          );
-        } else {
-          dispatch(
-            renderFlash(
-              "error",
-              `Unable to remove ${queryOrQueries}. Please try again.`
-            )
-          );
-        }
-      })
-      .finally(() => {
-        refetchFleetQueries();
-        // TODO: Delete this redux action after redux dependencies have been removed (e.g., schedules page
-        // depends on redux state for queries).
-        dispatch(queryActions.loadAll());
-        toggleRemoveQueryModal();
-      });
+    try {
+      await Promise.all(removeQueries);
+      renderFlash("success", `Successfully removed ${queryOrQueries}.`);
+      toggleRemoveQueryModal();
+      refetchFleetQueries();
+      toggleRemoveQueryModal();
+      dispatch(
+        renderFlash("success", `Successfully removed ${queryOrQueries}.`)
+      );
+      // TODO: Delete this redux action after redux dependencies have been removed (e.g., schedules page
+      // depends on redux state for queries).
+      dispatch(queryActions.loadAll());
+    } catch (errorResponse) {
+      dispatch(
+        renderFlash(
+          "error",
+          `There was an error removing your ${queryOrQueries}. Please try again later.`
+        )
+      );
+    }
   }, [dispatch, refetchFleetQueries, selectedQueryIds, toggleRemoveQueryModal]);
 
   const renderPlatformDropdown = () => {

frontend/pages/queries/QueryPage/screens/SelectTargets.tsx

@@ -98,7 +98,7 @@ const SelectTargets = ({
   goToQueryEditor,
   goToRunQuery,
   setSelectedTargets,
-}: ISelectTargetsProps) => {
+}: ISelectTargetsProps): JSX.Element => {
   const [targetsTotalCount, setTargetsTotalCount] = useState<number | null>(
     null
   );
@@ -114,7 +114,7 @@ const SelectTargets = ({
   const [searchText, setSearchText] = useState<string>("");
   const [relatedHosts, setRelatedHosts] = useState<IHost[]>([]);
 
-  const { isLoading: isTargetsLoading, isError: isTargetsError } = useQuery(
+  const { isFetching: isTargetsFetching, isError: isTargetsError } = useQuery(
     // triggers query on change
     ["targetsFromSearch", searchText, [...selectedTargets]],
     () =>
@@ -264,7 +264,7 @@ const SelectTargets = ({
     );
   };
 
-  if (isEmpty(searchText) && isTargetsLoading) {
+  if (isEmpty(searchText) && isTargetsFetching) {
     return (
       <div className={`${baseClass}__wrapper body-wrap`}>
         <h1>Select targets</h1>
@@ -320,7 +320,7 @@ const SelectTargets = ({
         tabIndex={inputTabIndex}
         searchText={searchText}
         relatedHosts={[...relatedHosts]}
-        isTargetsLoading={isTargetsLoading}
+        isTargetsLoading={isTargetsFetching}
         selectedTargets={[...selectedTargets]}
         hasFetchError={isTargetsError}
         setSearchText={setSearchText}