[Fleet] Bulk update agent tags improvements (#141376)

* update tags: added action doc and results, updating selected list immediately * fix tests, fix tag rename and delete * added unit test
elastic · Sep 23, 2022 · 30d5ed5 · 30d5ed5
1 parent 10564ef
commit 30d5ed5
Show file tree

Hide file tree

Showing 10 changed files with 186 additions and 23 deletions.
diff --git a/x-pack/plugins/fleet/common/types/models/agent.ts b/x-pack/plugins/fleet/common/types/models/agent.ts
@@ -36,7 +36,8 @@ export type AgentActionType =
   | 'SETTINGS'
   | 'POLICY_REASSIGN'
   | 'CANCEL'
-  | 'FORCE_UNENROLL';
+  | 'FORCE_UNENROLL'
+  | 'UPDATE_TAGS';
 
 type FleetServerAgentComponentStatusTuple = typeof FleetServerAgentComponentStatuses;
 export type FleetServerAgentComponentStatus = FleetServerAgentComponentStatusTuple[number];

diff --git a/...c/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx b/...c/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx
@@ -243,6 +243,11 @@ const actionNames: {
     completedText: 'force unenrolled',
     cancelledText: 'force unenrollment',
   },
+  UPDATE_TAGS: {
+    inProgressText: 'Updating tags of',
+    completedText: 'updated tags',
+    cancelledText: 'update tags',
+  },
   CANCEL: { inProgressText: 'Cancelling', completedText: 'cancelled', cancelledText: '' },
   ACTION: { inProgressText: 'Actioning', completedText: 'actioned', cancelledText: 'action' },
 };
