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

Save shape updates #141

Merged
merged 8 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Load boundary details in draw page [#139](https://github.com/azavea/iow-boundary-tool/pull/139)
- Let users select utility at login [#142](https://github.com/azavea/iow-boundary-tool/pull/142)
- Add Activity Log Serializer [#140](https://github.com/azavea/iow-boundary-tool/pull/140)
- Save shape updates on draw page [#141](https://github.com/azavea/iow-boundary-tool/pull/141)

### Changed

Expand Down
2 changes: 1 addition & 1 deletion src/app/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const privateRoutes = (
<Routes>
<Route path='/welcome' element={<Welcome />} />
<Route path='/draw' element={<NotFound />} />
<Route path='/draw/:id' element={<Draw />} />
<Route path='/draw/:boundaryId' element={<Draw />} />
<Route path='/submissions/*' element={<Submissions />} />
<Route path='*' element={<Navigate to='/welcome' replace />} />
</Routes>
Expand Down
11 changes: 10 additions & 1 deletion src/app/src/api/boundaries.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,19 @@ const boundaryApi = api.injectEndpoints({
query: newBoundary => ({
url: '/boundaries/',
method: 'POST',
body: newBoundary,
data: newBoundary,
}),
invalidatesTags: getNewItemTagInvalidator(TAGS.BOUNDARY),
}),

updateBoundaryShape: build.mutation({
query: ({ id, shape }) => ({
url: `/boundaries/${id}/shape/`,
method: 'PUT',
data: shape,
}),
}),

submitBoundary: build.mutation({
query: id => ({
url: `/boundaries/${id}/submit/`,
Expand All @@ -47,5 +55,6 @@ export const {
useGetBoundariesQuery,
useGetBoundaryDetailsQuery,
useStartNewBoundaryMutation,
useUpdateBoundaryShapeMutation,
useSubmitBoundaryMutation,
} = boundaryApi;
53 changes: 46 additions & 7 deletions src/app/src/components/DrawTools/DrawTools.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,59 @@
import { useEffect } from 'react';
import { useDispatch } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';

import { Button, Icon } from '@chakra-ui/react';
import { ArrowRightIcon } from '@heroicons/react/outline';

import EditToolbar from './EditToolbar';
import MapControlButtons from './MapControlButtons';

import { useGetBoundaryDetailsQuery } from '../../api/boundaries';
import { useBoundaryId, useEndpointToastError } from '../../hooks';

import useAddPolygonCursor from './useAddPolygonCursor';
import useEditingPolygon from './useEditingPolygon';
import useGeocoderResult from './useGeocoderResult';
import useTrackMapZoom from './useTrackMapZoom';

import { setPolygon } from '../../store/mapSlice';
import { BOUNDARY_STATUS, ROLES } from '../../constants';
import LoadingModal from '../LoadingModal';

const DRAW_MODES = {
FULLY_EDITABLE: 'fully_editable',
ANNOTATIONS_ONLY: 'annotations_only',
READ_ONLY: 'read_only',
};

export default function LoadBoundaryDetails() {
const user = useSelector(state => state.auth.user);
const id = useBoundaryId();

const { isFetching, data: details, error } = useGetBoundaryDetailsQuery(id);
useEndpointToastError(error);

if (isFetching) {
return <LoadingModal isOpen title='Loading boundary data...' />;
}

if (error || typeof details !== 'object') {
return null;
}

const mode = getDrawMode({ status: details.status, userRole: user.role });

export default function DrawTools({ details }) {
return <DrawTools mode={mode} details={details} />;
}

function DrawTools({ mode, details }) {
const dispatch = useDispatch();

// Add the polygon indicated by `details` to the state
useEffect(() => {
if (details) {
dispatch(
setPolygon({
// endpoint returns lngLat, leaflet needs latLng
points: details.submission.shape.coordinates[0].map(p => [
p[1],
p[0],
]),
points: details.submission.shape.coordinates[0],
visible: true,
label: details.utility.name,
})
Expand All @@ -49,6 +76,18 @@ export default function DrawTools({ details }) {
);
}

function getDrawMode({ status, userRole }) {
if (userRole === ROLES.VALIDATOR && status === BOUNDARY_STATUS.IN_REVIEW) {
Copy link
Contributor Author

@mstone121 mstone121 Oct 19, 2022

Choose a reason for hiding this comment

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

This used to be:

        [BOUNDARY_STATUS.SUBMITTED, BOUNDARY_STATUS.IN_REVIEW].includes(
            details.status
        ) &&
        user.role === ROLES.VALIDATOR

I'm thinking that the distinction between IN_REVIEW and SUBMITTED is the presence of a row in the Reviews database table. If a Validator clicks "Review" on a boudary, an entry should be added to the Reviews table before taking them to the /draw page.

Otherwise the Review creation logic would happen the first time a review is saved. This feels more complicated to me, however, it's something that could be handled entirely by the back-end.

Copy link
Contributor

Choose a reason for hiding this comment

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

This understanding is correct.

DRAFT = Row in Submission table, with submitted_at = NULL
SUBMITTED = That row has a value for submitted_at
IN_REVIEW = Row in Review table, with reviewed_at = NULL
NEEDS REVISIONS = That row has a value for reviewed_at
APPROVED = Row in Approval table, with a value for approved_at

I'm not sure how much we'll end up using IN_REVIEW in practice.

return DRAW_MODES.ANNOTATIONS_ONLY;
}

if (status === BOUNDARY_STATUS.DRAFT && userRole === ROLES.CONTRIBUTOR) {
return DRAW_MODES.FULLY_EDITABLE;
}

return DRAW_MODES.READ_ONLY;
}

function SaveAndBackButton() {
return (
<Button
Expand Down
46 changes: 33 additions & 13 deletions src/app/src/components/DrawTools/useEditingPolygon.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { useCallback, useEffect } from 'react';
import { useEffect } from 'react';
import { useMap } from 'react-leaflet';

import L from 'leaflet';
import 'leaflet-draw';
import { useDispatch, useSelector } from 'react-redux';
import { updatePolygon } from '../../store/mapSlice';

import { customizePrototypeIcon } from '../../utils';
import { PANES } from '../../constants';
import { useUpdateBoundaryShapeMutation } from '../../api/boundaries';
import { useBoundaryId, useTrailingDebounceCallback } from '../../hooks';
import api from '../../api/api';

const POLYGON_LAYER_OPTIONS = {
weight: 1,
Expand Down Expand Up @@ -36,28 +38,46 @@ function styleMidpointElement(element) {
element.className += ' edit-poly-marker-midpoint';
}

function getShapeFromDrawEvent(event) {
return {
coordinates: [
event.poly.getLatLngs()[0].map(point => [point.lng, point.lat]),
],
};
}

export default function useEditingPolygon() {
const dispatch = useDispatch();
const map = useMap();
const id = useBoundaryId();

const { polygon, editMode, basemapType } = useSelector(state => state.map);

const updatePolygonFromDrawEvent = useCallback(
event => {
const [updateShape] = useUpdateBoundaryShapeMutation();

const updatePolygonFromDrawEvent = useTrailingDebounceCallback({
callback: event => {
updateShape({ id, shape: getShapeFromDrawEvent(event) });
},
immediateCallback: event => {
dispatch(
updatePolygon({
points: event.poly
.getLatLngs()[0]
.map(point => [point.lat, point.lng]),
})
api.util.updateQueryData(
'getBoundaryDetails',
id,
draftDetails => {
draftDetails.submission.shape =
getShapeFromDrawEvent(event);
}
)
);
},
[dispatch]
);
interval: 3000,
});

useEffect(() => {
if (polygon && polygon.visible) {
const polygonLayer = new L.Polygon(
polygon.points.map(point => new L.latLng(point[0], point[1])),
polygon.points.map(point => new L.latLng(point[1], point[0])),
{
...POLYGON_LAYER_OPTIONS,
color: basemapType === 'satellite' ? 'white' : 'black',
Expand Down
28 changes: 28 additions & 0 deletions src/app/src/components/LoadingModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {
Flex,
Modal,
ModalBody,
ModalContent,
ModalFooter,
ModalHeader,
ModalOverlay,
Spinner,
} from '@chakra-ui/react';

export default function LoadingModal({ isOpen, title }) {
return (
<Modal isOpen={isOpen} isCentered size='xs'>
<ModalOverlay>
<ModalContent>
<ModalHeader alignItems='center'>{title ?? ''}</ModalHeader>
<ModalBody>
<Flex direction='column' alignItems='center'>
<Spinner size='xl' />
</Flex>
<ModalFooter />
</ModalBody>
</ModalContent>
</ModalOverlay>
</Modal>
);
}
56 changes: 56 additions & 0 deletions src/app/src/hooks.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { useCallback, useEffect, useRef, useState } from 'react';
import { useMap } from 'react-leaflet';
import { useDispatch, useSelector } from 'react-redux';
import { useToast } from '@chakra-ui/react';

import { convertIndexedObjectToArray } from './utils';
import {
createDefaultReferenceImage,
updateReferenceImage,
} from './store/mapSlice';
import { useParams } from 'react-router';

export function useDialogController() {
const [isOpen, setIsOpen] = useState(false);
Expand Down Expand Up @@ -110,3 +112,57 @@ export function useFilePicker(onChange) {

return openFileDialog;
}

export function useBoundaryId() {
return useParams().boundaryId;
}

/**
* Debounce a callback
* @template CallbackFunction
* @param {CallbackFunction} callback - A function to be called after no calls have been
* made to the returned callback for the duration of the interval. Its impending call
* will be replaced by newer ones.
* @param {CallbackFunction} immediateCallback - A function to be called immediately
* after calling the returned callback
* @param {number} interval
* @returns CallbackFunction
*/
export function useTrailingDebounceCallback({
callback,
immediateCallback,
mstone121 marked this conversation as resolved.
Show resolved Hide resolved
interval,
}) {
const timeout = useRef();

return useCallback(
(...args) => {
const scheduledCallback = () => {
callback(...args);
};

clearTimeout(timeout.current);
timeout.current = setTimeout(scheduledCallback, interval);

if (immediateCallback) {
immediateCallback(...args);
}
},
[callback, immediateCallback, interval]
);
}

export function useEndpointToastError(error, message = 'An error occured') {
const toast = useToast();

useEffect(() => {
if (error) {
toast({
title: message,
status: 'error',
isClosable: true,
duration: 5000,
});
}
}, [error, message, toast]);
}
53 changes: 3 additions & 50 deletions src/app/src/pages/Draw.js
Original file line number Diff line number Diff line change
@@ -1,65 +1,18 @@
import { useSelector } from 'react-redux';
import { useParams } from 'react-router-dom';
import { Box, Flex, Spinner } from '@chakra-ui/react';
import { Box, Flex } from '@chakra-ui/react';

import NotFound from './NotFound';
import DrawTools from '../components/DrawTools';
import Layers from '../components/Layers';
import Map from '../components/Map';
import Sidebar from '../components/Sidebar';

import { useGetBoundaryDetailsQuery } from '../api/boundaries';
import { BOUNDARY_STATUS, ROLES } from '../constants';

const DRAW_MODES = {
FULLY_EDITABLE: 'fully_editable',
ANNOTATIONS_ONLY: 'annotations_only',
READ_ONLY: 'read_only',
};

export default function Draw() {
const user = useSelector(state => state.auth.user);
const { id } = useParams();

const { isFetching, data: details, error } = useGetBoundaryDetailsQuery(id);

if (isFetching) {
return (
<Box w='100%' h='100vh'>
<Flex direction='column' alignItems='center'>
<Spinner mt={60} />
</Flex>
</Box>
);
}

if (error || typeof details !== 'object') {
return <NotFound />;
}

let mode = DRAW_MODES.READ_ONLY;

if (
[BOUNDARY_STATUS.SUBMITTED, BOUNDARY_STATUS.IN_REVIEW].includes(
details.status
) &&
user.role === ROLES.VALIDATOR
) {
mode = DRAW_MODES.ANNOTATIONS_ONLY;
} else if (
details.status === BOUNDARY_STATUS.DRAFT &&
user.role === ROLES.CONTRIBUTOR
) {
mode = DRAW_MODES.FULLY_EDITABLE;
}

return (
<Flex>
<Sidebar />
<Box flex={1} position='relative'>
<Map>
<Layers mode={mode} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is Layers going to need access to mode? If so, maybe boundary details should be loaded in a separate component above DrawTools and Layers, but below Map.

Copy link
Contributor

@rajadain rajadain Oct 19, 2022

Choose a reason for hiding this comment

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

maybe boundary details should be loaded in a separate component above DrawTools and Layers, but below Map.

I like that arrangement.

Alternatively, if Layers uses the same RTKQuery hook to get the boundary details, will it not get a cached value? i.e. it should be fetched only once for the different components that request it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, if Layers uses the same RTKQuery hook to get the boundary details, will it not get a cached value? i.e. it should be fetched only once for the different components that request it, right?

Yep, but then we would also need to handle the various states of the request (isFetching, error, success) in that component as well. I like to have one parent component handling all this and passing the non-null data to the children.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point.

<DrawTools mode={mode} details={details} />
<Layers />
<DrawTools />
</Map>
</Box>
</Flex>
Expand Down
13 changes: 13 additions & 0 deletions src/django/api/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from rest_framework.exceptions import APIException


class ForbiddenException(APIException):
status_code = 403
default_detail = 'You are not allowed to perform this action.'
default_code = 'forbidden'


class BadRequestException(APIException):
status_code = 400
default_detail = 'There was a problem with your request.'
default_code = 'bad_request'
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job adding clear exceptions

1 change: 1 addition & 0 deletions src/django/api/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
from .utility import UtilitySerializer
from .state import StateIDSerializer
from .boundary import BoundaryListSerializer, BoundaryDetailSerializer
from .shape import ShapeSerializer
Loading