Skip to content

Commit

Permalink
13121 deleting a group should be recursive (#13362)
Browse files Browse the repository at this point in the history
* Add optional chaining to fileuploadcomponent filter search

* Make sure all descendants are deleted when deleting a form item

* Update applicationmetadata when deleting a group with fileupload components

* Move `arrayUtils:prepend` to `ArrayUtils` class, use in `formLayoutUtils`

* Add utils function to get all form item ids from a layout

* Add userRequestsSynchronizationService semaphore lock to metadata deletion endpoint
  • Loading branch information
Jondyr authored Aug 20, 2024
1 parent 29196be commit 95d7612
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Altinn.Studio.Designer.Models.App;
using Altinn.Studio.Designer.Services.Interfaces;
Expand All @@ -18,14 +19,17 @@ namespace Altinn.Studio.Designer.Controllers
public class ApplicationMetadataController : ControllerBase
{
private readonly IApplicationMetadataService _applicationMetadataService;
private readonly IUserRequestsSynchronizationService _userRequestsSynchronizationService;

/// <summary>
/// Initializes a new instance of the <see cref="ApplicationMetadataController"/> class.
/// </summary>
/// <param name="applicationMetadataService">The application metadata service</param>
public ApplicationMetadataController(IApplicationMetadataService applicationMetadataService)
/// <param name="userRequestsSynchronizationService">The user requests synchronization service</param>
public ApplicationMetadataController(IApplicationMetadataService applicationMetadataService, IUserRequestsSynchronizationService userRequestsSynchronizationService)
{
_applicationMetadataService = applicationMetadataService;
_userRequestsSynchronizationService = userRequestsSynchronizationService;
}

/// <summary>
Expand Down Expand Up @@ -141,6 +145,8 @@ public async Task<ActionResult> UpdateMetadataForAttachment([FromBody] dynamic a
[Route("attachment-component")]
public async Task<ActionResult> DeleteMetadataForAttachment(string org, string app, [FromBody] string id)
{
SemaphoreSlim semaphore = _userRequestsSynchronizationService.GetRequestsSemaphore(org, app, "");
await semaphore.WaitAsync();
try
{
await _applicationMetadataService.DeleteMetadataForAttachment(org, app, id);
Expand All @@ -150,6 +156,10 @@ public async Task<ActionResult> DeleteMetadataForAttachment(string org, string a
{
return BadRequest("Could not delete metadata");
}
finally
{
semaphore.Release();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,11 @@ describe('ArrayUtils', () => {
expect(ArrayUtils.getNonEmptyArrayOrUndefined([])).toBeUndefined();
});
});

describe('prepend', () => {
it('Prepends item to array', () => {
expect(ArrayUtils.prepend([1, 2, 3], 0)).toEqual([0, 1, 2, 3]);
expect(ArrayUtils.prepend(['a', 'b', 'c'], 'd')).toEqual(['d', 'a', 'b', 'c']);
});
});
});
10 changes: 10 additions & 0 deletions frontend/libs/studio-pure-functions/src/ArrayUtils/ArrayUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,14 @@ export class ArrayUtils {
/** Returns the provided array if it has at least one item, otherwise returns undefined */
public static getNonEmptyArrayOrUndefined = <T>(array: T[]): T[] | undefined =>
array.length > 0 ? array : undefined;

/**
* Adds an item to the beginning of an array..
* @param array The array of interest.
* @param item The item to prepend.
* @returns The array with the item prepended.
*/
public static prepend<T>(array: T[], item: T): T[] {
return [item, ...array];
}
}
8 changes: 0 additions & 8 deletions frontend/packages/shared/src/utils/arrayUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,13 @@ import {
insertArrayElementAtPos,
mapByKey,
moveArrayItem,
prepend,
removeEmptyStrings,
replaceByPredicate,
replaceItemsByValue,
swapArrayElements,
} from './arrayUtils';

describe('arrayUtils', () => {
describe('prepend', () => {
it('Prepends item to array', () => {
expect(prepend([1, 2, 3], 0)).toEqual([0, 1, 2, 3]);
expect(prepend(['a', 'b', 'c'], 'd')).toEqual(['d', 'a', 'b', 'c']);
});
});

describe('areItemsUnique', () => {
it('Returns true if all items are unique', () => {
expect(areItemsUnique([1, 2, 3])).toBe(true);
Expand Down
8 changes: 0 additions & 8 deletions frontend/packages/shared/src/utils/arrayUtils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
import { ArrayUtils } from '@studio/pure-functions';

/**
* Adds an item to the beginning of an array..
* @param array The array of interest.
* @param item The item to prepend.
* @returns The array with the item prepended.
*/
export const prepend = <T>(array: T[], item: T): T[] => [item, ...array];

/**
* Replaces the last item in an array.
* @param array The array of interest.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,37 @@ import type { ComponentIdsChange } from 'app-shared/types/api/FormLayoutRequest'
import { ComponentType } from 'app-shared/types/ComponentType';
import { useUpdateBpmn } from 'app-shared/hooks/useUpdateBpmn';
import { removeDataTypeIdsToSign } from 'app-shared/utils/bpmnUtils';
import { getAllDescendants, getAllFormItemIds } from '../../utils/formLayoutUtils';
import { useDeleteAppAttachmentMetadataMutation } from '../../hooks/mutations/useDeleteAppAttachmentMetadataMutation';

export const useDeleteFormContainerMutation = (org: string, app: string, layoutSetName: string) => {
const { layout, layoutName } = useSelectedFormLayoutWithName();
const formLayoutsMutation = useFormLayoutMutation(org, app, layoutName, layoutSetName);
const deleteAppAttachmentMetadataMutation = useDeleteAppAttachmentMetadataMutation(org, app);
const updateBpmn = useUpdateBpmn(org, app);
return useMutation({
mutationFn: async (id: string) => {
const updatedLayout: IInternalLayout = ObjectUtils.deepCopy(layout);
const componentIdsChange: ComponentIdsChange = [];

const childrenComponentIds = layout.order[id];
const allComponentIds = Object.keys(layout.components);

const fileUploadComponentIds = childrenComponentIds.filter(
(componentId) =>
layout.components[componentId].type === ComponentType.FileUpload ||
layout.components[componentId].type === ComponentType.FileUploadWithTag,
);
const childrenFormItemIds = getAllDescendants(layout, id);
const allFormItemIds = getAllFormItemIds(layout);

const fileUploadComponentIds = childrenFormItemIds.filter((componentId) => {
return (
layout.components[componentId]?.type === ComponentType.FileUpload ||
layout.components[componentId]?.type === ComponentType.FileUploadWithTag
);
});
fileUploadComponentIds.forEach((id) => deleteAppAttachmentMetadataMutation.mutate(id));
if (fileUploadComponentIds.length > 0) {
await updateBpmn(removeDataTypeIdsToSign(fileUploadComponentIds));
}

// Delete child components:
// Todo: Consider if this should rather be done in the backend
for (const componentId of childrenComponentIds) {
if (allComponentIds.indexOf(componentId) > -1) {
for (const componentId of childrenFormItemIds) {
if (allFormItemIds.indexOf(componentId) > -1) {
delete updatedLayout.components[componentId];
delete updatedLayout.containers[componentId];
delete updatedLayout.order[componentId];
Expand Down
24 changes: 24 additions & 0 deletions frontend/packages/ux-editor/src/utils/formLayoutUtils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
updateContainer,
validateDepth,
findLayoutsContainingDuplicateComponents,
getAllDescendants,
getAllFormItemIds,
} from './formLayoutUtils';
import { ComponentType } from 'app-shared/types/ComponentType';
import type { IInternalLayout } from '../types/global';
Expand Down Expand Up @@ -559,6 +561,16 @@ describe('formLayoutUtils', () => {
});
});

describe('getDescendants', () => {
it('Returns the ids of the descendants of the given container', () => {
expect(getAllDescendants(mockInternal, groupId)).toEqual([
paragraphInGroupId,
groupInGroupId,
paragraphInGroupInGroupId,
]);
});
});

describe('getItem', () => {
it('Returns the item with the given id when it is a component', () => {
expect(getItem(mockInternal, paragraphId)).toEqual(paragraphComponent);
Expand Down Expand Up @@ -656,4 +668,16 @@ describe('formLayoutUtils', () => {
});
});
});

describe('getAllFormItemIds', () => {
const layout = { ...mockInternal };
expect(getAllFormItemIds(layout)).toEqual([
headerId,
paragraphId,
groupId,
paragraphInGroupId,
groupInGroupId,
paragraphInGroupInGroupId,
]);
});
});
19 changes: 19 additions & 0 deletions frontend/packages/ux-editor/src/utils/formLayoutUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,17 @@ export const isComponentTypeValidChild = (
export const getChildIds = (layout: IInternalLayout, parentId: string): string[] =>
layout.order?.[parentId] || [];

/**
* Recursively finds all the children of a container.
* @param layout The layout to search in.
* @param parentId The id of the container to find all children of.
* @returns An array of all the children of the container.
*/
export const getAllDescendants = (layout: IInternalLayout, parentId: string): string[] =>
getChildIds(layout, parentId).flatMap((id) =>
ArrayUtils.prepend(getAllDescendants(layout, id), id),
);

export const getItem = (layout: IInternalLayout, itemId: string): FormComponent | FormContainer =>
layout.components[itemId] || layout.containers[itemId];

Expand Down Expand Up @@ -471,3 +482,11 @@ export const getDuplicatedIds = (layout: IInternalLayout): string[] => {
const uniqueDuplicatedIds = Array.from(new Set(duplicatedIds));
return uniqueDuplicatedIds;
};

/**
* Get all (valid) ids in the layout
* @param layout The layout
* @returns An array of all ids in the layout
* */
export const getAllFormItemIds = (layout: IInternalLayout): string[] =>
flattenObjectValues(layout.order);

0 comments on commit 95d7612

Please sign in to comment.