Skip to content

Commit

Permalink
Select correct owner when creating a new app (#12612)
Browse files Browse the repository at this point in the history
* Set selected org as default owner when creating a new app

* Add unit tests

* Select current user when org is invalid (e.g. all)

* Redirect to right context when clicking cancel button

* Add missing tests

* Move DASHBOARD_ROOT_ROUTE into shared/src/constants.js

* Add unit tests for PageLayout
  • Loading branch information
mlqn authored Apr 5, 2024
1 parent cd168f2 commit 74c6ba1
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 8 deletions.
3 changes: 2 additions & 1 deletion frontend/dashboard/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ErrorMessage } from 'dashboard/components/ErrorMessage';
import './App.css';
import { PageLayout } from 'dashboard/pages/PageLayout';
import { useTranslation } from 'react-i18next';
import { DASHBOARD_ROOT_ROUTE } from 'app-shared/constants';

export const App = (): JSX.Element => {
const { t } = useTranslation();
Expand Down Expand Up @@ -51,7 +52,7 @@ export const App = (): JSX.Element => {
return (
<div className={classes.root}>
<Routes>
<Route element={<PageLayout />}>
<Route path={DASHBOARD_ROOT_ROUTE} element={<PageLayout />}>
<Route
path='/:selectedContext?'
element={<Dashboard user={user} organizations={organizations} />}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import React from 'react';
import { render as rtlRender, screen, within } from '@testing-library/react';
import type { ServiceOwnerSelectorProps } from './ServiceOwnerSelector';
import { ServiceOwnerSelector } from './ServiceOwnerSelector';
import { textMock } from '../../../testing/mocks/i18nMock';
import { user } from 'app-shared/mocks/mocks';

const defaultProps = {
selectedOrgOrUser: 'userLogin',
user: {
...user,
login: 'userLogin',
},
organizations: [
{
avatar_url: '',
id: 1,
username: 'organizationUsername',
},
],
errorMessage: '',
name: '',
};

const render = (props: Partial<ServiceOwnerSelectorProps> = {}) => {
rtlRender(<ServiceOwnerSelector {...defaultProps} {...props} />);
};

describe('ServiceOwnerSelector', () => {
it('renders select with all options', async () => {
render();

const select = screen.getByLabelText(textMock('general.service_owner'));
expect(
within(select).getByRole('option', { name: defaultProps.user.login }),
).toBeInTheDocument();
expect(
within(select).getByRole('option', { name: defaultProps.organizations[0].username }),
).toBeInTheDocument();
});

it('shows validation errors', async () => {
const errorMessage = 'Field cannot be empty';

render({ errorMessage });

expect(screen.getByText(errorMessage)).toBeInTheDocument();
});

it('selects the org when the current context is the org', async () => {
const selectedOrgOrUser = defaultProps.organizations[0].username;

render({ selectedOrgOrUser });

const select = screen.getByLabelText(textMock('general.service_owner'));
expect(select).toHaveValue(selectedOrgOrUser);
});

it('selects the user when the current context is the user', async () => {
const selectedOrgOrUser = defaultProps.user.login;

render({ selectedOrgOrUser });

const select = screen.getByLabelText(textMock('general.service_owner'));
expect(select).toHaveValue(selectedOrgOrUser);
});

it('selects the user when the current context is invalid', async () => {
const selectedOrgOrUser = 'all';

render({ selectedOrgOrUser });

const select = screen.getByLabelText(textMock('general.service_owner'));
expect(select).toHaveValue(defaultProps.user.login);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next';
import type { Organization } from 'app-shared/types/Organization';
import type { User } from 'app-shared/types/Repository';

type ServiceOwnerSelectorProps = {
export type ServiceOwnerSelectorProps = {
selectedOrgOrUser: string;
user: User;
organizations: Organization[];
Expand All @@ -26,12 +26,22 @@ export const ServiceOwnerSelector = ({
const selectableOrganizations: SelectableItem[] = mapOrganizationToSelectableItems(organizations);
const selectableOptions: SelectableItem[] = [selectableUser, ...selectableOrganizations];

const defaultValue: string =
selectableOptions.find((item) => item.value === selectedOrgOrUser)?.value ??
selectableUser.value;

return (
<div>
<Label spacing htmlFor={serviceOwnerId}>
{t('general.service_owner')}
</Label>
<NativeSelect hideLabel error={errorMessage} id={serviceOwnerId} name={name}>
<NativeSelect
hideLabel
error={errorMessage}
id={serviceOwnerId}
name={name}
defaultValue={defaultValue}
>
{selectableOptions.map(({ value, label }) => (
<option key={value} value={value}>
{label}
Expand Down
40 changes: 40 additions & 0 deletions frontend/dashboard/pages/CreateService/CreateService.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import type { Organization } from 'app-shared/types/Organization';
import { textMock } from '../../../testing/mocks/i18nMock';
import type { ServicesContextProps } from 'app-shared/contexts/ServicesContext';
import { repository, user as userMock } from 'app-shared/mocks/mocks';
import { useParams } from 'react-router-dom';
import { SelectedContextType } from 'app-shared/navigation/main-header/Header';
import { DASHBOARD_ROOT_ROUTE } from 'app-shared/constants';

const orgMock: Organization = {
avatar_url: '',
Expand All @@ -19,6 +22,7 @@ const orgMock: Organization = {
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useNavigate: jest.fn(),
useParams: jest.fn().mockReturnValue(''),
}));

const renderWithMockServices = (
Expand Down Expand Up @@ -281,4 +285,40 @@ describe('CreateService', () => {

expect(windowLocationAssignMock).toHaveBeenCalled();
});

it('should set cancel link to / when selected context is self', async () => {
const selectedContext = SelectedContextType.Self;
(useParams as jest.Mock).mockReturnValue({ selectedContext });

renderWithMockServices();
const cancelLink: HTMLElement = screen.getByRole('link', {
name: textMock('general.cancel'),
});

expect(cancelLink.getAttribute('href')).toBe(DASHBOARD_ROOT_ROUTE);
});

it('should set cancel link to /all when selected context is all', async () => {
const selectedContext = SelectedContextType.All;
(useParams as jest.Mock).mockReturnValue({ selectedContext });

renderWithMockServices();
const cancelLink: HTMLElement = screen.getByRole('link', {
name: textMock('general.cancel'),
});

expect(cancelLink.getAttribute('href')).toBe(DASHBOARD_ROOT_ROUTE + selectedContext);
});

it('should set cancel link to /org when selected context is org', async () => {
const selectedContext = 'ttd';
(useParams as jest.Mock).mockReturnValue({ selectedContext });

renderWithMockServices();
const cancelLink: HTMLElement = screen.getByRole('link', {
name: textMock('general.cancel'),
});

expect(cancelLink.getAttribute('href')).toBe(DASHBOARD_ROOT_ROUTE + selectedContext);
});
});
13 changes: 9 additions & 4 deletions frontend/dashboard/pages/CreateService/CreateService.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import type { AxiosError } from 'axios';
import { ServerCodes } from 'app-shared/enums/ServerCodes';
import { useCreateAppFormValidation } from './hooks/useCreateAppFormValidation';
import { navigateToAppDevelopment } from './utils/navigationUtils';

const DASHBOARD_ROOT_ROUTE: string = '/';
import { DASHBOARD_ROOT_ROUTE } from 'app-shared/constants';

const initialFormError: CreateAppForm = {
org: '',
Expand Down Expand Up @@ -51,7 +50,9 @@ export const CreateService = ({ user, organizations }: CreateServiceProps): JSX.
});

const defaultSelectedOrgOrUser: string =
selectedContext === SelectedContextType.Self ? user.login : selectedContext;
selectedContext === SelectedContextType.Self || selectedContext === SelectedContextType.All
? user.login
: selectedContext;
const createAppRepo = async (createAppForm: CreateAppForm) => {
addRepoMutation(
{
Expand Down Expand Up @@ -144,7 +145,11 @@ export const CreateService = ({ user, organizations }: CreateServiceProps): JSX.
<StudioButton type='submit' color='first' size='small'>
{t('dashboard.create_service_btn')}
</StudioButton>
<Link to={DASHBOARD_ROOT_ROUTE}>{t('general.cancel')}</Link>
<Link
to={`${DASHBOARD_ROOT_ROUTE}${selectedContext === SelectedContextType.Self ? '' : selectedContext}`}
>
{t('general.cancel')}
</Link>
</>
)}
</div>
Expand Down
73 changes: 73 additions & 0 deletions frontend/dashboard/pages/PageLayout/PageLayout.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React from 'react';
import { render } from '@testing-library/react';
import { MockServicesContextWrapper } from '../../dashboardTestUtils';
import type { ServicesContextProps } from 'app-shared/contexts/ServicesContext';
import { organization, user } from 'app-shared/mocks/mocks';
import { PageLayout } from './PageLayout';
import { createQueryClientMock } from 'app-shared/mocks/queryClientMock';
import { QueryKey } from 'app-shared/types/QueryKey';
import { DASHBOARD_ROOT_ROUTE } from 'app-shared/constants';
import { useParams } from 'react-router-dom';
import { SelectedContextType } from 'app-shared/navigation/main-header/Header';

const mockedNavigate = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useNavigate: () => mockedNavigate,
useParams: jest.fn(),
}));

const renderWithMockServices = (services?: Partial<ServicesContextProps>) => {
const queryClient = createQueryClientMock();
queryClient.setQueryData(
[QueryKey.Organizations],
[
{
...organization,
username: 'ttd',
},
],
);
queryClient.setQueryData([QueryKey.CurrentUser], user);

render(
<MockServicesContextWrapper customServices={services} client={queryClient}>
<PageLayout />
</MockServicesContextWrapper>,
);
};

describe('PageLayout', () => {
test('should not redirect to root if context is self', async () => {
(useParams as jest.Mock).mockReturnValue({
selectedContext: SelectedContextType.Self,
});
renderWithMockServices();
expect(mockedNavigate).not.toHaveBeenCalled();
});

test('should not redirect to root if context is all', async () => {
(useParams as jest.Mock).mockReturnValue({
selectedContext: SelectedContextType.All,
});
renderWithMockServices();
expect(mockedNavigate).not.toHaveBeenCalled();
});

test('should not redirect to root if user have access to selected context', async () => {
(useParams as jest.Mock).mockReturnValue({
selectedContext: 'ttd',
});
renderWithMockServices();
expect(mockedNavigate).not.toHaveBeenCalled();
});

test('should redirect to root if user does not have access to selected context', async () => {
(useParams as jest.Mock).mockReturnValue({
selectedContext: 'test',
});
renderWithMockServices();
expect(mockedNavigate).toHaveBeenCalledTimes(1);
expect(mockedNavigate).toHaveBeenCalledWith(DASHBOARD_ROOT_ROUTE);
});
});
3 changes: 2 additions & 1 deletion frontend/dashboard/pages/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { IHeaderContext } from 'app-shared/navigation/main-header/Header';

import { userHasAccessToSelectedContext } from '../../utils/userUtils';
import { useSelectedContext } from 'dashboard/hooks/useSelectedContext';
import { DASHBOARD_ROOT_ROUTE } from 'app-shared/constants';

export const PageLayout = () => {
const { data: user } = useUserQuery();
Expand All @@ -20,7 +21,7 @@ export const PageLayout = () => {
organizations &&
!userHasAccessToSelectedContext({ selectedContext, orgs: organizations })
) {
navigate('/');
navigate(DASHBOARD_ROOT_ROUTE);
}
}, [organizations, selectedContext, user.login, navigate]);

Expand Down
2 changes: 2 additions & 0 deletions frontend/packages/shared/src/constants.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// TODO: Extract/Centralize react-router routes (https://github.com/Altinn/altinn-studio/issues/12624)
export const APP_DEVELOPMENT_BASENAME = '/editor';
export const DASHBOARD_BASENAME = '/dashboard';
export const DASHBOARD_ROOT_ROUTE = '/';
export const RESOURCEADM_BASENAME = '/resourceadm';
export const PREVIEW_BASENAME = '/preview';
export const STUDIO_ROOT_BASENAME = '/';
Expand Down

0 comments on commit 74c6ba1

Please sign in to comment.