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

PSP-5951 refactor left hand file navigation - Acquisition ONLY #3288

Merged
merged 24 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
310cedc
Update useQuery global hook
asanchezr Jun 20, 2023
08469d3
Update shared tabbed forms
asanchezr Jun 20, 2023
2cd481c
Refactor property tabs to work with state OR routing
asanchezr Jun 20, 2023
dd8892a
Refactor acquisition left hand navigation to use routing
asanchezr Jun 21, 2023
2d4ecb4
Remove setContainerState callback from acquisition forms
asanchezr Jun 21, 2023
aeabddd
Test corrections
asanchezr Jun 21, 2023
7884923
Improve tab names for properties
asanchezr Jun 21, 2023
004d3ff
Set header title based on current tab route
asanchezr Jun 21, 2023
f1d152e
Fix routing issue with requisition compensations
asanchezr Jun 21, 2023
399e362
Update snapshots
asanchezr Jun 21, 2023
6bcd184
Fix failing tests
asanchezr Jun 23, 2023
2eeb023
Merge branch 'dev' into psp-5951-sidebar-routing
asanchezr Jun 26, 2023
ea9ec62
Merge branch 'dev' into psp-5951-sidebar-routing
asanchezr Jun 26, 2023
6b37f94
Merge branch 'dev' into psp-5951-sidebar-routing
asanchezr Jun 26, 2023
7d1d4bf
Fix bug when removing property from File
asanchezr Jun 26, 2023
c1e5b91
Add tests for routing in Acquisition View
asanchezr Jun 27, 2023
812b0a6
Redirect to default tabs when editing and path doesn't match
asanchezr Jun 27, 2023
dde59f2
Merge remote-tracking branch 'upstream/dev' into psp-5951-sidebar-rou…
asanchezr Jun 27, 2023
72e9b67
Test corrections
asanchezr Jun 27, 2023
a91a303
Fix dereference error when accessing non-existing property index
asanchezr Jun 27, 2023
0c4329b
Remove unused code
asanchezr Jun 27, 2023
6379a18
Move router components to router folder
asanchezr Jun 27, 2023
0b215ec
Test corrections
asanchezr Jun 27, 2023
9582015
Merge remote-tracking branch 'upstream/dev' into psp-5951-sidebar-rou…
asanchezr Jun 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion source/frontend/src/features/account/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import { LoginStyled } from './LoginStyled';
* @returns Login component.
*/
const Login = () => {
const { redirect } = useQuery();
const query = useQuery();
const redirect = query.get('redirect');
const keyCloakWrapper = useKeycloakWrapper();
const keycloak = keyCloakWrapper.obj;
const isIE = usingIE();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import { Formik } from 'formik';
import { createMemoryHistory } from 'history';
import { noop } from 'lodash';

import { FileTypes } from '@/constants/index';
Expand All @@ -24,8 +25,8 @@ import {
import { SideBarContextProvider } from '../context/sidebarContext';
import { AcquisitionContainer, IAcquisitionContainerProps } from './AcquisitionContainer';
import { IAcquisitionViewProps } from './AcquisitionView';
import { EditFormType } from './EditFormNames';

const history = createMemoryHistory();
const mockAxios = new MockAdapter(axios);
const generateFn = jest.fn();

Expand Down Expand Up @@ -84,6 +85,7 @@ describe('AcquisitionContainer component', () => {
},
useMockAuthentication: true,
claims: renderOptions?.claims ?? [],
history,
...renderOptions,
},
);
Expand Down Expand Up @@ -136,11 +138,9 @@ describe('AcquisitionContainer component', () => {
await waitForElementToBeRemoved(spinner);

mockAxios.onGet(new RegExp('acquisitionfiles/1/properties')).timeout();
await act(async () => {
viewProps.setContainerState({ activeEditForm: EditFormType.PROPERTY_SELECTOR });
viewProps.canRemove(1);
expect(spinner).not.toBeVisible();
});
await act(async () => viewProps.onShowPropertySelector());
await act(async () => viewProps.canRemove(1));
expect(spinner).not.toBeVisible();
});

it('canRemove returns true if file property has no associated entities', async () => {
Expand All @@ -158,10 +158,8 @@ describe('AcquisitionContainer component', () => {
},
},
]);
await act(async () => {
viewProps.setContainerState({ activeEditForm: EditFormType.PROPERTY_SELECTOR });
});
const canRemoveResponse = await viewProps.canRemove(1);
await act(async () => viewProps.onShowPropertySelector());
const canRemoveResponse = await act(() => viewProps.canRemove(1));
expect(canRemoveResponse).toBe(true);
});

Expand All @@ -181,10 +179,9 @@ describe('AcquisitionContainer component', () => {
activityInstanceProperties: [{}],
},
]);
await act(async () => {
viewProps.setContainerState({ activeEditForm: EditFormType.PROPERTY_SELECTOR });
});
expect(await viewProps.canRemove(1)).toBe(false);
await act(async () => viewProps.onShowPropertySelector());
const canRemoveResponse = await act(() => viewProps.canRemove(1));
expect(canRemoveResponse).toBe(false);
});

it('should change menu index when not editing', async () => {
Expand All @@ -194,7 +191,7 @@ describe('AcquisitionContainer component', () => {
await waitForElementToBeRemoved(spinner);

await act(async () => viewProps.onMenuChange(1));
await waitFor(async () => expect(viewProps.containerState.selectedMenuIndex).toBe(1));
expect(history.location.pathname).toBe('/property/1');
});

it('displays a warning if form is dirty and menu index changes', async () => {
Expand All @@ -204,13 +201,15 @@ describe('AcquisitionContainer component', () => {
const spinner = getByTestId('filter-backdrop-loading');
await waitForElementToBeRemoved(spinner);

await act(async () => viewProps.setContainerState({ isEditing: true }));
await act(async () => viewProps.setIsEditing(true));
await act(async () => (viewProps.formikRef.current as any).setFieldValue('value', 1));
await screen.findByText('1');
await act(async () => viewProps.onMenuChange(1));

await waitFor(async () => expect(viewProps.containerState.showConfirmModal).toBe(true));
await waitFor(async () => expect(viewProps.containerState.isEditing).toBe(true));
expect(viewProps.containerState.showConfirmModal).toBe(true);
expect(history.location.pathname).toBe('/property/1');
const params = new URLSearchParams(history.location.search);
expect(params.has('edit')).toBe(false);
});

it('cancels edit if form is not dirty and menu index changes', async () => {
Expand All @@ -220,10 +219,11 @@ describe('AcquisitionContainer component', () => {
const spinner = getByTestId('filter-backdrop-loading');
await waitForElementToBeRemoved(spinner);

await act(async () => viewProps.setContainerState({ isEditing: true }));
await act(async () => viewProps.setIsEditing(true));
await act(async () => viewProps.onMenuChange(1));

await waitFor(async () => expect(viewProps.containerState.isEditing).toBe(false));
const params = new URLSearchParams(history.location.search);
expect(params.has('edit')).toBe(false);
});

it('on success function refetches acq file', async () => {
Expand All @@ -245,10 +245,11 @@ describe('AcquisitionContainer component', () => {
const spinner = getByTestId('filter-backdrop-loading');
await waitForElementToBeRemoved(spinner);

await act(async () => viewProps.setContainerState({ isEditing: true }));
await act(async () => viewProps.setIsEditing(true));
await act(async () => viewProps.onSuccess());

expect(viewProps.containerState.isEditing).toBe(false);
const params = new URLSearchParams(history.location.search);
expect(params.has('edit')).toBe(false);
});

it('on save function submits the form', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { FormikProps } from 'formik';
import React, { useCallback, useContext, useEffect, useReducer, useRef } from 'react';
import { useHistory, useRouteMatch } from 'react-router-dom';

import LoadingBackdrop from '@/components/common/LoadingBackdrop';
import { useMapSearch } from '@/components/maps/hooks/useMapSearch';
import { FileTypes } from '@/constants/index';
import { InventoryTabNames } from '@/features/mapSideBar/property/InventoryTabs';
import { useAcquisitionProvider } from '@/hooks/repositories/useAcquisitionProvider';
import { useQuery } from '@/hooks/use-query';
import useApiUserOverride from '@/hooks/useApiUserOverride';
import { Api_AcquisitionFile } from '@/models/api/AcquisitionFile';
import { Api_File } from '@/models/api/File';
import { UserOverrideCode } from '@/models/api/UserOverrideCode';
import { stripTrailingSlash } from '@/utils';

import { SideBarContext } from '../context/sidebarContext';
import { FileTabType } from '../shared/detail/FileTabs';
Expand Down Expand Up @@ -63,6 +66,20 @@ export const AcquisitionContainer: React.FunctionComponent<IAcquisitionContainer

const formikRef = useRef<FormikProps<any>>(null);

const history = useHistory();
const match = useRouteMatch();
const query = useQuery();
const isEditing = query.get('edit') === 'true';

const setIsEditing = (value: boolean) => {
if (value) {
query.set('edit', value.toString());
} else {
query.delete('edit');
}
history.push({ search: query.toString() });
};

/**
See here that we are using `newState: Partial<AcquisitionContainerState>` in our reducer
so we can provide only the properties that are updated on our state
Expand Down Expand Up @@ -115,24 +132,33 @@ export const AcquisitionContainer: React.FunctionComponent<IAcquisitionContainer

const close = useCallback(() => onClose && onClose(), [onClose]);

const navigateToMenuRoute = (selectedIndex: number) => {
const route = selectedIndex === 0 ? '' : `/property/${selectedIndex}`;
history.push(`${stripTrailingSlash(match.url)}${route}`);
};

const onMenuChange = (selectedIndex: number) => {
if (containerState.isEditing) {
if (isEditing) {
if (formikRef?.current?.dirty) {
if (
window.confirm('You have made changes on this form. Do you wish to leave without saving?')
) {
handleCancelClick();
setContainerState({ selectedMenuIndex: selectedIndex });
navigateToMenuRoute(selectedIndex);
}
} else {
handleCancelClick();
setContainerState({ selectedMenuIndex: selectedIndex });
navigateToMenuRoute(selectedIndex);
}
} else {
setContainerState({ selectedMenuIndex: selectedIndex });
navigateToMenuRoute(selectedIndex);
}
};

const onShowPropertySelector = () => {
history.push(`${stripTrailingSlash(match.url)}/property/selector`);
};

const handleSaveClick = () => {
if (formikRef !== undefined) {
formikRef.current?.setSubmitting(true);
Expand All @@ -156,17 +182,14 @@ export const AcquisitionContainer: React.FunctionComponent<IAcquisitionContainer
if (formikRef !== undefined) {
formikRef.current?.resetForm();
}
setContainerState({
showConfirmModal: false,
isEditing: false,
activeEditForm: undefined,
});
setContainerState({ showConfirmModal: false });
setIsEditing(false);
};

const onSuccess = () => {
fetchAcquisitionFile();
searchMany();
setContainerState({ activeEditForm: undefined, isEditing: false });
setIsEditing(false);
};

const canRemove = async (propertyId: number) => {
Expand All @@ -183,7 +206,10 @@ export const AcquisitionContainer: React.FunctionComponent<IAcquisitionContainer
return withUserOverride((userOverrideCodes: UserOverrideCode[]) => {
return updateAcquisitionProperties
.execute({ ...file, productId: null, projectId: null }, userOverrideCodes)
.then(() => onSuccess());
.then(response => {
onSuccess();
return response;
});
});
};

Expand All @@ -198,12 +224,15 @@ export const AcquisitionContainer: React.FunctionComponent<IAcquisitionContainer

return (
<View
isEditing={isEditing}
setIsEditing={setIsEditing}
containerState={containerState}
setContainerState={setContainerState}
onClose={close}
onCancel={handleCancelClick}
onSave={handleSaveClick}
onMenuChange={onMenuChange}
onShowPropertySelector={onShowPropertySelector}
onCancelConfirm={handleCancelConfirm}
onUpdateProperties={onUpdateProperties}
onSuccess={onSuccess}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { FormikProps } from 'formik';
import React from 'react';
import { Redirect, Route, Switch, useRouteMatch } from 'react-router-dom';

import { InventoryTabNames } from '@/features/mapSideBar/property/InventoryTabs';
import { FileTabType } from '@/features/mapSideBar/shared/detail/FileTabs';
import { Api_AcquisitionFile } from '@/models/api/AcquisitionFile';
import { stripTrailingSlash } from '@/utils';

import { EditFormType } from './EditFormNames';
import { AcquisitionFileTabs } from './tabs/AcquisitionFileTabs';
import { UpdateAgreementsContainer } from './tabs/agreement/update/UpdateAgreementsContainer';
import { UpdateAgreementsForm } from './tabs/agreement/update/UpdateAgreementsForm';
import { UpdateAcquisitionChecklistContainer } from './tabs/checklist/update/UpdateAcquisitionChecklistContainer';
import { UpdateAcquisitionChecklistForm } from './tabs/checklist/update/UpdateAcquisitionChecklistForm';
import { UpdateAcquisitionContainer } from './tabs/fileDetails/update/UpdateAcquisitionContainer';
import { UpdateAcquisitionForm } from './tabs/fileDetails/update/UpdateAcquisitionForm';
import { UpdateStakeHolderContainer } from './tabs/stakeholders/update/UpdateStakeHolderContainer';
import { UpdateStakeHolderForm } from './tabs/stakeholders/update/UpdateStakeHolderForm';

export interface IAcquisitionRouterProps {
formikRef: React.Ref<FormikProps<any>>;
acquisitionFile?: Api_AcquisitionFile;
isEditing: boolean;
setIsEditing: (value: boolean) => void;
activeEditForm?: EditFormType;
defaultFileTab: FileTabType;
defaultPropertyTab: InventoryTabNames;
onSuccess: () => void;
}

export const AcquisitionRouter: React.FC<IAcquisitionRouterProps> = props => {
const { path, url } = useRouteMatch();

if (props.acquisitionFile === undefined || props.acquisitionFile === null) {
return null;
}

// render edit forms
if (props.isEditing) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what should we do if we are editing and our path doesn't match one of these? currently we display an empty edit screen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old code in ViewSelector throws an error - throw Error('Active edit form not defined');
I can create a default route that does the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. We redirect to the default File tab (file details)

return (
<Switch>
<Route exact path={`${stripTrailingSlash(path)}/${FileTabType.FILE_DETAILS}`}>
<UpdateAcquisitionContainer
ref={props.formikRef}
acquisitionFile={props.acquisitionFile}
onSuccess={props.onSuccess}
View={UpdateAcquisitionForm}
/>
</Route>
<Route exact path={`${stripTrailingSlash(path)}/${FileTabType.CHECKLIST}`}>
<UpdateAcquisitionChecklistContainer
formikRef={props.formikRef}
acquisitionFile={props.acquisitionFile}
onSuccess={props.onSuccess}
View={UpdateAcquisitionChecklistForm}
/>
</Route>
<Route exact path={`${stripTrailingSlash(path)}/${FileTabType.AGREEMENTS}`}>
<UpdateAgreementsContainer
acquisitionFileId={props.acquisitionFile.id || -1}
View={UpdateAgreementsForm}
formikRef={props.formikRef}
onSuccess={() => props.setIsEditing(false)}
/>
</Route>
<Route exact path={`${stripTrailingSlash(path)}/${FileTabType.STAKEHOLDERS}`}>
<UpdateStakeHolderContainer
View={UpdateStakeHolderForm}
formikRef={props.formikRef}
acquisitionFile={props.acquisitionFile}
onSuccess={() => props.setIsEditing(false)}
/>
</Route>
</Switch>
);
} else {
// render read-only views
return (
<Switch>
{/* Ignore property-related routes (which are handled in separate FilePropertyRouter) */}
<Route path={`${stripTrailingSlash(path)}/property`}>
<></>
</Route>
<Route path={`${stripTrailingSlash(path)}/:tab`}>
<AcquisitionFileTabs
acquisitionFile={props.acquisitionFile}
defaultTab={props.defaultFileTab}
setIsEditing={props.setIsEditing}
/>
</Route>
<Redirect from={`${path}`} to={`${stripTrailingSlash(url)}/${FileTabType.FILE_DETAILS}`} />
</Switch>
);
}
};

export default AcquisitionRouter;
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const onCancelConfirm = jest.fn();
const onUpdateProperties = jest.fn();
const canRemove = jest.fn();
const setContainerState = jest.fn();
const setIsEditing = jest.fn();
const onEditFileProperties = jest.fn();

// Need to mock this library for unit tests
jest.mock('react-visibility-sensor', () => {
Expand All @@ -50,6 +52,9 @@ const DEFAULT_PROPS: IAcquisitionViewProps = {
onCancelConfirm,
onUpdateProperties,
canRemove,
isEditing: false,
setIsEditing,
onShowPropertySelector: onEditFileProperties,
setContainerState,
containerState: {
acquisitionFile: mockAcquisitionFileResponse(),
Expand Down
Loading