Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

12634 implement validation across layouts when loading layout set with modal warning user of the issue #12988

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: 3 additions & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,9 @@
"ux_editor.modal_properties_valid_file_endings_all": "Alle filtyper",
"ux_editor.modal_properties_valid_file_endings_custom": "Egendefinerte filtyper",
"ux_editor.modal_properties_valid_file_endings_helper": "Skriv inn gyldige filtyper (Eksempel: .jpeg, .pdf, .png)",
"ux_editor.modal_properties_warning_modal_instructive_text_body": "For å fikse dette, må du gå til de aktuelle komponentene og endre ID-ene. Sidene og komponentene som har samme ID er markert i rødt.",
"ux_editor.modal_properties_warning_modal_sub_title": "Alle komponent-ID-er må være unike. Appen kan ikke publiseres før du har rettet feilen.",
"ux_editor.modal_properties_warning_modal_title": "Du har samme ID på flere komponenter",
"ux_editor.modal_radio_button_add_label": "Legg til tekst",
"ux_editor.modal_radio_button_increment": "Radioknapp",
"ux_editor.modal_radio_button_set_preselected": "Sett forhåndsvalgt radioknapp",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const updateDataTypesToSign = (

export const getSelectedDataTypes = (bpmnDetails: BpmnDetails): string[] => {
return (
bpmnDetails.element.businessObject.extensionElements.values[0].signatureConfig?.dataTypesToSign?.dataTypes?.map(
bpmnDetails.element.businessObject.extensionElements?.values[0].signatureConfig?.dataTypesToSign?.dataTypes?.map(
(element: ModdleElement) => element.dataType,
) || []
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ const getExistingDataTypesProps = () => {
};

describe('EditDataTypesToSign', () => {
afterEach(jest.clearAllMocks);
beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation(() => {});
});

afterEach(() => {
jest.restoreAllMocks();
jest.clearAllMocks();
});

it('should display a combobox with an error message when task has no data type', () => {
renderEditDataType();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { screen } from '@testing-library/react';
import { screen, waitFor } from '@testing-library/react';
import { renderWithProviders } from '../../../testing/mocks';
import { PageConfigPanel } from './PageConfigPanel';
import { QueryKey } from 'app-shared/types/QueryKey';
Expand All @@ -11,6 +11,12 @@ import type { IFormLayouts } from '../../../types/global';
import { layout1NameMock, layoutMock } from '@altinn/ux-editor/testing/layoutMock';
import { layoutSet1NameMock } from '@altinn/ux-editor/testing/layoutSetsMock';
import { app, org } from '@studio/testing/testids';
import { findLayoutsContainingDuplicateComponents } from '../../../utils/formLayoutUtils';

jest.mock('../../../utils/formLayoutUtils', () => ({
...jest.requireActual('../../../utils/formLayoutUtils'),
findLayoutsContainingDuplicateComponents: jest.fn(),
}));

// Test data
const layoutSet = layoutSet1NameMock;
Expand Down Expand Up @@ -39,7 +45,10 @@ const layouts: IFormLayouts = {
};

describe('PageConfigPanel', () => {
it('render heading with "no selected page" message when selected layout is "default"', () => {
beforeEach(() => {
(findLayoutsContainingDuplicateComponents as jest.Mock).mockReturnValue([]);
});
it('render heading with "no selected page" message when selected layout is "default"', async () => {
renderPageConfigPanel();
screen.getByRole('heading', { name: textMock('right_menu.content_empty') });
});
Expand Down Expand Up @@ -75,6 +84,7 @@ describe('PageConfigPanel', () => {

it('render warning when layout is selected and has duplicated ids', () => {
renderPageConfigPanel(duplicatedLayout);

screen.getByRole('heading', { name: textMock('ux_editor.config.warning_duplicates.heading') });
});

Expand All @@ -87,6 +97,25 @@ describe('PageConfigPanel', () => {
const uniqueIds = screen.queryByText(/<idcontainer>, <idContainer3>/i);
expect(uniqueIds).not.toBeInTheDocument();
});

it('should not show warning modal when there are no duplicated ids across layouts', () => {
renderPageConfigPanel();

const modal = screen.queryByRole('dialog');

expect(modal).not.toBeInTheDocument();
});

it('should show warning modal when there are duplicated ids across layouts', async () => {
(findLayoutsContainingDuplicateComponents as jest.Mock).mockReturnValue({
duplicateLayouts: [duplicatedLayout],
});
renderPageConfigPanel();
await waitFor(() => {
const modal = screen.getByRole('dialog');
expect(modal).toBeInTheDocument();
});
});
});

const renderPageConfigPanel = (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useEffect, useRef } from 'react';
import { Accordion } from '@digdir/design-system-react';
import { FileIcon } from '@studio/icons';
import { StudioSectionHeader } from '@studio/components';
Expand All @@ -9,17 +9,21 @@ import { TextResource } from '../../TextResource/TextResource';
import { EditPageId } from './EditPageId';
import { textResourceByLanguageAndIdSelector } from '../../../selectors/textResourceSelectors';
import type { ITextResource } from 'app-shared/types/global';
import { duplicatedIdsExistsInLayout } from '../../../utils/formLayoutUtils';
import {
duplicatedIdsExistsInLayout,
findLayoutsContainingDuplicateComponents,
} from '../../../utils/formLayoutUtils';
import { PageConfigWarning } from './PageConfigWarning';
import classes from './PageConfigPanel.module.css';
import { PageConfigWarningModal } from './PageConfigWarningModal';
import type { IInternalLayout } from '@altinn/ux-editor/types/global';

export const PageConfigPanel = () => {
const { selectedFormLayoutName } = useAppContext();
const t = useText();

const modalRef = useRef<HTMLDialogElement>(null);
const layoutIsSelected =
selectedFormLayoutName !== DEFAULT_SELECTED_LAYOUT_NAME && selectedFormLayoutName !== undefined;

const layoutNameTextResourceSelector = textResourceByLanguageAndIdSelector(
DEFAULT_LANGUAGE,
selectedFormLayoutName,
Expand All @@ -28,14 +32,24 @@ export const PageConfigPanel = () => {
layoutNameTextResourceSelector,
);
const layoutNameText = layoutNameTextResource?.value;

const headingTitle = !layoutIsSelected
? t('right_menu.content_empty')
: layoutNameText ?? selectedFormLayoutName;

const layout = useFormLayouts()[selectedFormLayoutName];
const layouts: Record<string, IInternalLayout> = useFormLayouts();
const layout = layouts[selectedFormLayoutName];
const hasDuplicatedIds = duplicatedIdsExistsInLayout(layout);

const duplicateLayouts: string[] =
findLayoutsContainingDuplicateComponents(layouts).duplicateLayouts;
const hasDuplicatedIdsInAllLayouts = duplicateLayouts?.length > 0;

useEffect(() => {
if (hasDuplicatedIdsInAllLayouts) {
modalRef.current?.showModal();
}
}, [hasDuplicatedIdsInAllLayouts]);

if (layoutIsSelected && hasDuplicatedIds) {
return <PageConfigWarning selectedFormLayoutName={selectedFormLayoutName} layout={layout} />;
}
Expand Down Expand Up @@ -72,6 +86,7 @@ export const PageConfigPanel = () => {
</Accordion>
</>
)}
<PageConfigWarningModal modalRef={modalRef} />
</>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.headingWrapper {
display: flex;
align-items: center;
}

.subTitle {
margin-bottom: var(--fds-spacing-6);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import type { ReactNode } from 'react';
import React from 'react';
import classes from './PageConfigWarningModal.module.css';
import { Modal } from '@digdir/design-system-react';
import { useTranslation } from 'react-i18next';

export interface PageConfigWarningModalProps {
modalRef: React.MutableRefObject<HTMLDialogElement>;
}
export const PageConfigWarningModal = ({ modalRef }: PageConfigWarningModalProps): ReactNode => {
const { t } = useTranslation();
return (
<Modal ref={modalRef} role='dialog'>
<Modal.Header closeButton={true}>
{t('ux_editor.modal_properties_warning_modal_title')}
</Modal.Header>
<Modal.Content>
<div className={classes.subTitle}>
{t('ux_editor.modal_properties_warning_modal_sub_title')}
</div>
{t('ux_editor.modal_properties_warning_modal_instructive_text_body')}
</Modal.Content>
<Modal.Footer />
</Modal>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { PageConfigWarningModal } from './PageConfigWarningModal';
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ReactNode } from 'react';
import React from 'react';
import React, { useMemo } from 'react';
import classes from './DesignView.module.css';
import { useTranslation } from 'react-i18next';
import { useStudioEnvironmentParams } from 'app-shared/hooks/useStudioEnvironmentParams';
Expand All @@ -14,7 +14,10 @@ import { ReceiptContent } from './ReceiptContent';
import { useAppContext, useFormLayouts } from '../../hooks';
import { FormLayout } from './FormLayout';
import { StudioButton } from '@studio/components';
import { duplicatedIdsExistsInLayout } from '../../utils/formLayoutUtils';
import {
duplicatedIdsExistsInLayout,
findLayoutsContainingDuplicateComponents,
} from '../../utils/formLayoutUtils';

/**
* Maps the IFormLayouts object to a list of FormLayouts
Expand Down Expand Up @@ -89,6 +92,11 @@ export const DesignView = (): ReactNode => {
);
};

const layoutsWithDuplicateComponents = useMemo(
() => findLayoutsContainingDuplicateComponents(layouts),
[layouts],
);

/**
* Displays the pages as an ordered list
*/
Expand All @@ -100,16 +108,22 @@ export const DesignView = (): ReactNode => {

// Check if the layout has unique component IDs
const isValidLayout = !duplicatedIdsExistsInLayout(layout.data);

return (
<PageAccordion
key={i}
pageName={layout.page}
isOpen={layout.page === selectedFormLayoutName}
onClick={() => handleClickAccordion(layout.page)}
isValid={isValidLayout}
hasUniqueIds={!layoutsWithDuplicateComponents.duplicateLayouts.includes(layout.page)}
>
{layout.page === selectedFormLayoutName && (
<FormLayout layout={layout.data} isValid={isValidLayout} />
<FormLayout
layout={layout.data}
isValid={isValidLayout}
duplicateComponents={layoutsWithDuplicateComponents.duplicateComponents}
/>
)}
</PageAccordion>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ import { FormLayoutWarning } from './FormLayoutWarning';
export interface FormLayoutProps {
layout: IInternalLayout;
isValid: boolean;
duplicateComponents?: string[];
}

export const FormLayout = ({ layout, isValid }: FormLayoutProps) => {
export const FormLayout = ({ layout, isValid, duplicateComponents }: FormLayoutProps) => {
if (!isValid) {
return <FormLayoutWarning layout={layout} />;
}
return (
<>
{hasMultiPageGroup(layout) && <MultiPageWarning />}
<FormTree layout={layout} />
<FormTree duplicateComponents={duplicateComponents} layout={layout} />
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { QuestionmarkDiamondIcon } from '@studio/icons';
export type FormItemProps = {
layout: IInternalLayout;
id: string;
duplicateComponents?: string[];
};

export const FormItem = ({ layout, id }: FormItemProps) => {
export const FormItem = ({ layout, id, duplicateComponents }: FormItemProps) => {
const itemTitle = useItemTitle();
const { t } = useTranslation();

Expand All @@ -32,7 +33,9 @@ export const FormItem = ({ layout, id }: FormItemProps) => {
: formItemConfigs[formItem.type]?.icon;

const labelWrapper = (label: string) => (
<FormItemTitle formItem={formItem}>{label}</FormItemTitle>
<FormItemTitle duplicateComponents={duplicateComponents} formItem={formItem}>
{label}
</FormItemTitle>
);

return (
Expand All @@ -44,7 +47,7 @@ export const FormItem = ({ layout, id }: FormItemProps) => {
labelWrapper={labelWrapper}
nodeId={id}
>
{renderItemList(layout, id)}
{renderItemList(layout, duplicateComponents, id)}
</DragAndDropTree.Item>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@
.root:not(:hover):not(:focus-within) .deleteButton {
visibility: hidden;
}

.duplicateComponentIds {
background-color: var(--fds-semantic-surface-danger-subtle) !important;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import { useDeleteItem } from './useDeleteItem';
import { isContainer } from '../../../../../utils/formItemUtils';
import { useFormItemContext } from '../../../../FormItemContext';
import { useAppContext } from '../../../../../hooks';
import classNames from 'classnames';

export interface FormItemTitleProps {
children: ReactNode;
formItem: FormComponent | FormContainer;
duplicateComponents?: string[];
}

export const FormItemTitle = ({ children, formItem }: FormItemTitleProps) => {
export const FormItemTitle = ({ children, formItem, duplicateComponents }: FormItemTitleProps) => {
const { t } = useTranslation();
const deleteItem = useDeleteItem(formItem);
const { selectedFormLayoutSetName, refetchLayouts } = useAppContext();
Expand All @@ -38,7 +40,11 @@ export const FormItemTitle = ({ children, formItem }: FormItemTitleProps) => {
}, [formItem, t, deleteItem, refetchLayouts, selectedFormLayoutSetName, handleDiscard]);

return (
<div className={classes.root}>
<div
className={classNames(classes.root, {
[classes.duplicateComponentIds]: duplicateComponents?.includes(formItem.id),
})}
>
<div className={classes.label}>{children}</div>
<StudioButton
className={classes.deleteButton}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { useTranslation } from 'react-i18next';

export type FormTreeProps = {
layout: IInternalLayout;
duplicateComponents?: string[];
};

export const FormTree = ({ layout }: FormTreeProps) => {
export const FormTree = ({ layout, duplicateComponents }: FormTreeProps) => {
const { handleEdit, formItemId: formId } = useFormItemContext();
const { t } = useTranslation();

Expand All @@ -23,7 +24,7 @@ export const FormTree = ({ layout }: FormTreeProps) => {
emptyMessage={t('ux_editor.container_empty')}
selectedId={formId}
>
{renderItemList(layout, BASE_CONTAINER_ID)}
{renderItemList(layout, duplicateComponents, BASE_CONTAINER_ID)}
</DragAndDropTree.Root>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ import { getChildIds } from '../../../utils/formLayoutUtils';
import { FormItem } from './FormItem';
import type { IInternalLayout } from '../../../types/global';

export const renderItemList = (layout: IInternalLayout, parentId: string) => {
export const renderItemList = (
layout: IInternalLayout,
duplicateComponents: string[],
parentId: string,
) => {
const childIds = getChildIds(layout, parentId);
return childIds.length ? (
<>
{childIds.map((id) => (
<FormItem layout={layout} id={id} key={id} />
<FormItem duplicateComponents={duplicateComponents} layout={layout} id={id} key={id} />
))}
</>
) : null;
Expand Down
Loading