@@ -362,7 +367,7 @@ const ActivityItem: React.FunctionComponent<{ action: ActionStatus }> = ({ actio
           <p>
             <FormattedMessage
               id="xpack.fleet.agentActivityFlyout.failureDescription"
-              defaultMessage=" A problem occured during this operation."
+              defaultMessage=" A problem occurred during this operation."
             />
             &nbsp;
             {inProgressDescription(action.creationTime)}

diff --git a/...leet/public/applications/fleet/sections/agents/agent_list_page/components/tag_options.tsx b/...leet/public/applications/fleet/sections/agents/agent_list_page/components/tag_options.tsx
@@ -25,7 +25,7 @@ import { sanitizeTag } from '../utils';
 interface Props {
   tagName: string;
   isTagHovered: boolean;
-  onTagsUpdated: () => void;
+  onTagsUpdated: (tagsToAdd: string[], tagsToRemove: string[], hasCompleted?: boolean) => void;
 }
 
 export const TagOptions: React.FC<Props> = ({ tagName, isTagHovered, onTagsUpdated }: Props) => {
@@ -65,7 +65,7 @@ export const TagOptions: React.FC<Props> = ({ tagName, isTagHovered, onTagsUpdat
       kuery,
       [newName],
       [tagName],
-      () => onTagsUpdated(),
+      (hasCompleted) => onTagsUpdated([newName], [tagName], hasCompleted),
       i18n.translate('xpack.fleet.renameAgentTags.successNotificationTitle', {
         defaultMessage: 'Tag renamed',
       }),
@@ -81,7 +81,7 @@ export const TagOptions: React.FC<Props> = ({ tagName, isTagHovered, onTagsUpdat
       kuery,
       [],
       [tagName],
-      () => onTagsUpdated(),
+      (hasCompleted) => onTagsUpdated([], [tagName], hasCompleted),
       i18n.translate('xpack.fleet.deleteAgentTags.successNotificationTitle', {
         defaultMessage: 'Tag deleted',
       }),

diff --git a/...ic/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx b/...ic/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx
@@ -226,6 +226,30 @@ describe('TagsAddRemove', () => {
     );
   });
 
+  it('should add to selected tags on add if action not completed immediately', async () => {
+    mockBulkUpdateTags.mockImplementation((agents, tagsToAdd, tagsToRemove, onSuccess) => {
+      onSuccess(false);
+    });
+    const result = renderComponent(undefined, '');
+    const getTag = (name: string) => result.getByText(name);
+
+    fireEvent.click(getTag('tag2'));
+
+    expect(result.getByTitle('tag2').getAttribute('aria-checked')).toEqual('true');
+  });
+
+  it('should remove from selected tags on remove if action not completed immediately', async () => {
+    mockBulkUpdateTags.mockImplementation((agents, tagsToAdd, tagsToRemove, onSuccess) => {
+      onSuccess(false);
+    });
+    const result = renderComponent(undefined, '');
+    const getTag = (name: string) => result.getByText(name);
+
+    fireEvent.click(getTag('tag1'));
+
+    expect(result.getByTitle('tag1').getAttribute('aria-checked')).toEqual('false');
+  });
+
   it('should add new tag when not found in search and button clicked - bulk selection', () => {
     const result = renderComponent(undefined, 'query');
     const searchInput = result.getByRole('combobox');
@@ -257,4 +281,39 @@ describe('TagsAddRemove', () => {
 
     expect(result.queryByRole('button')).not.toBeInTheDocument();
   });
+
+  it('should remove from selected and all tags on delete if action not completed immediately', async () => {
+    mockBulkUpdateTags.mockImplementation((agents, tagsToAdd, tagsToRemove, onSuccess) => {
+      onSuccess(false);
+    });
+    const result = renderComponent('agent1');
+    fireEvent.mouseEnter(result.getByText('tag1').closest('.euiFlexGroup')!);
+
+    fireEvent.click(result.getByRole('button'));
+
+    fireEvent.click(result.getByText('Delete tag').closest('button')!);
+
+    expect(result.queryByTitle('tag1')).toBeNull();
+  });
+
+  it('should update selected and all tags on rename if action not completed immediately', async () => {
+    mockBulkUpdateTags.mockImplementation((agents, tagsToAdd, tagsToRemove, onSuccess) => {
+      onSuccess(false);
+    });
+    const result = renderComponent('agent1');
+    fireEvent.mouseEnter(result.getByText('tag1').closest('.euiFlexGroup')!);
+
+    fireEvent.click(result.getByRole('button'));
+
+    const input = result.getByDisplayValue('tag1');
+    fireEvent.input(input, {
+      target: { value: 'newName' },
+    });
+    fireEvent.keyDown(input, {
+      key: 'Enter',
+    });
+
+    expect(result.queryByTitle('tag1')).toBeNull();
+    expect(result.getByText('newName')).toBeInTheDocument();
+  });
 });
diff --git a/.../public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx b/.../public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import React, { Fragment, useCallback, useEffect, useState, useMemo } from 'react';
+import React, { Fragment, useEffect, useState, useMemo, useCallback } from 'react';
 import { difference } from 'lodash';
 import styled from 'styled-components';
 import type { EuiSelectableOption } from '@elastic/eui';
@@ -54,16 +54,18 @@ export const TagsAddRemove: React.FC<Props> = ({
   onClosePopover,
 }: Props) => {
   const labelsFromTags = useCallback(
-    (tags: string[]) =>
+    (tags: string[], selected: string[]) =>
       tags.map((tag: string) => ({
         label: tag,
-        checked: selectedTags.includes(tag) ? 'on' : undefined,
+        checked: selected.includes(tag) ? 'on' : undefined,
         onFocusBadge: false,
       })),
-    [selectedTags]
+    []
   );
 
-  const [labels, setLabels] = useState<Array<EuiSelectableOption<any>>>(labelsFromTags(allTags));
+  const [labels, setLabels] = useState<Array<EuiSelectableOption<any>>>(
+    labelsFromTags(allTags, selectedTags)
+  );
   const [searchValue, setSearchValue] = useState<string | undefined>(undefined);
   const [isPopoverOpen, setIsPopoverOpen] = useState(true);
   const [isTagHovered, setIsTagHovered] = useState<{ [tagName: string]: boolean }>({});
@@ -76,24 +78,42 @@ export const TagsAddRemove: React.FC<Props> = ({
 
   // update labels after tags changing
   useEffect(() => {
-    setLabels(labelsFromTags(allTags));
-  }, [allTags, labelsFromTags]);
+    setLabels(labelsFromTags(allTags, selectedTags));
+  }, [allTags, labelsFromTags, selectedTags]);
 
   const isExactMatch = useMemo(
     () => labels.some((label) => label.label === searchValue),
     [labels, searchValue]
   );
 
+  const handleTagsUpdated = (
+    tagsToAdd: string[],
+    tagsToRemove: string[],
+    hasCompleted: boolean = true,
+    isRenameOrDelete = false
+  ) => {
+    if (hasCompleted) {
+      return onTagsUpdated();
+    }
+    const newSelectedTags = difference(selectedTags, tagsToRemove).concat(tagsToAdd);
+    const allTagsWithNew = allTags.includes(tagsToAdd[0]) ? allTags : allTags.concat(tagsToAdd);
+    const allTagsWithRemove = isRenameOrDelete
+      ? difference(allTagsWithNew, tagsToRemove)
+      : allTagsWithNew;
+    setLabels(labelsFromTags(allTagsWithRemove, newSelectedTags));
+  };
+
   const updateTags = async (
     tagsToAdd: string[],
     tagsToRemove: string[],
     successMessage?: string,
     errorMessage?: string
   ) => {
+    const newSelectedTags = difference(selectedTags, tagsToRemove).concat(tagsToAdd);
     if (agentId) {
       updateTagsHook.updateTags(
         agentId,
-        difference(selectedTags, tagsToRemove).concat(tagsToAdd),
+        newSelectedTags,
         () => onTagsUpdated(),
         successMessage,
         errorMessage
@@ -103,7 +123,7 @@ export const TagsAddRemove: React.FC<Props> = ({
         agents!,
         tagsToAdd,
         tagsToRemove,
-        () => onTagsUpdated(),
+        (hasCompleted) => handleTagsUpdated(tagsToAdd, tagsToRemove, hasCompleted),
         successMessage,
         errorMessage
       );
@@ -133,7 +153,9 @@ export const TagsAddRemove: React.FC<Props> = ({
           <TagOptions
             tagName={option.label}
             isTagHovered={isTagHovered[option.label]}
-            onTagsUpdated={onTagsUpdated}
+            onTagsUpdated={(tagsToAdd, tagsToRemove, hasCompleted) =>
+              handleTagsUpdated(tagsToAdd, tagsToRemove, hasCompleted, true)
+            }
           />
         </EuiFlexItem>
       </EuiFlexGroup>

diff --git a/.../public/applications/fleet/sections/agents/agent_list_page/hooks/use_update_tags.test.tsx b/.../public/applications/fleet/sections/agents/agent_list_page/hooks/use_update_tags.test.tsx
@@ -35,10 +35,11 @@ describe('useUpdateTags', () => {
   const mockOnSuccess = jest.fn();
   beforeEach(() => {
     mockSendPutAgentTagsUpdate.mockReset();
+    mockSendPostBulkAgentTagsUpdate.mockReset();
     mockOnSuccess.mockReset();
   });
   it('should call onSuccess when update tags succeeds', async () => {
-    mockSendPutAgentTagsUpdate.mockResolvedValueOnce({});
+    mockSendPutAgentTagsUpdate.mockResolvedValueOnce({ data: {} });
 
     const { result } = renderHook(() => useUpdateTags());
     await act(() => result.current.updateTags('agent1', ['tag1'], mockOnSuccess));
@@ -61,7 +62,7 @@ describe('useUpdateTags', () => {
   });
 
   it('should call onSuccess when bulk update tags succeeds', async () => {
-    mockSendPostBulkAgentTagsUpdate.mockResolvedValueOnce({});
+    mockSendPostBulkAgentTagsUpdate.mockResolvedValueOnce({ data: { actionId: 'action' } });
 
     const { result } = renderHook(() => useUpdateTags());
     await act(() => result.current.bulkUpdateTags('query', ['tag1'], [], mockOnSuccess));

diff --git a/...fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_update_tags.tsx b/...fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_update_tags.tsx
@@ -20,7 +20,7 @@ export const useUpdateTags = () => {
   const wrapRequest = useCallback(
     async (
       requestFn: () => Promise<any>,
-      onSuccess: () => void,
+      onSuccess: (hasCompleted?: boolean) => void,
       successMessage?: string,
       errorMessage?: string
     ) => {
@@ -30,14 +30,15 @@ export const useUpdateTags = () => {
         if (res.error) {
           throw res.error;
         }
+        const hasCompleted = !res.data.actionId;
         const message =
           successMessage ??
           i18n.translate('xpack.fleet.updateAgentTags.successNotificationTitle', {
             defaultMessage: 'Tags updated',
           });
         notifications.toasts.addSuccess(message);
 
-        onSuccess();
+        onSuccess(hasCompleted);
       } catch (error) {
         const errorTitle =
           errorMessage ??
@@ -73,7 +74,7 @@ export const useUpdateTags = () => {
       agents: string[] | string,
       tagsToAdd: string[],
       tagsToRemove: string[],
-      onSuccess: () => void,
+      onSuccess: (hasCompleted?: boolean) => void,
       successMessage?: string,
       errorMessage?: string
     ) => {

diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts
@@ -144,7 +144,7 @@ export async function updateActionsForForceUnenroll(
   actionId: string,
   total: number
 ) {
-  // creating an unenroll so that force unenroll shows up in activity
+  // creating an action doc so that force unenroll shows up in activity
   await createAgentAction(esClient, {
     id: actionId,
     agents: [],

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
@@ -8,6 +8,7 @@ import type { SavedObjectsClientContract } from '@kbn/core/server';
 import type { ElasticsearchClientMock } from '@kbn/core/server/mocks';
 import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
 
+import { createClientMock } from './action.mock';
 import { updateAgentTags } from './update_agent_tags';
 
 jest.mock('./filter_hosted_agents', () => ({
@@ -105,6 +106,31 @@ describe('update_agent_tags', () => {
     await updateAgentTags(soClient, esClient, { agentIds: ['agent1'] }, ['newName'], []);
 
     expectTagsInEsBulk(['one', 'two', 'three', 'newName']);
+
+    const actionResults = esClient.bulk.mock.calls[1][0] as any;
+    const resultIds = actionResults?.body
+      ?.filter((i: any) => i.agent_id)
+      .map((i: any) => i.agent_id);
+    expect(resultIds).toEqual(['agent1']);
+
+    const action = esClient.create.mock.calls[0][0] as any;
+    expect(action.body.type).toEqual('UPDATE_TAGS');
+  });
+
+  it('should write error action results for hosted agent', async () => {
+    const { esClient: esClientMock, agentInHostedDoc } = createClientMock();
+
+    await updateAgentTags(
+      soClient,
+      esClientMock,
+      { agentIds: [agentInHostedDoc._id] },
+      ['newName'],
+      []
+    );
+
+    const errorResults = esClientMock.bulk.mock.calls[1][0] as any;
+    const errorIds = errorResults?.body?.filter((i: any) => i.agent_id).map((i: any) => i.agent_id);
+    expect(errorIds).toEqual([agentInHostedDoc._id]);
   });
 
   it('should add tag at the end when tagsToRemove not in existing tags', async () => {