Optimize requests on packs page (#3327)

Improves #3259

changes/fix-packs-page

@@ -0,0 +1 @@
+* Fix bugs with targeting and improve performance on the Packs edit page.

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

@@ -12,6 +12,7 @@ describe(
     it("Create, edit, and delete a pack and pack query successfully", () => {
       cy.visit("/packs/manage");
 
+      cy.wait(1000); // eslint-disable-line cypress/no-unnecessary-waiting
       cy.findByRole("button", { name: /create new pack/i }).click();
 
       cy.wait(1000); // eslint-disable-line cypress/no-unnecessary-waiting
@@ -116,7 +117,7 @@ describe(
 
       cy.get(".remove-pack-modal__btn-wrap > .button--alert")
         .contains("button", /delete/i)
-        .click();
+        .click({ force: true });
 
       cy.findByText(/successfully deleted/i).should("be.visible");
 

frontend/components/forms/packs/EditPackForm/EditPackForm.tsx

@@ -12,7 +12,6 @@ import InputField from "components/forms/fields/InputField";
 import SelectTargetsDropdown from "components/forms/fields/SelectTargetsDropdown";
 import PackQueriesListWrapper from "components/queries/PackQueriesListWrapper";
 
-const fieldNames = ["description", "name", "targets"];
 const baseClass = "edit-pack-form";
 
 interface IEditPackForm {

frontend/components/forms/packs/PackForm/PackForm.tsx

@@ -1,5 +1,4 @@
-import React, { Component, useState } from "react";
-import PropTypes from "prop-types";
+import React, { useState } from "react";
 import classnames from "classnames";
 
 import Button from "components/buttons/Button";
@@ -41,7 +40,6 @@ const EditPackForm = ({
   onFetchTargets,
   selectedTargetsCount,
   isPremiumTier,
-  formData,
   baseError,
 }: IPackForm): JSX.Element => {
   const [errors, setErrors] = useState<{ [key: string]: any }>({});

frontend/interfaces/pack.ts

@@ -1,4 +1,7 @@
 import PropTypes from "prop-types";
+import { IHost } from "./host";
+import { ILabel } from "./label";
+import { ITeam } from "./team";
 
 export default PropTypes.shape({
   created_at: PropTypes.string,
@@ -25,7 +28,10 @@ export interface IPack {
   disabled?: boolean;
   query_count: number;
   total_hosts_count: number;
+  hosts: IHost[];
   host_ids: number[];
+  labels: ILabel[];
   label_ids: number[];
+  teams: ITeam[];
   team_ids: number[];
 }

frontend/pages/packs/EditPackPage/EditPackPage.tsx

@@ -1,8 +1,7 @@
-import React, { useState, useEffect, useCallback, useContext } from "react";
+import React, { useState, useCallback, useContext } from "react";
 import { useQuery } from "react-query";
 import { Params } from "react-router/lib/Router";
 
-import { filter, includes } from "lodash";
 import { useDispatch } from "react-redux";
 import { push } from "react-router-redux";
 
@@ -18,12 +17,9 @@ import { ITarget, ITargetsAPIResponse } from "interfaces/target";
 import { ITeam } from "interfaces/team";
 import { AppContext } from "context/app";
 
-import hostsAPI from "services/entities/hosts";
-import labelsAPI from "services/entities/labels";
 import packsAPI from "services/entities/packs";
 import queriesAPI from "services/entities/queries";
 import scheduledqueriesAPI from "services/entities/scheduled_queries";
-import teamsAPI from "services/entities/teams";
 
 // @ts-ignore
 import { renderFlash } from "redux/nodes/notifications/actions";
@@ -44,17 +40,6 @@ interface IStoredFleetQueriesResponse {
   queries: IQuery[];
 }
 
-interface IStoredLabelsResponse {
-  labels: ILabel[];
-}
-interface IStoredHostsResponse {
-  hosts: IHost[];
-}
-
-interface IStoredTeamsResponse {
-  teams: ITeam[];
-}
-
 interface IStoredPackResponse {
   pack: IPack;
 }
@@ -88,22 +73,6 @@ const EditPacksPage = ({
     select: (data: IStoredFleetQueriesResponse) => data.queries,
   });
 
-  const { data: labels } = useQuery<IStoredLabelsResponse, Error, ILabel[]>(
-    ["labels"],
-    () => labelsAPI.loadAll(),
-    {
-      select: (data: IStoredLabelsResponse) => data.labels,
-    }
-  );
-
-  const { data: hosts } = useQuery<IStoredHostsResponse, Error, IHost[]>(
-    ["all hosts"],
-    () => hostsAPI.loadAll({ perPage: 30000 }),
-    {
-      select: (data: IStoredHostsResponse) => data.hosts,
-    }
-  );
-
   const { data: storedPack } = useQuery<IStoredPackResponse, Error, IPack>(
     ["stored pack"],
     () => packsAPI.load(packId),
@@ -137,58 +106,23 @@ const EditPacksPage = ({
   const [selectedPackQueryIds, setSelectedPackQueryIds] = useState<
     number[] | never[]
   >([]);
-  const [storedPackLabels, setStoredPackLabels] = useState<ILabel[]>([]);
-  const [storedPackHosts, setStoredPackHosts] = useState<IHost[]>([]);
-  const [storedPackTeams, setStoredPackTeams] = useState<ITeam[]>([]);
 
-  useEffect(() => {
-    if (labels && storedPack) {
-      const packLabels = filter(labels, (label) => {
-        return includes(storedPack.label_ids, label.id);
-      });
-      setStoredPackLabels(packLabels);
-    }
-  }, [labels, storedPack]);
-
-  useEffect(() => {
-    if (hosts && storedPack) {
-      const packHosts = filter(hosts, (host) => {
-        return includes(storedPack.host_ids, host.id);
-      });
-      setStoredPackHosts(packHosts);
-    }
-  }, [hosts, storedPack]);
-
-  const { data: teams } = useQuery<IStoredTeamsResponse, Error, ITeam[]>(
-    ["all teams"],
-    () => teamsAPI.loadAll(),
-    {
-      enabled: !!isPremiumTier,
-      select: (data: IStoredTeamsResponse) => data.teams,
-    }
-  );
-
-  useEffect(() => {
-    if (teams && storedPack) {
-      const packTeams = filter(teams, (team) => {
-        return includes(storedPack.team_ids, team.id);
-      });
-      setStoredPackTeams(packTeams);
-    }
-  }, [teams, storedPack]);
-
-  const packTargets = [
-    ...storedPackHosts.map((host) => ({
+  const packTargets = storedPack
+    ? [
+        ...storedPack.hosts.map((host) => ({
           ...host,
           target_type: "hosts",
         })),
-    ...storedPackLabels,
-    ...storedPackTeams.map((team) => ({
+        ...storedPack.labels.map((label) => ({
+          ...label,
+          target_type: "labels",
+        })),
+        ...storedPack.teams.map((team) => ({
           ...team,
           target_type: "teams",
-      display_text: team.name,
         })),
-  ];
+      ]
+    : [];
 
   const onCancelEditPack = () => {
     return dispatch(push(PATHS.MANAGE_PACKS));

server/datastore/mysql/packs.go

@@ -315,21 +315,38 @@ func replacePackTargetsDB(ctx context.Context, tx sqlx.ExecerContext, pack *flee
 }
 
 func loadPackTargetsDB(ctx context.Context, q sqlx.QueryerContext, pack *fleet.Pack) error {
-	var targets []fleet.PackTarget
-	sql := `SELECT * FROM pack_targets WHERE pack_id = ?`
-	if err := sqlx.SelectContext(ctx, q, &targets, sql, pack.ID); err != nil {
+	var targets []fleet.Target
+	sql := `
+	SELECT type, target_id,
+		COALESCE(
+			CASE
+				WHEN type = ? THEN (SELECT hostname FROM hosts WHERE id = target_id)
+				WHEN type = ? THEN (SELECT name FROM teams WHERE id = target_id)
+				WHEN type = ? THEN (SELECT name FROM labels WHERE id = target_id)
+			END
+		, '') AS display_text
+	FROM pack_targets
+	WHERE pack_id = ?`
+	if err := sqlx.SelectContext(
+		ctx, q, &targets, sql,
+		fleet.TargetHost, fleet.TargetTeam, fleet.TargetLabel, pack.ID,
+	); err != nil {
 		return ctxerr.Wrap(ctx, err, "select pack targets")
 	}
 
 	pack.HostIDs, pack.LabelIDs, pack.TeamIDs = []uint{}, []uint{}, []uint{}
+	pack.Hosts, pack.Labels, pack.Teams = []fleet.Target{}, []fleet.Target{}, []fleet.Target{}
 	for _, target := range targets {
 		switch target.Type {
 		case fleet.TargetHost:
 			pack.HostIDs = append(pack.HostIDs, target.TargetID)
+			pack.Hosts = append(pack.Hosts, target)
 		case fleet.TargetLabel:
 			pack.LabelIDs = append(pack.LabelIDs, target.TargetID)
+			pack.Labels = append(pack.Labels, target)
 		case fleet.TargetTeam:
 			pack.TeamIDs = append(pack.TeamIDs, target.TargetID)
+			pack.Teams = append(pack.Teams, target)
 		default:
 			return ctxerr.Errorf(ctx, "unknown target type: %d", target.Type)
 		}

server/datastore/mysql/packs_test.go

@@ -65,8 +65,11 @@ func testPacksDelete(t *testing.T, ds *Datastore) {
 func testPacksSave(t *testing.T, ds *Datastore) {
 	expectedPack := &fleet.Pack{
 		Name:     "foo",
+		Hosts:    []fleet.Target{{TargetID: 1, Type: fleet.TargetHost}},
 		HostIDs:  []uint{1},
+		Labels:   []fleet.Target{{TargetID: 1, Type: fleet.TargetLabel}},
 		LabelIDs: []uint{1},
+		Teams:    []fleet.Target{{TargetID: 1, Type: fleet.TargetTeam}},
 		TeamIDs:  []uint{1},
 	}
 
@@ -82,8 +85,11 @@ func testPacksSave(t *testing.T, ds *Datastore) {
 	expectedPack = &fleet.Pack{
 		ID:       pack.ID,
 		Name:     "bar",
+		Hosts:    []fleet.Target{{TargetID: 3, Type: fleet.TargetHost}},
 		HostIDs:  []uint{3},
+		Labels:   []fleet.Target{{TargetID: 4, Type: fleet.TargetLabel}, {TargetID: 6, Type: fleet.TargetLabel}},
 		LabelIDs: []uint{4, 6},
+		Teams:    []fleet.Target{},
 		TeamIDs:  []uint{},
 	}
 

server/fleet/packs.go

@@ -16,8 +16,11 @@ type Pack struct {
 	Platform    string   `json:"platform,omitempty"`
 	Disabled    bool     `json:"disabled"`
 	Type        *string  `json:"type" db:"pack_type"`
+	Labels      []Target `json:"labels"`
 	LabelIDs    []uint   `json:"label_ids"`
+	Hosts       []Target `json:"hosts"`
 	HostIDs     []uint   `json:"host_ids"`
+	Teams       []Target `json:"teams"`
 	TeamIDs     []uint   `json:"team_ids"`
 }
 
@@ -75,8 +78,8 @@ type PackSpecQuery struct {
 
 // PackTarget targets a pack to a host, label, or team.
 type PackTarget struct {
-	ID     uint `db:"id"`
-	PackID uint `db:"pack_id"`
+	ID     uint `db:"id" json:"-"`
+	PackID uint `db:"pack_id" json:"-"`
 	Target
 }
 

server/fleet/targets.go

@@ -1,5 +1,10 @@
 package fleet
 
+import (
+	"encoding/json"
+	"fmt"
+)
+
 type TargetSearchResults struct {
 	Hosts  []*Host
 	Labels []*Label
@@ -47,9 +52,53 @@ const (
 	TargetTeam
 )
 
+func (t TargetType) String() string {
+	switch t {
+	case TargetLabel:
+		return "label"
+	case TargetHost:
+		return "host"
+	case TargetTeam:
+		return "team"
+	default:
+		return fmt.Sprintf("unknown: %d", t)
+	}
+}
+
+func ParseTargetType(s string) (TargetType, error) {
+	switch s {
+	case "label":
+		return TargetLabel, nil
+	case "host":
+		return TargetHost, nil
+	case "team":
+		return TargetTeam, nil
+	default:
+		return 0, fmt.Errorf("invalid TargetType: %s", s)
+	}
+}
+
+func (t TargetType) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.String())
+}
+
+func (t *TargetType) UnmarshalJSON(b []byte) error {
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+	parsed, err := ParseTargetType(s)
+	if err != nil {
+		return err
+	}
+	*t = parsed
+	return nil
+}
+
 type Target struct {
-	Type     TargetType `db:"type"`
-	TargetID uint       `db:"target_id"`
+	Type        TargetType `db:"type" json:"type"`
+	TargetID    uint       `db:"target_id" json:"id"`
+	DisplayText string     `db:"display_text" json:"display_text"`
 }
 
 func (t Target) AuthzType() string {

server/fleet/targets_test.go

@@ -0,0 +1,36 @@
+package fleet_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTargetTypeJSON(t *testing.T) {
+	testCases := []struct {
+		expected  fleet.TargetType
+		shouldErr bool
+	}{
+		{fleet.TargetLabel, false},
+		{fleet.TargetHost, false},
+		{fleet.TargetTeam, false},
+		{fleet.TargetType(37), true},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.expected.String(), func(t *testing.T) {
+			b, err := json.Marshal(tt.expected)
+			require.NoError(t, err)
+			var target fleet.TargetType
+			err = json.Unmarshal(b, &target)
+			if tt.shouldErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.expected, target)
+			}
+		})
+	}
+}