From 1bd7f3fecd8a1d88f2a480e9c5dc6f254c9401f2 Mon Sep 17 00:00:00 2001 From: Martin Gunnerud Date: Tue, 19 Nov 2024 16:25:30 +0100 Subject: [PATCH 01/18] chore(resource-adm): remove asChild prop from links (#14106) Co-authored-by: Tomas Engebretsen --- .../AccessListDetail.test.tsx | 10 ++++++++ .../AccessListDetails/AccessListDetail.tsx | 17 +++++++++----- .../GoBackButton/GoBackButton.tsx | 4 ++-- .../components/LeftNavigationBar/Tab/Tab.tsx | 4 ++-- .../LinkButton/LinkButton.module.css | 8 ------- .../components/LinkButton/LinkButton.test.tsx | 3 ++- .../components/LinkButton/LinkButton.tsx | 23 +++++++++++++------ .../ResourceAccessLists.test.tsx | 17 ++++++++++++++ .../ResourceAccessLists.tsx | 21 ++++++++++------- .../ResourceDeployStatus.test.tsx | 12 +++------- .../ResourceDeployStatus.tsx | 2 +- .../ListAdminPage/ListAdminPage.test.tsx | 14 +++++++++++ .../pages/ListAdminPage/ListAdminPage.tsx | 17 +++++++++----- 13 files changed, 102 insertions(+), 50 deletions(-) delete mode 100644 frontend/resourceadm/components/LinkButton/LinkButton.module.css diff --git a/frontend/resourceadm/components/AccessListDetails/AccessListDetail.test.tsx b/frontend/resourceadm/components/AccessListDetails/AccessListDetail.test.tsx index fece855eee9..b5dbeededff 100644 --- a/frontend/resourceadm/components/AccessListDetails/AccessListDetail.test.tsx +++ b/frontend/resourceadm/components/AccessListDetails/AccessListDetail.test.tsx @@ -150,6 +150,16 @@ describe('AccessListDetail', () => { screen.queryByText(textMock('resourceadm.listadmin_delete_list_header')), ).not.toBeInTheDocument(); }); + + it('should call back on back click', async () => { + const user = userEvent.setup(); + renderAccessListDetail(); + + const backLink = screen.getByRole('link', { name: textMock('general.back') }); + await user.click(backLink); + + expect(mockedNavigate).toHaveBeenCalledWith('/listadmin'); + }); }); const renderAccessListDetail = ( diff --git a/frontend/resourceadm/components/AccessListDetails/AccessListDetail.tsx b/frontend/resourceadm/components/AccessListDetails/AccessListDetail.tsx index f9604f8912d..ab34d00e33a 100644 --- a/frontend/resourceadm/components/AccessListDetails/AccessListDetail.tsx +++ b/frontend/resourceadm/components/AccessListDetails/AccessListDetail.tsx @@ -1,8 +1,8 @@ import React, { useRef, useState } from 'react'; -import { useNavigate, Link } from 'react-router-dom'; +import { useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { toast } from 'react-toastify'; -import { Textfield, Heading, Link as DigdirLink } from '@digdir/designsystemet-react'; +import { Textfield, Heading } from '@digdir/designsystemet-react'; import classes from './AccessListDetail.module.css'; import type { AccessList, ResourceError } from 'app-shared/types/ResourceAdm'; import { FieldWrapper } from '../FieldWrapper'; @@ -10,7 +10,7 @@ import { useEditAccessListMutation } from '../../hooks/mutations/useEditAccessLi import { useDeleteAccessListMutation } from '../../hooks/mutations/useDeleteAccessListMutation'; import { AccessListMembers } from '../AccessListMembers'; import { TrashIcon } from '@studio/icons'; -import { StudioButton, StudioModal } from '@studio/components'; +import { StudioButton, StudioLink, StudioModal } from '@studio/components'; import { ServerCodes } from 'app-shared/enums/ServerCodes'; import { AccessListPreconditionFailedToast } from '../AccessListPreconditionFailedToast'; @@ -81,6 +81,11 @@ export const AccessListDetail = ({ deleteWarningModalRef.current?.close(); }; + const handleBackClick = (event: React.MouseEvent): void => { + event.preventDefault(); + navigate(backUrl); + }; + return (
@@ -104,9 +109,9 @@ export const AccessListDetail = ({
- - {t('general.back')} - + + {t('general.back')} +
{t('resourceadm.listadmin_list_detail_header')} diff --git a/frontend/resourceadm/components/LeftNavigationBar/GoBackButton/GoBackButton.tsx b/frontend/resourceadm/components/LeftNavigationBar/GoBackButton/GoBackButton.tsx index 5632702119f..1c86417a303 100644 --- a/frontend/resourceadm/components/LeftNavigationBar/GoBackButton/GoBackButton.tsx +++ b/frontend/resourceadm/components/LeftNavigationBar/GoBackButton/GoBackButton.tsx @@ -42,8 +42,8 @@ export const GoBackButton = ({ className, text, to }: GoBackButtonProps): ReactN return ( - - {text} + + {text} ); diff --git a/frontend/resourceadm/components/LeftNavigationBar/Tab/Tab.tsx b/frontend/resourceadm/components/LeftNavigationBar/Tab/Tab.tsx index bae7884b52b..aa70fd908d1 100644 --- a/frontend/resourceadm/components/LeftNavigationBar/Tab/Tab.tsx +++ b/frontend/resourceadm/components/LeftNavigationBar/Tab/Tab.tsx @@ -55,8 +55,8 @@ export const Tab = ({ onKeyDown={onKeyDown} > {tab.icon} - - {t(tab.tabName)} + + {t(tab.tabName)} ); diff --git a/frontend/resourceadm/components/LinkButton/LinkButton.module.css b/frontend/resourceadm/components/LinkButton/LinkButton.module.css deleted file mode 100644 index fbe658e8e2c..00000000000 --- a/frontend/resourceadm/components/LinkButton/LinkButton.module.css +++ /dev/null @@ -1,8 +0,0 @@ -.linkButton { - background-color: inherit; - border: none; - font-size: inherit; - font-family: inherit; - font-weight: inherit; - padding: 0; -} diff --git a/frontend/resourceadm/components/LinkButton/LinkButton.test.tsx b/frontend/resourceadm/components/LinkButton/LinkButton.test.tsx index 048baac7520..4507e73fb34 100644 --- a/frontend/resourceadm/components/LinkButton/LinkButton.test.tsx +++ b/frontend/resourceadm/components/LinkButton/LinkButton.test.tsx @@ -8,6 +8,7 @@ describe('LinkButton', () => { const mockOnClick = jest.fn(); const defaultProps: LinkButtonProps = { + page: 'about', onClick: mockOnClick, children: 'Click Me', }; @@ -16,7 +17,7 @@ describe('LinkButton', () => { const user = userEvent.setup(); render(); - const linkButton = screen.getByRole('button', { name: /click me/i }); + const linkButton = screen.getByRole('link', { name: /click me/i }); await user.click(linkButton); expect(mockOnClick).toHaveBeenCalled(); diff --git a/frontend/resourceadm/components/LinkButton/LinkButton.tsx b/frontend/resourceadm/components/LinkButton/LinkButton.tsx index 65d6ee0a94c..0dc241eb5b5 100644 --- a/frontend/resourceadm/components/LinkButton/LinkButton.tsx +++ b/frontend/resourceadm/components/LinkButton/LinkButton.tsx @@ -1,14 +1,18 @@ import type { ReactNode } from 'react'; import React from 'react'; -import classes from './LinkButton.module.css'; -import { Link } from '@digdir/designsystemet-react'; +import type { NavigationBarPage } from '../../types/NavigationBarPage'; +import { StudioLink } from '@studio/components'; export type LinkButtonProps = { + /** + * Page name + */ + page: NavigationBarPage; /** * Function to handle the click of the link * @returns void */ - onClick?: () => void; + onClick?: (page: NavigationBarPage) => void; /** * Children of the component */ @@ -29,10 +33,15 @@ export type LinkButtonProps = { * * @returns {React.JSX.Element} - The rendered component */ -export const LinkButton = ({ onClick, children }: LinkButtonProps): React.JSX.Element => { +export const LinkButton = ({ page, onClick, children }: LinkButtonProps): React.JSX.Element => { + const handleButtonClick = (event: React.MouseEvent): void => { + event.preventDefault(); + onClick(page); + }; + return ( - - - + + {children} + ); }; diff --git a/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.test.tsx b/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.test.tsx index b4230c70750..eaa238fdd03 100644 --- a/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.test.tsx +++ b/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.test.tsx @@ -66,9 +66,11 @@ const defaultProps: ResourceAccessListsProps = { const checkListMock = jest.fn(); const uncheckListMock = jest.fn(); +const mockedNavigate = jest.fn(); jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), + useNavigate: () => mockedNavigate, useParams: () => ({ org: org, }), @@ -199,6 +201,21 @@ describe('ResourceAccessLists', () => { ), ).toBeInTheDocument(); }); + + it('should navigate back on back button click', async () => { + const user = userEvent.setup(); + + renderResourceAccessLists(); + + const spinnerTitle = screen.queryByText(textMock('resourceadm.loading_lists')); + await waitForElementToBeRemoved(spinnerTitle); + + const backLink = screen.getByRole('link', { name: textMock('general.back') }); + await user.click(backLink); + + const expectedBackUrl = `/${org}/${org}-resources/resource/${resourceId}/about`; + expect(mockedNavigate).toHaveBeenCalledWith(expectedBackUrl); + }); }); const renderResourceAccessLists = (getResourceAccessListsMock?: jest.Mock) => { diff --git a/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.tsx b/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.tsx index 493ed4f7935..4278ab90cfe 100644 --- a/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.tsx +++ b/frontend/resourceadm/components/ResourceAccessLists/ResourceAccessLists.tsx @@ -1,9 +1,9 @@ import React, { useEffect, useState, useRef } from 'react'; -import { Link } from 'react-router-dom'; +import { useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; -import { Checkbox, Heading, Link as DigdirLink } from '@digdir/designsystemet-react'; +import { Checkbox, Heading } from '@digdir/designsystemet-react'; import classes from './ResourceAccessLists.module.css'; -import { StudioSpinner, StudioButton } from '@studio/components'; +import { StudioSpinner, StudioButton, StudioLink } from '@studio/components'; import { PencilWritingIcon, PlusIcon } from '@studio/icons'; import { useGetResourceAccessListsQuery } from '../../hooks/queries/useGetResourceAccessListsQuery'; import { useAddResourceAccessListMutation } from '../../hooks/mutations/useAddResourceAccessListMutation'; @@ -26,9 +26,11 @@ export const ResourceAccessLists = ({ resourceData, }: ResourceAccessListsProps): React.JSX.Element => { const { t } = useTranslation(); + const navigate = useNavigate(); const { org, app } = useUrlParams(); const createAccessListModalRef = useRef(null); + const backUrl = getResourcePageURL(org, app, resourceData.identifier, 'about'); const [selectedLists, setSelectedLists] = useState([]); @@ -72,6 +74,11 @@ export const ResourceAccessLists = ({ setSelectedLists((old) => [...old, listItemId]); }; + const handleBackClick = (event: React.MouseEvent): void => { + event.preventDefault(); + navigate(backUrl); + }; + if (isLoadingAccessLists) { return ; } @@ -94,11 +101,9 @@ export const ResourceAccessLists = ({ )}/${env}/`} onClose={() => createAccessListModalRef.current?.close()} /> - - - {t('general.back')} - - + + {t('general.back')} + {t('resourceadm.listadmin_resource_header', { resourceTitle: resourceData.title.nb, diff --git a/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.test.tsx b/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.test.tsx index 2311645beb4..407185dd881 100644 --- a/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.test.tsx +++ b/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.test.tsx @@ -68,14 +68,10 @@ describe('ResourceDeployStatus', () => { it('renders error messages with links when error is an array', () => { render(); - const firstErrorMessageLink = screen.getByRole('button', { - name: textMock(mockDeployError1.message), - }); + const firstErrorMessageLink = screen.getByText(textMock(mockDeployError1.message)); expect(firstErrorMessageLink).toBeInTheDocument(); - const secondErrorMessageLink = screen.getByRole('button', { - name: textMock(mockDeployError2.message), - }); + const secondErrorMessageLink = screen.getByText(textMock(mockDeployError2.message)); expect(secondErrorMessageLink).toBeInTheDocument(); }); @@ -84,9 +80,7 @@ describe('ResourceDeployStatus', () => { const user = userEvent.setup(); render(); - const linkButton = screen.getByRole('button', { - name: textMock(mockDeployError1.message), - }); + const linkButton = screen.getByText(textMock(mockDeployError1.message)); await user.click(linkButton); diff --git a/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.tsx b/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.tsx index 0b53158381a..871392559b1 100644 --- a/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.tsx +++ b/frontend/resourceadm/components/ResourceDeployStatus/ResourceDeployStatus.tsx @@ -63,7 +63,7 @@ export const ResourceDeployStatus = ({ - onNavigateToPageWithError(errorItem.pageWithError)} /> +
diff --git a/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.test.tsx b/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.test.tsx index 63c730f85d6..4fccf3c7e2f 100644 --- a/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.test.tsx +++ b/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.test.tsx @@ -137,6 +137,20 @@ describe('ListAdminPage', () => { ), ).toBeInTheDocument(); }); + + it('should navigate back on back button click', async () => { + (useParams as jest.Mock).mockReturnValue({ + org: 'ttd', + }); + + const user = userEvent.setup(); + renderListAdminPage(); + + const backLink = screen.getByRole('link', { name: textMock('resourceadm.listadmin_back') }); + await user.click(backLink); + + expect(mockedNavigate).toHaveBeenCalledWith('/ttd/ttd-resources'); + }); }); const renderListAdminPage = (isError?: boolean) => { diff --git a/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.tsx b/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.tsx index 1ab6bf10e60..df06aa7ccd1 100644 --- a/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.tsx +++ b/frontend/resourceadm/pages/ListAdminPage/ListAdminPage.tsx @@ -1,8 +1,8 @@ import React, { useRef, useEffect, useCallback } from 'react'; -import { Link, useNavigate } from 'react-router-dom'; +import { useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; -import { Heading, Link as DigdirLink, ToggleGroup } from '@digdir/designsystemet-react'; -import { StudioSpinner, StudioButton } from '@studio/components'; +import { Heading, ToggleGroup } from '@digdir/designsystemet-react'; +import { StudioSpinner, StudioButton, StudioLink } from '@studio/components'; import { PencilWritingIcon, PlusIcon } from '@studio/icons'; import classes from './ListAdminPage.module.css'; import { useGetAccessListsQuery } from '../../hooks/queries/useGetAccessListsQuery'; @@ -43,13 +43,18 @@ export const ListAdminPage = (): React.JSX.Element => { } }, [org, selectedEnv, navigateToListEnv]); + const handleBackClick = (event: React.MouseEvent): void => { + event.preventDefault(); + navigate(getResourceDashboardURL(org, app)); + }; + const createAccessListModalRef = useRef(null); return (
- - {t('resourceadm.listadmin_back')} - + + {t('resourceadm.listadmin_back')} + {t('resourceadm.listadmin_header')} From 9616e619bef23ab622f692631989185c719b2022 Mon Sep 17 00:00:00 2001 From: William Thorenfeldt <48119543+wrt95@users.noreply.github.com> Date: Wed, 20 Nov 2024 09:44:46 +0100 Subject: [PATCH 02/18] refactor: Replace table and checkbox with StudioCheckboxTable in accessControl tab (#14112) --- .../SelectAllowedPartyTypes.module.css | 4 -- .../SelectAllowedPartyTypes.tsx | 70 +++++++------------ .../StudioCheckboxTableRow.module.css | 4 ++ .../StudioCheckboxTableRow.tsx | 2 +- 4 files changed, 32 insertions(+), 48 deletions(-) diff --git a/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.module.css b/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.module.css index a21b976bd23..6d35050a332 100644 --- a/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.module.css +++ b/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.module.css @@ -6,7 +6,3 @@ .header { background-color: var(--fds-colors-grey-100); } - -.checkboxContent { - width: 3rem; -} diff --git a/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.tsx b/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.tsx index a169a474cd5..4800cb26b1f 100644 --- a/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.tsx +++ b/frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AccessControlTab/SelectAllowedPartyTypes/SelectAllowedPartyTypes.tsx @@ -1,5 +1,4 @@ -import React, { useRef } from 'react'; -import { Table, Checkbox } from '@digdir/designsystemet-react'; +import React, { type ReactElement, useRef } from 'react'; import type { ApplicationMetadata, PartyTypesAllowed } from 'app-shared/types/ApplicationMetadata'; import classes from './SelectAllowedPartyTypes.module.css'; import { useTranslation } from 'react-i18next'; @@ -7,12 +6,15 @@ import { getPartyTypesAllowedOptions } from '../../../../utils/tabUtils/accessCo import { useAppMetadataMutation } from 'app-development/hooks/mutations'; import { AccessControlWarningModal } from '../AccessControWarningModal'; import { useStudioEnvironmentParams } from 'app-shared/hooks/useStudioEnvironmentParams'; +import { StudioCheckboxTable } from '@studio/components'; -export interface SelectAllowedPartyTypesProps { +export type SelectAllowedPartyTypesProps = { appMetadata: ApplicationMetadata; -} +}; -export const SelectAllowedPartyTypes = ({ appMetadata }: SelectAllowedPartyTypesProps) => { +export const SelectAllowedPartyTypes = ({ + appMetadata, +}: SelectAllowedPartyTypesProps): ReactElement => { const { t } = useTranslation(); const { org, app } = useStudioEnvironmentParams(); @@ -59,45 +61,27 @@ export const SelectAllowedPartyTypes = ({ appMetadata }: SelectAllowedPartyTypes return ( <> - - - - - - - - {t('settings_modal.access_control_tab_option_all_types')} - - - - - {getPartyTypesAllowedOptions().map((mappedOption, key) => ( - - - - handleAllowedPartyTypeChange(partyTypesAllowed, mappedOption.value) - } - size='small' - value={mappedOption.value} - checked={partyTypesAllowed[mappedOption.value] || isNoCheckboxesChecked} - /> - - {t(mappedOption.label)} - + + + + {getPartyTypesAllowedOptions().map((mappedOption) => ( + handleAllowedPartyTypeChange(partyTypesAllowed, mappedOption.value)} + /> ))} - -
+ + ); diff --git a/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.module.css b/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.module.css index b622b9c5f9e..47482c8e3a5 100644 --- a/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.module.css +++ b/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.module.css @@ -5,3 +5,7 @@ .descriptionText { color: var(--fds-semantic-text-neutral-subtle); } + +.chexboxTextContent { + width: 100%; +} diff --git a/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.tsx b/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.tsx index 36f3f1df02a..4081a658947 100644 --- a/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.tsx +++ b/frontend/libs/studio-components/src/components/StudioCheckboxTable/StudioCheckboxTableRow/StudioCheckboxTableRow.tsx @@ -27,7 +27,7 @@ export const StudioCheckboxTableRow = ({ checked={checked} /> - + {label} {description && ( From 8f11d67315d31e560569fb2c8ba5775dab75962a Mon Sep 17 00:00:00 2001 From: andreastanderen <71079896+standeren@users.noreply.github.com> Date: Wed, 20 Nov 2024 09:56:55 +0100 Subject: [PATCH 03/18] fix: use gitRepoMethods to move files/dirs in appGitRepo (#14113) --- .../GitRepository/AltinnAppGitRepository.cs | 27 ++--------- .../GitRepository/GitRepository.cs | 45 +++++++++++++++---- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs b/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs index 7299c3c512b..93ddf76bcc4 100644 --- a/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs +++ b/backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs @@ -425,20 +425,9 @@ public string[] GetLayoutSetNames() public void ChangeLayoutSetFolderName(string oldLayoutSetName, string newLayoutSetName, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (DirectoryExistsByRelativePath(GetPathToLayoutSet(newLayoutSetName))) - { - throw new NonUniqueLayoutSetIdException("Suggested new layout set name already exist"); - } - string destAbsolutePath = GetAbsoluteFileOrDirectoryPathSanitized(GetPathToLayoutSet(newLayoutSetName, true)); - - string sourceRelativePath = GetPathToLayoutSet(oldLayoutSetName, true); - if (!DirectoryExistsByRelativePath(sourceRelativePath)) - { - throw new NotFoundException("Layout set you are trying to change doesn't exist"); - } - - string sourceAbsolutePath = GetAbsoluteFileOrDirectoryPathSanitized(sourceRelativePath); - Directory.Move(sourceAbsolutePath, destAbsolutePath); + string currentDirectoryPath = GetPathToLayoutSet(oldLayoutSetName, true); + string newDirectoryPath = GetPathToLayoutSet(newLayoutSetName, true); + MoveDirectoryByRelativePath(currentDirectoryPath, newDirectoryPath); } /// @@ -575,15 +564,7 @@ public void UpdateFormLayoutName(string layoutSetName, string layoutFileName, st { string currentFilePath = GetPathToLayoutFile(layoutSetName, layoutFileName); string newFilePath = GetPathToLayoutFile(layoutSetName, newFileName); - if (!FileExistsByRelativePath(currentFilePath)) - { - throw new FileNotFoundException("Layout does not exist."); - } - if (FileExistsByRelativePath(newFilePath)) - { - throw new ArgumentException("New layout name must be unique."); - } - File.Move(GetAbsoluteFileOrDirectoryPathSanitized(currentFilePath), GetAbsoluteFileOrDirectoryPathSanitized(newFilePath)); + MoveFileByRelativePath(currentFilePath, newFilePath, newFileName); } public async Task GetLayoutSetsFile(CancellationToken cancellationToken = default) diff --git a/backend/src/Designer/Infrastructure/GitRepository/GitRepository.cs b/backend/src/Designer/Infrastructure/GitRepository/GitRepository.cs index 6b78fe844ba..4b999c8cc03 100644 --- a/backend/src/Designer/Infrastructure/GitRepository/GitRepository.cs +++ b/backend/src/Designer/Infrastructure/GitRepository/GitRepository.cs @@ -254,19 +254,46 @@ public void DeleteFileByAbsolutePath(string absoluteFilePath) /// FileName for the destination file protected void MoveFileByRelativePath(string sourceRelativeFilePath, string destRelativeFilePath, string destinationFileName) { - if (FileExistsByRelativePath(sourceRelativeFilePath)) + if (!FileExistsByRelativePath(sourceRelativeFilePath)) { - Guard.AssertNotNullOrEmpty(sourceRelativeFilePath, nameof(sourceRelativeFilePath)); + throw new FileNotFoundException($"File {sourceRelativeFilePath} does not exist."); + } - string sourceAbsoluteFilePath = GetAbsoluteFileOrDirectoryPathSanitized(sourceRelativeFilePath); - string destAbsoluteFilePath = GetAbsoluteFileOrDirectoryPathSanitized(destRelativeFilePath); - string destAbsoluteParentDirPath = destAbsoluteFilePath.Remove(destAbsoluteFilePath.IndexOf(destinationFileName, StringComparison.Ordinal)); - Directory.CreateDirectory(destAbsoluteParentDirPath); - Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, sourceAbsoluteFilePath); - Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, destAbsoluteFilePath); + if (FileExistsByRelativePath(destRelativeFilePath)) + { + throw new IOException($"Suggested file name {destinationFileName} already exists."); + } + string sourceAbsoluteFilePath = GetAbsoluteFileOrDirectoryPathSanitized(sourceRelativeFilePath); + string destAbsoluteFilePath = GetAbsoluteFileOrDirectoryPathSanitized(destRelativeFilePath); + string destAbsoluteParentDirPath = destAbsoluteFilePath.Remove(destAbsoluteFilePath.IndexOf(destinationFileName, StringComparison.Ordinal)); + Directory.CreateDirectory(destAbsoluteParentDirPath); + Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, sourceAbsoluteFilePath); + Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, destAbsoluteFilePath); + + File.Move(sourceAbsoluteFilePath, destAbsoluteFilePath); + } - File.Move(sourceAbsoluteFilePath, destAbsoluteFilePath); + /// + /// Move the specified folder to specified destination + /// + /// Relative path to folder to be moved. + /// Relative path to destination of moved folder. + protected void MoveDirectoryByRelativePath(string sourceRelativeDirectoryPath, string destRelativeDirectoryPath) + { + if (!DirectoryExistsByRelativePath(sourceRelativeDirectoryPath)) + { + throw new DirectoryNotFoundException($"Directory {sourceRelativeDirectoryPath} does not exist."); } + if (DirectoryExistsByRelativePath(destRelativeDirectoryPath)) + { + throw new IOException($"Suggested directory {destRelativeDirectoryPath} already exists."); + } + string sourceAbsoluteDirectoryPath = GetAbsoluteFileOrDirectoryPathSanitized(sourceRelativeDirectoryPath); + string destAbsoluteDirectoryPath = GetAbsoluteFileOrDirectoryPathSanitized(destRelativeDirectoryPath); + Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, sourceAbsoluteDirectoryPath); + Guard.AssertFilePathWithinParentDirectory(RepositoryDirectory, destAbsoluteDirectoryPath); + + Directory.Move(sourceAbsoluteDirectoryPath, destAbsoluteDirectoryPath); } /// From d362ec28118d33f2d874de15cba02c79c9c51763 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 20 Nov 2024 10:33:20 +0100 Subject: [PATCH 04/18] fix: Fixed empty data model bindings (#14101) --- frontend/packages/ux-editor/src/hooks/useValidDataModels.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/ux-editor/src/hooks/useValidDataModels.ts b/frontend/packages/ux-editor/src/hooks/useValidDataModels.ts index 1bb5717757b..8e2b05c6665 100644 --- a/frontend/packages/ux-editor/src/hooks/useValidDataModels.ts +++ b/frontend/packages/ux-editor/src/hooks/useValidDataModels.ts @@ -22,7 +22,7 @@ export const useValidDataModels = (currentDataModel: string) => { org, app, layoutSetName: selectedFormLayoutSetName, - dataModelName: isDataModelValid ? currentDataModel : '', + dataModelName: isDataModelValid && currentDataModel ? currentDataModel : dataModels?.[0], }, { enabled: !isPendingDataModels && !isFetchingDataModels }, ); From 7840b40849dd11afc7e3d235450d0175fd561d3a Mon Sep 17 00:00:00 2001 From: Nina Kylstad Date: Wed, 20 Nov 2024 13:34:15 +0100 Subject: [PATCH 05/18] chore(ui-editor): Remove LikertItem component (#14108) Co-authored-by: William Thorenfeldt <48119543+wrt95@users.noreply.github.com> --- frontend/language/src/nb.json | 1 - .../src/types/ComponentSpecificConfig.ts | 6 - .../shared/src/types/ComponentType.ts | 1 - .../config/Expressions/utils/utils.ts | 1 - .../ux-editor/src/data/formItemConfig.ts | 12 - .../src/testing/componentSchemaMocks.ts | 2 - .../json/component/LikertItem.schema.v1.json | 265 ------------------ 7 files changed, 288 deletions(-) delete mode 100644 frontend/packages/ux-editor/src/testing/schemas/json/component/LikertItem.schema.v1.json diff --git a/frontend/language/src/nb.json b/frontend/language/src/nb.json index d534e2dc7a0..36ff8751b4f 100644 --- a/frontend/language/src/nb.json +++ b/frontend/language/src/nb.json @@ -1432,7 +1432,6 @@ "ux_editor.component_title.InstanceInformation": "Informasjon om eksemplaret", "ux_editor.component_title.InstantiationButton": "Start eksemplar", "ux_editor.component_title.Likert": "Likert-skala", - "ux_editor.component_title.LikertItem": "Likertelement", "ux_editor.component_title.Link": "Lenke", "ux_editor.component_title.List": "Liste", "ux_editor.component_title.Map": "Stedfeste i kart", diff --git a/frontend/packages/shared/src/types/ComponentSpecificConfig.ts b/frontend/packages/shared/src/types/ComponentSpecificConfig.ts index d36b87df446..9dbccf45a19 100644 --- a/frontend/packages/shared/src/types/ComponentSpecificConfig.ts +++ b/frontend/packages/shared/src/types/ComponentSpecificConfig.ts @@ -285,12 +285,6 @@ export type ComponentSpecificConfig = { dataModelBindings: DataModelBindingsLikert; filter?: { key: 'start' | 'stop'; value: string | number }; }; - [ComponentType.LikertItem]: FormComponentProps & - SummarizableComponentProps & - SelectionComponentFull & { - dataModelBindings: DataModelBindingsOptionsSimple; - layout?: LayoutStyle; - }; [ComponentType.Link]: { style: 'primary' | 'secondary' | 'link'; openInNewTab?: boolean; diff --git a/frontend/packages/shared/src/types/ComponentType.ts b/frontend/packages/shared/src/types/ComponentType.ts index 7fa28585a34..51f7b0b46bc 100644 --- a/frontend/packages/shared/src/types/ComponentType.ts +++ b/frontend/packages/shared/src/types/ComponentType.ts @@ -23,7 +23,6 @@ export enum ComponentType { InstanceInformation = 'InstanceInformation', InstantiationButton = 'InstantiationButton', Likert = 'Likert', - LikertItem = 'LikertItem', Link = 'Link', List = 'List', Map = 'Map', diff --git a/frontend/packages/ux-editor/src/components/config/Expressions/utils/utils.ts b/frontend/packages/ux-editor/src/components/config/Expressions/utils/utils.ts index fb78014ef42..2cbe6c93c21 100644 --- a/frontend/packages/ux-editor/src/components/config/Expressions/utils/utils.ts +++ b/frontend/packages/ux-editor/src/components/config/Expressions/utils/utils.ts @@ -18,7 +18,6 @@ export const expressionPropertiesOnFormItem = ( case ComponentType.FileUploadWithTag: case ComponentType.Input: case ComponentType.Likert: - case ComponentType.LikertItem: case ComponentType.List: case ComponentType.Map: case ComponentType.MultipleSelect: diff --git a/frontend/packages/ux-editor/src/data/formItemConfig.ts b/frontend/packages/ux-editor/src/data/formItemConfig.ts index c55b1edb32a..11cfd67c746 100644 --- a/frontend/packages/ux-editor/src/data/formItemConfig.ts +++ b/frontend/packages/ux-editor/src/data/formItemConfig.ts @@ -333,17 +333,6 @@ export const formItemConfigs: FormItemConfigs = { propertyPath: 'definitions/radioAndCheckboxComponents', icon: LikertIcon, }, - [ComponentType.LikertItem]: { - name: ComponentType.LikertItem, - itemType: LayoutItemType.Component, - defaultProperties: { - dataModelBindings: { - simpleBinding: '', - }, - }, - propertyPath: 'definitions/radioAndCheckboxComponents', - icon: LikertIcon, - }, [ComponentType.Link]: { name: ComponentType.Link, itemType: LayoutItemType.Component, @@ -533,7 +522,6 @@ export const schemaComponents: FormItemConfigs[ComponentType][] = [ formItemConfigs[ComponentType.Dropdown], formItemConfigs[ComponentType.MultipleSelect], formItemConfigs[ComponentType.Likert], - formItemConfigs[ComponentType.LikertItem], formItemConfigs[ComponentType.Datepicker], formItemConfigs[ComponentType.FileUpload], formItemConfigs[ComponentType.FileUploadWithTag], diff --git a/frontend/packages/ux-editor/src/testing/componentSchemaMocks.ts b/frontend/packages/ux-editor/src/testing/componentSchemaMocks.ts index 3fd45f481e1..8db5480c7b5 100644 --- a/frontend/packages/ux-editor/src/testing/componentSchemaMocks.ts +++ b/frontend/packages/ux-editor/src/testing/componentSchemaMocks.ts @@ -22,7 +22,6 @@ import InputSchema from './schemas/json/component/Input.schema.v1.json'; import InstanceInformationSchema from './schemas/json/component/InstanceInformation.schema.v1.json'; import InstantiationButtonSchema from './schemas/json/component/InstantiationButton.schema.v1.json'; import LikertSchema from './schemas/json/component/Likert.schema.v1.json'; -import LikertItemSchema from './schemas/json/component/LikertItem.schema.v1.json'; import LinkSchema from './schemas/json/component/Link.schema.v1.json'; import ListSchema from './schemas/json/component/List.schema.v1.json'; import MapSchema from './schemas/json/component/Map.schema.v1.json'; @@ -68,7 +67,6 @@ export const componentSchemaMocks: Record = { [ComponentType.InstanceInformation]: InstanceInformationSchema, [ComponentType.InstantiationButton]: InstantiationButtonSchema, [ComponentType.Likert]: LikertSchema, - [ComponentType.LikertItem]: LikertItemSchema, [ComponentType.Link]: LinkSchema, [ComponentType.List]: ListSchema, [ComponentType.Map]: MapSchema, diff --git a/frontend/packages/ux-editor/src/testing/schemas/json/component/LikertItem.schema.v1.json b/frontend/packages/ux-editor/src/testing/schemas/json/component/LikertItem.schema.v1.json deleted file mode 100644 index dddb7c22ae5..00000000000 --- a/frontend/packages/ux-editor/src/testing/schemas/json/component/LikertItem.schema.v1.json +++ /dev/null @@ -1,265 +0,0 @@ -{ - "$id": "https://altinncdn.no/schemas/json/component/LikertItem.schema.v1.json", - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "id": { - "title": "ID", - "description": "The component ID. Must be unique within all layouts/pages in a layout-set. Cannot end with .", - "type": "string", - "pattern": "^[0-9a-zA-Z][0-9a-zA-Z-]*(-?[a-zA-Z]+|[a-zA-Z][0-9]+|-[0-9]{6,})$" - }, - "hidden": { - "title": "Hidden", - "description": "Boolean value or expression indicating if the component should be hidden. Defaults to false.", - "default": false, - "$ref": "expression.schema.v1.json#/definitions/boolean" - }, - "grid": { - "properties": { - "xs": { "$ref": "#/definitions/IGridSize" }, - "sm": { "$ref": "#/definitions/IGridSize" }, - "md": { "$ref": "#/definitions/IGridSize" }, - "lg": { "$ref": "#/definitions/IGridSize" }, - "xl": { "$ref": "#/definitions/IGridSize" }, - "labelGrid": { "$ref": "#/definitions/IGridStyling" }, - "innerGrid": { "$ref": "#/definitions/IGridStyling" } - } - }, - "pageBreak": { - "title": "Page break", - "description": "Optionally insert page-break before/after component when rendered in PDF", - "type": "object", - "properties": { - "breakBefore": { - "title": "Page break before", - "description": "PDF only: Value or expression indicating whether a page break should be added before the component. Can be either: 'auto' (default), 'always', or 'avoid'.", - "examples": ["auto", "always", "avoid"], - "default": "auto", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "breakAfter": { - "title": "Page break after", - "description": "PDF only: Value or expression indicating whether a page break should be added after the component. Can be either: 'auto' (default), 'always', or 'avoid'.", - "examples": ["auto", "always", "avoid"], - "default": "auto", - "$ref": "expression.schema.v1.json#/definitions/string" - } - }, - "additionalProperties": false - }, - "readOnly": { - "title": "Read only/disabled?", - "description": "Boolean value or expression indicating if the component should be read only/disabled. Defaults to false.
Please note that even with read-only fields in components, it may currently be possible to update the field by modifying the request sent to the API or through a direct API call.", - "default": false, - "$ref": "expression.schema.v1.json#/definitions/boolean" - }, - "required": { - "title": "Required?", - "description": "Boolean value or expression indicating if the component should be required. Defaults to false.", - "default": false, - "$ref": "expression.schema.v1.json#/definitions/boolean" - }, - "showValidations": { - "title": "Validation types", - "description": "List of validation types to show", - "type": "array", - "items": { - "enum": [ - "Schema", - "Component", - "Expression", - "CustomBackend", - "Required", - "AllExceptRequired", - "All" - ], - "type": "string" - } - }, - "renderAsSummary": { - "title": "Render as summary", - "description": "Boolean value indicating if the component should be rendered as a summary. Defaults to false.", - "default": false, - "type": "boolean" - }, - "forceShowInSummary": { - "title": "Force show in summary", - "description": "Will force show the component in a summary even if hideEmptyFields is set to true in the summary component.", - "default": false, - "$ref": "expression.schema.v1.json#/definitions/boolean" - }, - "optionsId": { - "title": "Dynamic options (fetched from server)", - "description": "ID of the option list to fetch from the server", - "type": "string" - }, - "mapping": { - "title": "Mapping", - "description": "A mapping of key-value pairs (usually used for mapping a path in the data model to a query string parameter).", - "type": "object", - "properties": {}, - "additionalProperties": { "type": "string" } - }, - "queryParameters": { - "title": "Query parameters", - "description": "A mapping of query string parameters to values. Will be appended to the URL when fetching options.", - "type": "object", - "properties": {}, - "additionalProperties": { "type": "string" } - }, - "options": { - "title": "Static options", - "description": "List of static options", - "type": "array", - "items": { - "title": "IRawOption", - "examples": [{ "label": "", "value": "" }], - "type": "object", - "properties": { - "label": { "type": "string" }, - "value": { - "anyOf": [ - { "type": "string" }, - { "type": "number" }, - { "type": "boolean" }, - { "const": null } - ] - }, - "description": { "type": "string" }, - "helpText": { "type": "string" } - }, - "required": ["label", "value"], - "additionalProperties": false - } - }, - "secure": { - "title": "Secure options (when using optionsId)", - "description": "Whether to call the secure API endpoint when fetching options from the server (allows for user/instance-specific options)", - "default": false, - "type": "boolean" - }, - "sortOrder": { - "description": "Sorts the code list in either ascending or descending order by label.", - "enum": ["asc", "desc"], - "type": "string" - }, - "source": { - "title": "Option source", - "description": "Allows for fetching options from the data model, pointing to a repeating group structure", - "type": "object", - "properties": { - "group": { - "title": "Group", - "description": "The repeating group to base options on.", - "examples": ["model.some.group"], - "type": "string" - }, - "label": { - "title": "Label", - "description": "A label of the option displayed in Radio- and Checkbox groups. Can be plain text, a text resource binding, or a dynamic expression.", - "examples": ["some.text.key"], - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "value": { - "title": "Value", - "description": "Field in the group that should be used as value", - "examples": ["model.some.group[{0}].someField"], - "type": "string" - }, - "description": { - "title": "Description", - "description": "A description of the option displayed in Radio- and Checkbox groups. Can be plain text, a text resource binding, or a dynamic expression.", - "examples": ["some.text.key", "My Description"], - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "helpText": { - "title": "Help Text", - "description": "A help text for the option displayed in Radio- and Checkbox groups. Can be plain text, a text resource binding, or a dynamic expression.", - "examples": ["some.text.key", "My Help Text"], - "$ref": "expression.schema.v1.json#/definitions/string" - } - }, - "required": ["group", "label", "value"], - "additionalProperties": false - }, - "preselectedOptionIndex": { - "title": "Preselected option index", - "description": "Index of the option to preselect (if no option has been selected yet)", - "type": "integer" - }, - "type": { "const": "LikertItem" }, - "textResourceBindings": { - "properties": { - "title": { - "title": "Title", - "description": "Title of the Likert component/row", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "description": { - "title": "Description", - "description": "Description of the Likert component/row", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "help": { - "title": "Help", - "description": "Help text of the Likert component/row", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "tableTitle": { - "title": "Table title", - "description": "Title used in the table view (overrides the default title)", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "shortName": { - "title": "Short name (for validation)", - "description": "Alternative name used for required validation messages (overrides the default title)", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "requiredValidation": { - "title": "Required validation message", - "description": "Full validation message shown when the component is required and no value has been entered (overrides both the default and shortName)", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "summaryTitle": { - "title": "Summary title", - "description": "Title used in the summary view (overrides the default title)", - "$ref": "expression.schema.v1.json#/definitions/string" - }, - "summaryAccessibleTitle": { - "title": "Accessible summary title", - "description": "Title used for aria-label on the edit button in the summary view (overrides the default and summary title)", - "$ref": "expression.schema.v1.json#/definitions/string" - } - } - }, - "dataModelBindings": { - "title": "Data model binding", - "description": "Describes the location in the data model where the component should store its value(s). A simple binding is used for components that only store a single value, usually a string.", - "type": "object", - "properties": { - "simpleBinding": { "type": "string" }, - "label": { "type": "string" }, - "metadata": { - "description": "Describes the location where metadata for the option based component should be stored in the datamodel.", - "type": "string" - } - }, - "required": ["simpleBinding"], - "additionalProperties": false - }, - "showLabelsInTable": { - "title": "Show label when single option in table", - "description": "Boolean value indicating if the label should be visible when only one option exists in table", - "default": false, - "type": "boolean" - }, - "layout": { - "title": "Layout", - "description": "Define the layout style for the options", - "enum": ["column", "row", "table"], - "type": "string" - } - }, - "required": ["id", "type", "dataModelBindings"], - "title": "LikertItem component schema" -} From eb8f6139192424d4e121fe8bcf9cc2fbc9ccedcd Mon Sep 17 00:00:00 2001 From: andreastanderen <71079896+standeren@users.noreply.github.com> Date: Thu, 21 Nov 2024 08:56:39 +0100 Subject: [PATCH 06/18] fix: Add custom attribute for OptionValue and OptionLabel and allow empty string (#14035) --- .../Options/InvalidOptionsFormatException.cs | 25 ++ .../Filters/Options/OptionsErrorCodes.cs | 6 + .../OptionsExceptionFilterAttribute.cs | 26 +++ .../Filters/ProblemDetailsExtensionsCodes.cs | 2 +- .../NotNullableAttribute.cs | 15 ++ .../OptionConverterHelper.cs | 7 +- .../Infrastructure/MvcConfiguration.cs | 2 + backend/src/Designer/Models/Option.cs | 11 +- .../Services/Implementation/OptionsService.cs | 36 +-- .../GetOptionListIdsTests.cs | 3 +- .../OptionsController/GetOptionsTests.cs | 26 ++- .../OptionsController/UpdateOptionsTests.cs | 218 ++++++++++-------- .../OptionsController/UploadOptionsTests.cs | 196 ++++++++++++++++ .../AltinnAppGitRepositoryTests.cs | 2 +- .../Services/OptionsServiceTests.cs | 15 +- .../App/options/options-with-null-fields.json | 6 + frontend/language/src/nb.json | 1 + 17 files changed, 463 insertions(+), 134 deletions(-) create mode 100644 backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs create mode 100644 backend/src/Designer/Filters/Options/OptionsErrorCodes.cs create mode 100644 backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs create mode 100644 backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs create mode 100644 backend/tests/Designer.Tests/Controllers/OptionsController/UploadOptionsTests.cs create mode 100644 backend/tests/Designer.Tests/_TestData/Repositories/testUser/ttd/app-with-options/App/options/options-with-null-fields.json diff --git a/backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs b/backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs new file mode 100644 index 00000000000..0dc69bc495d --- /dev/null +++ b/backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs @@ -0,0 +1,25 @@ +using System; + +namespace Altinn.Studio.Designer.Exceptions.Options; + +/// +/// Indicates that an error occurred during json serialization of options. +/// +[Serializable] +public class InvalidOptionsFormatException : Exception +{ + /// + public InvalidOptionsFormatException() + { + } + + /// + public InvalidOptionsFormatException(string message) : base(message) + { + } + + /// + public InvalidOptionsFormatException(string message, Exception innerException) : base(message, innerException) + { + } +} diff --git a/backend/src/Designer/Filters/Options/OptionsErrorCodes.cs b/backend/src/Designer/Filters/Options/OptionsErrorCodes.cs new file mode 100644 index 00000000000..9053087b1b5 --- /dev/null +++ b/backend/src/Designer/Filters/Options/OptionsErrorCodes.cs @@ -0,0 +1,6 @@ +namespace Altinn.Studio.Designer.Filters.Options; + +public class OptionsErrorCodes +{ + public const string InvalidOptionsFormat = nameof(InvalidOptionsFormat); +} diff --git a/backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs b/backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs new file mode 100644 index 00000000000..ead5e0a589a --- /dev/null +++ b/backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs @@ -0,0 +1,26 @@ +using System.Net; +using Altinn.Studio.Designer.Exceptions.Options; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Altinn.Studio.Designer.Filters.Options; + +public class OptionsExceptionFilterAttribute : ExceptionFilterAttribute +{ + public override void OnException(ExceptionContext context) + { + base.OnException(context); + + if (context.ActionDescriptor is not ControllerActionDescriptor) + { + return; + } + + if (context.Exception is InvalidOptionsFormatException) + { + context.Result = new ObjectResult(ProblemDetailsUtils.GenerateProblemDetails(context.Exception, OptionsErrorCodes.InvalidOptionsFormat, HttpStatusCode.BadRequest)) { StatusCode = (int)HttpStatusCode.BadRequest }; + } + } + +} diff --git a/backend/src/Designer/Filters/ProblemDetailsExtensionsCodes.cs b/backend/src/Designer/Filters/ProblemDetailsExtensionsCodes.cs index 4064393c200..a70c0f6f152 100644 --- a/backend/src/Designer/Filters/ProblemDetailsExtensionsCodes.cs +++ b/backend/src/Designer/Filters/ProblemDetailsExtensionsCodes.cs @@ -3,5 +3,5 @@ namespace Altinn.Studio.Designer.Filters; public static class ProblemDetailsExtensionsCodes { public const string ErrorCode = "errorCode"; - + public const string Detail = "detail"; } diff --git a/backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs b/backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs new file mode 100644 index 00000000000..c4a556f977c --- /dev/null +++ b/backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs @@ -0,0 +1,15 @@ +using System.ComponentModel.DataAnnotations; + +namespace Altinn.Studio.Designer.Helpers.JsonConverterHelpers; + +public class NotNullableAttribute : ValidationAttribute +{ + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + if (value == null) + { + return new ValidationResult("The field is required."); + } + return ValidationResult.Success; + } +} diff --git a/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs b/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs index a15ad94deeb..9ba74b172ef 100644 --- a/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs +++ b/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs @@ -1,10 +1,11 @@ using System; using System.Text.Json; using System.Text.Json.Serialization; +using Altinn.Studio.Designer.Exceptions.Options; namespace Altinn.Studio.Designer.Helpers.JsonConverterHelpers; -public class OptionConverter : JsonConverter +public class OptionValueConverter : JsonConverter { public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { @@ -14,7 +15,7 @@ public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonS JsonTokenType.Number when reader.TryGetDouble(out double d) => d, JsonTokenType.True => true, JsonTokenType.False => false, - _ => throw new JsonException($"Unsupported JSON token for Option.Value: {reader.TokenType}") + _ => throw new InvalidOptionsFormatException($"Unsupported JSON token for Value field, {typeToConvert}: {reader.TokenType}.") }; } @@ -32,7 +33,7 @@ public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOp writer.WriteBooleanValue(b); break; default: - throw new JsonException("Unsupported type for Option.Value."); + throw new InvalidOptionsFormatException($"{value} is an unsupported type for Value field. Accepted types are string, double and bool."); } } } diff --git a/backend/src/Designer/Infrastructure/MvcConfiguration.cs b/backend/src/Designer/Infrastructure/MvcConfiguration.cs index b135b5a35cc..990b05a5498 100644 --- a/backend/src/Designer/Infrastructure/MvcConfiguration.cs +++ b/backend/src/Designer/Infrastructure/MvcConfiguration.cs @@ -2,6 +2,7 @@ using Altinn.Studio.Designer.Filters.DataModeling; using Altinn.Studio.Designer.Filters.Git; using Altinn.Studio.Designer.Filters.IO; +using Altinn.Studio.Designer.Filters.Options; using Altinn.Studio.Designer.ModelBinding; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.DependencyInjection; @@ -26,6 +27,7 @@ public static IServiceCollection ConfigureMvc(this IServiceCollection services) options.Filters.Add(typeof(DataModelingExceptionFilterAttribute)); options.Filters.Add(typeof(GitExceptionFilterAttribute)); options.Filters.Add(typeof(IoExceptionFilterAttribute)); + options.Filters.Add(typeof(OptionsExceptionFilterAttribute)); }) .AddNewtonsoftJson(options => options.SerializerSettings.Converters.Add(new Newtonsoft.Json.Converters.StringEnumConverter())); diff --git a/backend/src/Designer/Models/Option.cs b/backend/src/Designer/Models/Option.cs index ddfa6b9559e..b49a499ad28 100644 --- a/backend/src/Designer/Models/Option.cs +++ b/backend/src/Designer/Models/Option.cs @@ -1,4 +1,3 @@ -using System.ComponentModel.DataAnnotations; using System.Text.Json.Serialization; using Altinn.Studio.Designer.Helpers.JsonConverterHelpers; @@ -12,17 +11,17 @@ public class Option /// /// Value that connects the option to the data model. /// - [Required] + [NotNullable] [JsonPropertyName("value")] - [JsonConverter(typeof(OptionConverter))] - public object Value { get; set; } + [JsonConverter(typeof(OptionValueConverter))] + public required object Value { get; set; } /// /// Label to present to the user. /// - [Required] + [NotNullable] [JsonPropertyName("label")] - public string Label { get; set; } + public required string Label { get; set; } /// /// Description, typically displayed below the label. diff --git a/backend/src/Designer/Services/Implementation/OptionsService.cs b/backend/src/Designer/Services/Implementation/OptionsService.cs index f1834c3156a..137b0cf5c34 100644 --- a/backend/src/Designer/Services/Implementation/OptionsService.cs +++ b/backend/src/Designer/Services/Implementation/OptionsService.cs @@ -1,8 +1,9 @@ using System.Collections.Generic; -using System.Linq; +using System.ComponentModel.DataAnnotations; using System.Text.Json; using System.Threading; using System.Threading.Tasks; +using Altinn.Studio.Designer.Exceptions.Options; using Altinn.Studio.Designer.Models; using Altinn.Studio.Designer.Services.Interfaces; using LibGit2Sharp; @@ -50,9 +51,26 @@ public async Task> GetOptionsList(string org, string repo, string d string optionsListString = await altinnAppGitRepository.GetOptionsList(optionsListId, cancellationToken); var optionsList = JsonSerializer.Deserialize>(optionsListString); + + try + { + optionsList.ForEach(ValidateOption); + } + catch (ValidationException) + { + throw new InvalidOptionsFormatException($"One or more of the options have an invalid format in option list: {optionsListId}."); + } + + return optionsList; } + private void ValidateOption(Option option) + { + var validationContext = new ValidationContext(option); + Validator.ValidateObject(option, validationContext, validateAllProperties: true); + } + /// public async Task> CreateOrOverwriteOptionsList(string org, string repo, string developer, string optionsListId, List