Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion x-pack/plugins/fleet/common/types/models/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ const actionNames: {
completedText: 'force unenrolled',
cancelledText: 'force unenrollment',
},
UPDATE_TAGS: {
inProgressText: 'Updating tags of',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, are these strings translated? Is it something that we should do in this context?

Copy link
Contributor Author

@juliaElastic juliaElastic Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using them in a FormattedMessage component here, though it might not be best to translate like this. The challenge is that there is a combination of statuses and action types, so writing them out separately would create about 20 strings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. I think that this could be handled in a separate ticket, as it's potentially so complex.

completedText: 'updated tags',
cancelledText: 'update tags',
},
CANCEL: { inProgressText: 'Cancelling', completedText: 'cancelled', cancelledText: '' },
ACTION: { inProgressText: 'Actioning', completedText: 'actioned', cancelledText: 'action' },
};
Expand Down Expand Up @@ -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."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed spelling #141384

/>
&nbsp;
{inProgressDescription(action.creationTime)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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',
}),
Expand All @@ -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',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 }>({});
Expand All @@ -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
Expand All @@ -103,7 +123,7 @@ export const TagsAddRemove: React.FC<Props> = ({
agents!,
tagsToAdd,
tagsToRemove,
() => onTagsUpdated(),
(hasCompleted) => handleTagsUpdated(tagsToAdd, tagsToRemove, hasCompleted),
successMessage,
errorMessage
);
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const useUpdateTags = () => {
const wrapRequest = useCallback(
async (
requestFn: () => Promise<any>,
onSuccess: () => void,
onSuccess: (hasCompleted?: boolean) => void,
successMessage?: string,
errorMessage?: string
) => {
Expand All @@ -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 ??
Expand Down Expand Up @@ -73,7 +74,7 @@ export const useUpdateTags = () => {
agents: string[] | string,
tagsToAdd: string[],
tagsToRemove: string[],
onSuccess: () => void,
onSuccess: (hasCompleted?: boolean) => void,
successMessage?: string,
errorMessage?: string
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading