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.

Sign up for GitHub

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

Jump to bottom

PSP-8103 Warn user that property is part of any disposition file #3968

Merged
merged 3 commits into from
Apr 23, 2024
Merged

PSP-8103 Warn user that property is part of any disposition file #3968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
890d40f
Fix property warning for acquisition files
asanchezr Apr 23, 2024
f247064
PSP-8103 show warning when adding a property included in another file
asanchezr Apr 23, 2024
47e7f72
Test corrections
asanchezr Apr 23, 2024
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
2 changes: 1 addition & 1 deletion ...ce/frontend/src/features/mapSideBar/acquisition/add/AcquisitionPropertiesSubForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('AcquisitionProperties component', () => {
const setup = (
props: {
initialForm: AcquisitionForm;
confirmBeforeAdd?: (propertyId: number) => Promise<boolean>;
confirmBeforeAdd?: (propertyForm: PropertyForm) => Promise<boolean>;
},
renderOptions: RenderOptions = {},
) => {
Expand Down
4 changes: 2 additions & 2 deletions source/frontend/src/features/mapSideBar/acquisition/add/AcquisitionPropertiesSubForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { AcquisitionForm } from './models';

export interface IAcquisitionPropertiesProps {
formikProps: FormikProps<AcquisitionForm>;
confirmBeforeAdd: (propertyId: number) => Promise<boolean>;
confirmBeforeAdd: (propertyForm: PropertyForm) => Promise<boolean>;
}

export const AcquisitionPropertiesSubForm: React.FunctionComponent<IAcquisitionPropertiesProps> = ({
Expand Down Expand Up @@ -51,7 +51,7 @@ export const AcquisitionPropertiesSubForm: React.FunctionComponent<IAcquisitionP
: undefined;
}

if (formProperty.apiId && (await confirmBeforeAdd(formProperty.apiId))) {
if (await confirmBeforeAdd(formProperty)) {
// Require user confirmation before adding property to file
setModalContent({
variant: 'warning',
Expand Down
41 changes: 33 additions & 8 deletions source/frontend/src/features/mapSideBar/acquisition/add/AddAcquisitionContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import RealEstateAgent from '@/assets/images/real-estate-agent.svg?react';
import LoadingBackdrop from '@/components/common/LoadingBackdrop';
import { useMapStateMachine } from '@/components/common/mapFSM/MapStateMachineContext';
import MapSideBarLayout from '@/features/mapSideBar/layout/MapSideBarLayout';
import { usePimsPropertyRepository } from '@/hooks/repositories/usePimsPropertyRepository';
import { usePropertyAssociations } from '@/hooks/repositories/usePropertyAssociations';
import { getCancelModalProps, useModalContext } from '@/hooks/useModalContext';
import { ApiGen_Concepts_AcquisitionFile } from '@/models/api/generated/ApiGen_Concepts_AcquisitionFile';
import { exists } from '@/utils';
import { exists, isValidId, isValidString } from '@/utils';
import { featuresetToMapProperty } from '@/utils/mapPropertyUtils';

import { PropertyForm } from '../../shared/models';
Expand All @@ -35,17 +36,41 @@ export const AddAcquisitionContainer: React.FC<IAddAcquisitionContainerProps> =
const selectedFeatureDataset = mapMachine.selectedFeatureDataset;

const { execute: getPropertyAssociations } = usePropertyAssociations();
const {
getPropertyByPidWrapper: { execute: getPropertyByPid },
getPropertyByPinWrapper: { execute: getPropertyByPin },
} = usePimsPropertyRepository();
const [needsUserConfirmation, setNeedsUserConfirmation] = useState<boolean>(true);

// Warn user that property is part of an existing acquisition file
const confirmBeforeAdd = useCallback(
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems like this function has more to do with properties then acquisition files, would recommend moving this to a shared space to reduce code duplication.

Copy link
Collaborator

Choose a reason for hiding this comment

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

would need to handle the associations for each type, perhaps via a param? not strictly necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree with the code duplication - have to to think about that. Given that we are so close to sprint end I would lean towards merging as-is (if not broken) and then refactoring next sprint

async (propertyId: number) => {
const response = await getPropertyAssociations(propertyId);
const acquisitionAssociations = response?.acquisitionAssociations ?? [];
const otherAcqFiles = acquisitionAssociations.filter(a => exists(a.id));
return otherAcqFiles.length > 0;
async (propertyForm: PropertyForm) => {
let apiId;
try {
if (isValidId(propertyForm.apiId)) {
apiId = propertyForm.apiId;
} else if (isValidString(propertyForm.pid)) {
const result = await getPropertyByPid(propertyForm.pid);
apiId = result?.id;
} else if (isValidString(propertyForm.pin)) {
const result = await getPropertyByPin(Number(propertyForm.pin));
apiId = result?.id;
}
} catch (e) {
apiId = 0;
}

if (isValidId(apiId)) {
const response = await getPropertyAssociations(apiId);
const acquisitionAssociations = response?.acquisitionAssociations ?? [];
const otherAcqFiles = acquisitionAssociations.filter(a => exists(a.id));
return otherAcqFiles.length > 0;
} else {
// the property is not in PIMS db -> no need to confirm
return false;
}
},
[getPropertyAssociations],
[getPropertyAssociations, getPropertyByPid, getPropertyByPin],
);

const initialForm = useMemo(() => {
Expand Down Expand Up @@ -119,7 +144,7 @@ export const AddAcquisitionContainer: React.FC<IAddAcquisitionContainerProps> =
if (exists(initialValues) && exists(formikRef.current) && needsUserConfirmation) {
if (initialValues.properties.length > 0) {
const formProperty = initialValues.properties[0];
if (formProperty.apiId && (await confirmBeforeAdd(formProperty.apiId))) {
if (await confirmBeforeAdd(formProperty)) {
setModalContent({
variant: 'warning',
title: 'User Override Required',
Expand Down
5 changes: 3 additions & 2 deletions source/frontend/src/features/mapSideBar/acquisition/add/AddAcquisitionForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { UserOverrideCode } from '@/models/api/UserOverrideCode';
import { isValidId, isValidString } from '@/utils';
import { formatApiPersonNames } from '@/utils/personUtils';

import { PropertyForm } from '../../shared/models';
import { AcquisitionFormModal } from '../common/modals/AcquisitionFormModal';
import UpdateAcquisitionOwnersSubForm from '../common/update/acquisitionOwners/UpdateAcquisitionOwnersSubForm';
import { UpdateAcquisitionTeamSubForm } from '../common/update/acquisitionTeam/UpdateAcquisitionTeamSubForm';
Expand All @@ -45,7 +46,7 @@ export interface IAddAcquisitionFormProps {
setSubmitting: (isSubmitting: boolean) => void,
userOverrides: UserOverrideCode[],
) => void | Promise<any>;
confirmBeforeAdd: (propertyId: number) => Promise<boolean>;
confirmBeforeAdd: (propertyForm: PropertyForm) => Promise<boolean>;
}

export const AddAcquisitionForm = React.forwardRef<
Expand Down Expand Up @@ -105,7 +106,7 @@ const AddAcquisitionDetailSubForm: React.FC<{
) => void | Promise<any>;
showDiffMinistryRegionModal: boolean;
setShowDiffMinistryRegionModal: React.Dispatch<React.SetStateAction<boolean>>;
confirmBeforeAdd: (propertyId: number) => Promise<boolean>;
confirmBeforeAdd: (propertyForm: PropertyForm) => Promise<boolean>;
}> = ({
formikProps,
onSubmit,
Expand Down
46 changes: 41 additions & 5 deletions source/frontend/src/features/mapSideBar/disposition/DispositionContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ import { matchPath, useHistory, useLocation, useRouteMatch } from 'react-router-
import LoadingBackdrop from '@/components/common/LoadingBackdrop';
import { useMapStateMachine } from '@/components/common/mapFSM/MapStateMachineContext';
import { useDispositionProvider } from '@/hooks/repositories/useDispositionProvider';
import { usePimsPropertyRepository } from '@/hooks/repositories/usePimsPropertyRepository';
import { usePropertyAssociations } from '@/hooks/repositories/usePropertyAssociations';
import { useQuery } from '@/hooks/use-query';
import useApiUserOverride from '@/hooks/useApiUserOverride';
import { getCancelModalProps, useModalContext } from '@/hooks/useModalContext';
import { IApiError } from '@/interfaces/IApiError';
import { ApiGen_Concepts_DispositionFile } from '@/models/api/generated/ApiGen_Concepts_DispositionFile';
import { ApiGen_Concepts_File } from '@/models/api/generated/ApiGen_Concepts_File';
import { UserOverrideCode } from '@/models/api/UserOverrideCode';
import { exists, stripTrailingSlash } from '@/utils';
import { exists, isValidId, isValidString, stripTrailingSlash } from '@/utils';

import { SideBarContext } from '../context/sidebarContext';
import { PropertyForm } from '../shared/models';
import { IDispositionViewProps } from './DispositionView';

export interface IDispositionContainerProps {
Expand Down Expand Up @@ -54,11 +57,17 @@ export const DispositionContainer: React.FunctionComponent<IDispositionContainer
updateDispositionProperties,
getLastUpdatedBy: { execute: getLastUpdatedBy, loading: loadingGetLastUpdatedBy },
} = useDispositionProvider();
const { execute: getPropertyAssociations } = usePropertyAssociations();
const {
getPropertyByPidWrapper: { execute: getPropertyByPid },
getPropertyByPinWrapper: { execute: getPropertyByPin },
} = usePimsPropertyRepository();

const { setModalContent, setDisplayModal } = useModalContext();

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

const formikRef = useRef<FormikProps<any>>(null);
const location = useLocation();
const history = useHistory();
const match = useRouteMatch();
Expand Down Expand Up @@ -268,9 +277,36 @@ export const DispositionContainer: React.FunctionComponent<IDispositionContainer
return true;
};

const confirmBeforeAdd = async () => {
return false;
};
// Warn user that property is part of an existing disposition file
const confirmBeforeAdd = useCallback(
async (propertyForm: PropertyForm): Promise<boolean> => {
let apiId;
try {
if (isValidId(propertyForm.apiId)) {
apiId = propertyForm.apiId;
} else if (isValidString(propertyForm.pid)) {
const result = await getPropertyByPid(propertyForm.pid);
apiId = result?.id;
} else if (isValidString(propertyForm.pin)) {
const result = await getPropertyByPin(Number(propertyForm.pin));
apiId = result?.id;
}
} catch (e) {
apiId = 0;
}

if (isValidId(apiId)) {
const response = await getPropertyAssociations(apiId);
const fileAssociations = response?.dispositionAssociations ?? [];
const otherFiles = fileAssociations.filter(a => exists(a.id) && a.id !== dispositionFileId);
return otherFiles.length > 0;
} else {
// the property is not in PIMS db -> no need to confirm
return false;
}
},
[dispositionFileId, getPropertyAssociations, getPropertyByPid, getPropertyByPin],
);

// UI components
const loading =
Expand Down
98 changes: 92 additions & 6 deletions source/frontend/src/features/mapSideBar/disposition/add/AddDispositionContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import { useHistory } from 'react-router-dom';

import { useMapStateMachine } from '@/components/common/mapFSM/MapStateMachineContext';
import { useDispositionProvider } from '@/hooks/repositories/useDispositionProvider';
import { usePimsPropertyRepository } from '@/hooks/repositories/usePimsPropertyRepository';
import { usePropertyAssociations } from '@/hooks/repositories/usePropertyAssociations';
import useApiUserOverride from '@/hooks/useApiUserOverride';
import { useInitialMapSelectorProperties } from '@/hooks/useInitialMapSelectorProperties';
import { useModalContext } from '@/hooks/useModalContext';
import { IApiError } from '@/interfaces/IApiError';
import { ApiGen_Concepts_DispositionFile } from '@/models/api/generated/ApiGen_Concepts_DispositionFile';
import { UserOverrideCode } from '@/models/api/UserOverrideCode';
import { featuresetToMapProperty } from '@/utils';
import { exists, featuresetToMapProperty, isValidId, isValidString } from '@/utils';

import { PropertyForm } from '../../shared/models';
import { DispositionFormModel } from '../models/DispositionFormModel';
Expand All @@ -30,11 +32,48 @@ const AddDispositionContainer: React.FC<IAddDispositionContainerProps> = ({ onCl
const selectedFeatureDataset = mapMachine.selectedFeatureDataset;

const { setModalContent, setDisplayModal } = useModalContext();
const { execute: getPropertyAssociations } = usePropertyAssociations();
const {
getPropertyByPidWrapper: { execute: getPropertyByPid },
getPropertyByPinWrapper: { execute: getPropertyByPin },
} = usePimsPropertyRepository();
const [needsUserConfirmation, setNeedsUserConfirmation] = useState<boolean>(true);

const {
addDispositionFileApi: { execute: addDispositionFileApi, loading },
} = useDispositionProvider();

// Warn user that property is part of an existing disposition file
const confirmBeforeAdd = useCallback(
async (propertyForm: PropertyForm): Promise<boolean> => {
let apiId;
try {
if (isValidId(propertyForm.apiId)) {
apiId = propertyForm.apiId;
} else if (isValidString(propertyForm.pid)) {
const result = await getPropertyByPid(propertyForm.pid);
apiId = result?.id;
} else if (isValidString(propertyForm.pin)) {
const result = await getPropertyByPin(Number(propertyForm.pin));
apiId = result?.id;
}
} catch (e) {
apiId = 0;
}

if (isValidId(apiId)) {
const response = await getPropertyAssociations(apiId);
const fileAssociations = response?.dispositionAssociations ?? [];
const otherFiles = fileAssociations.filter(a => exists(a.id));
return otherFiles.length > 0;
} else {
// the property is not in PIMS db -> no need to confirm
return false;
}
},
[getPropertyAssociations, getPropertyByPid, getPropertyByPin],
);

const initialForm = useMemo(() => {
const dispositionForm = new DispositionFormModel();
// support creating a new disposition file from the map popup
Expand All @@ -55,12 +94,58 @@ const AddDispositionContainer: React.FC<IAddDispositionContainerProps> = ({ onCl
initialForm.fileProperties[0].address = initialProperty.address;
}

// Require user confirmation before adding a property to file
// This is the flow for Map Marker -> right-click -> create Disposition File
useEffect(() => {
if (!!initialForm && !!formikRef.current) {
formikRef.current.resetForm();
formikRef.current?.setFieldValue('fileProperties', initialForm.fileProperties);
}
}, [initialForm]);
const runAsync = async () => {
if (exists(initialForm) && exists(formikRef.current) && needsUserConfirmation) {
if (initialForm.fileProperties.length > 0) {
const formProperty = initialForm.fileProperties[0];
if (await confirmBeforeAdd(formProperty)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't love the name of this function. Confirm what before Add?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Open to suggestions on better naming. I believe you created a similar "canDelete" in the same area of code but "canAdd" did not make sense to me.
Basically if this function returns true we need to "Confirm via a popup before we let the user add the property". IF the function returns false we add the property to the property picker without any confirmation - open to naming suggestions as I said

setModalContent({
variant: 'warning',
title: 'User Override Required',
message: (
<>
<p>This property has already been added to one or more disposition files.</p>
<p>Do you want to acknowledge and proceed?</p>
</>
),
okButtonText: 'Yes',
cancelButtonText: 'No',
handleOk: () => {
// allow the property to be added to the file being created
formikRef.current.resetForm();
formikRef.current.setFieldValue('fileProperties', initialForm.fileProperties);
setDisplayModal(false);
// show the user confirmation modal only once when creating a file
setNeedsUserConfirmation(false);
},
handleCancel: () => {
// clear out the properties array as the user did not agree to the popup
initialForm.fileProperties.splice(0, initialForm.fileProperties.length);
formikRef.current.resetForm();
formikRef.current.setFieldValue('fileProperties', initialForm.fileProperties);
setDisplayModal(false);
// show the user confirmation modal only once when creating a file
setNeedsUserConfirmation(false);
},
});
setDisplayModal(true);
}
}
}
};

runAsync();
}, [
confirmBeforeAdd,
initialForm,
needsUserConfirmation,
setDisplayModal,
setModalContent,
bcaLoading,
]);

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

Expand Down Expand Up @@ -109,6 +194,7 @@ const AddDispositionContainer: React.FC<IAddDispositionContainerProps> = ({ onCl
dispositionInitialValues={initialForm}
loading={loading || bcaLoading}
displayFormInvalid={!isFormValid}
confirmBeforeAdd={confirmBeforeAdd}
onSave={handleSave}
onCancel={handleCancel}
onSubmit={(
Expand Down
2 changes: 2 additions & 0 deletions source/frontend/src/features/mapSideBar/disposition/add/AddDispositionContainerView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ jest.mock('@react-keycloak/web');
const onCancel = jest.fn();
const onSave = jest.fn();
const onSubmit = jest.fn();
const confirmBeforeAdd = jest.fn();

const initialValues = new DispositionFormModel();

Expand All @@ -36,6 +37,7 @@ describe('Add Disposition Container View', () => {
onCancel={onCancel}
onSave={onSave}
onSubmit={onSubmit}
confirmBeforeAdd={confirmBeforeAdd}
/>,
{
...renderOptions,
Expand Down
10 changes: 6 additions & 4 deletions source/frontend/src/features/mapSideBar/disposition/add/AddDispositionContainerView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import styled from 'styled-components';
import LoadingBackdrop from '@/components/common/LoadingBackdrop';

import MapSideBarLayout from '../../layout/MapSideBarLayout';
import { PropertyForm } from '../../shared/models';
import SidebarFooter from '../../shared/SidebarFooter';
import DispositionForm from '../form/DispositionForm';
import { DispositionFormModel } from '../models/DispositionFormModel';
Expand All @@ -20,18 +21,18 @@ export interface IAddDispositionContainerViewProps {
) => void | Promise<any>;
onCancel: () => void;
onSave: () => void;
confirmBeforeAdd: (propertyForm: PropertyForm) => Promise<boolean>;
}

const AddDispositionContainerView: React.FunctionComponent<
React.PropsWithChildren<IAddDispositionContainerViewProps>
> = ({
const AddDispositionContainerView: React.FunctionComponent<IAddDispositionContainerViewProps> = ({
formikRef,
dispositionInitialValues,
loading,
displayFormInvalid,
onSubmit,
onSave,
onCancel,
confirmBeforeAdd,
}) => {
return (
<MapSideBarLayout
Expand Down Expand Up @@ -59,9 +60,10 @@ const AddDispositionContainerView: React.FunctionComponent<
<StyledFormWrapper>
<LoadingBackdrop show={loading} parentScreen={true} />
<DispositionForm
ref={formikRef}
formikRef={formikRef}
initialValues={dispositionInitialValues}
onSubmit={onSubmit}
confirmBeforeAdd={confirmBeforeAdd}
></DispositionForm>
</StyledFormWrapper>
</MapSideBarLayout>
Expand Down
Loading
Loading