From 5a403cb236436aadc7ac95721194292df2f2db1d Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Mon, 17 Oct 2022 16:35:36 -0500 Subject: [PATCH 1/8] Add useBoundaryId hook There are two reasons to do this: The first is that it's less ambiguous than using `const { id } = useParams()`. This is especially important in deeply nested components where there are other elements with their own IDs. Second, if this logic were to change at a later point (perhaps it is moved into a Context), then it can be updated in one place. --- src/app/src/App.js | 2 +- src/app/src/hooks.js | 5 +++++ src/app/src/pages/Draw.js | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/app/src/App.js b/src/app/src/App.js index 548fbd5e..a092aa16 100644 --- a/src/app/src/App.js +++ b/src/app/src/App.js @@ -28,7 +28,7 @@ const privateRoutes = ( } /> } /> - } /> + } /> } /> } /> diff --git a/src/app/src/hooks.js b/src/app/src/hooks.js index 277eddcd..56d64b49 100644 --- a/src/app/src/hooks.js +++ b/src/app/src/hooks.js @@ -7,6 +7,7 @@ import { createDefaultReferenceImage, updateReferenceImage, } from './store/mapSlice'; +import { useParams } from 'react-router'; export function useDialogController() { const [isOpen, setIsOpen] = useState(false); @@ -110,3 +111,7 @@ export function useFilePicker(onChange) { return openFileDialog; } + +export function useBoundaryId() { + return useParams().boundaryId; +} diff --git a/src/app/src/pages/Draw.js b/src/app/src/pages/Draw.js index a4d50cdf..4cedb46d 100644 --- a/src/app/src/pages/Draw.js +++ b/src/app/src/pages/Draw.js @@ -1,5 +1,4 @@ import { useSelector } from 'react-redux'; -import { useParams } from 'react-router-dom'; import { Box, Flex, Spinner } from '@chakra-ui/react'; import NotFound from './NotFound'; @@ -10,6 +9,7 @@ import Sidebar from '../components/Sidebar'; import { useGetBoundaryDetailsQuery } from '../api/boundaries'; import { BOUNDARY_STATUS, ROLES } from '../constants'; +import { useBoundaryId } from '../hooks'; const DRAW_MODES = { FULLY_EDITABLE: 'fully_editable', @@ -19,7 +19,7 @@ const DRAW_MODES = { export default function Draw() { const user = useSelector(state => state.auth.user); - const { id } = useParams(); + const id = useBoundaryId(); const { isFetching, data: details, error } = useGetBoundaryDetailsQuery(id); From e0bf015f010f0849bf8b705bffc1763e44b9c154 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Tue, 18 Oct 2022 10:40:00 -0500 Subject: [PATCH 2/8] Update draw page to save polygon This does not include the back-end changes which will follow in a later commit. --- src/app/src/api/boundaries.js | 11 +++- src/app/src/components/DrawTools/DrawTools.js | 6 +- .../components/DrawTools/useEditingPolygon.js | 61 ++++++++++++++----- src/app/src/hooks.js | 59 ++++++++++++++++++ 4 files changed, 117 insertions(+), 20 deletions(-) diff --git a/src/app/src/api/boundaries.js b/src/app/src/api/boundaries.js index c362afdc..c417d156 100644 --- a/src/app/src/api/boundaries.js +++ b/src/app/src/api/boundaries.js @@ -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: 'POST', + data: shape, + }), + }), + submitBoundary: build.mutation({ query: id => ({ url: `/boundaries/${id}/submit/`, @@ -47,5 +55,6 @@ export const { useGetBoundariesQuery, useGetBoundaryDetailsQuery, useStartNewBoundaryMutation, + useUpdateBoundaryShapeMutation, useSubmitBoundaryMutation, } = boundaryApi; diff --git a/src/app/src/components/DrawTools/DrawTools.js b/src/app/src/components/DrawTools/DrawTools.js index eff2f84e..64131a99 100644 --- a/src/app/src/components/DrawTools/DrawTools.js +++ b/src/app/src/components/DrawTools/DrawTools.js @@ -22,11 +22,7 @@ export default function DrawTools({ details }) { 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, }) diff --git a/src/app/src/components/DrawTools/useEditingPolygon.js b/src/app/src/components/DrawTools/useEditingPolygon.js index 12ac5458..5a7e1173 100644 --- a/src/app/src/components/DrawTools/useEditingPolygon.js +++ b/src/app/src/components/DrawTools/useEditingPolygon.js @@ -1,12 +1,18 @@ -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, + useReverseQueue, + useTrailingDebounceCallback, +} from '../../hooks'; +import api from '../../api/api'; const POLYGON_LAYER_OPTIONS = { weight: 1, @@ -36,28 +42,55 @@ 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 => { - dispatch( - updatePolygon({ - points: event.poly - .getLatLngs()[0] - .map(point => [point.lat, point.lng]), - }) + const [updateShape] = useUpdateBoundaryShapeMutation(); + const reverseQueue = useReverseQueue({ + endpointName: 'getBoundaryDetails', + queryArgs: id, + }); + + const updatePolygonFromDrawEvent = useTrailingDebounceCallback({ + callback: event => { + updateShape({ id, shape: getShapeFromDrawEvent(event) }) + .unwrap() + .then(reverseQueue.clear) + .catch(reverseQueue.apply); + }, + immediateCallback: event => { + const { inversePatches } = dispatch( + api.util.updateQueryData( + 'getBoundaryDetails', + id, + draftDetails => { + draftDetails.submission.shape = + getShapeFromDrawEvent(event); + } + ) ); + + reverseQueue.push(inversePatches); }, - [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', diff --git a/src/app/src/hooks.js b/src/app/src/hooks.js index 56d64b49..d4b92517 100644 --- a/src/app/src/hooks.js +++ b/src/app/src/hooks.js @@ -8,6 +8,7 @@ import { updateReferenceImage, } from './store/mapSlice'; import { useParams } from 'react-router'; +import api from './api/api'; export function useDialogController() { const [isOpen, setIsOpen] = useState(false); @@ -115,3 +116,61 @@ export function useFilePicker(onChange) { 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, + 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 useReverseQueue({ queryArgs, endpointName }) { + const dispatch = useDispatch(); + + const queue = useRef([]); + + const clear = useCallback(() => { + queue.current = []; + }, []); + + const apply = useCallback(() => { + dispatch( + api.util.patchQueryData(endpointName, queryArgs, queue.current) + ); + clear(); + }, [dispatch, queryArgs, endpointName, clear]); + + const push = useCallback(patches => { + queue.current = [...patches, ...queue.current]; + }, []); + + return { queue, clear, apply, push }; +} From b5ad76204179891578fccbc7056f53a5fdf0cbc4 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Tue, 18 Oct 2022 12:40:51 -0500 Subject: [PATCH 3/8] Add update boundary shape route --- src/app/src/api/boundaries.js | 2 +- src/django/api/exceptions.py | 13 +++++++++ src/django/api/serializers/__init__.py | 1 + src/django/api/serializers/shape.py | 32 +++++++++++++++++++++ src/django/api/urls.py | 3 +- src/django/api/views/boundary.py | 40 ++++++++++++++++++++++++-- 6 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 src/django/api/exceptions.py create mode 100644 src/django/api/serializers/shape.py diff --git a/src/app/src/api/boundaries.js b/src/app/src/api/boundaries.js index c417d156..0e54cefa 100644 --- a/src/app/src/api/boundaries.js +++ b/src/app/src/api/boundaries.js @@ -36,7 +36,7 @@ const boundaryApi = api.injectEndpoints({ updateBoundaryShape: build.mutation({ query: ({ id, shape }) => ({ url: `/boundaries/${id}/shape/`, - method: 'POST', + method: 'PUT', data: shape, }), }), diff --git a/src/django/api/exceptions.py b/src/django/api/exceptions.py new file mode 100644 index 00000000..52d7f29a --- /dev/null +++ b/src/django/api/exceptions.py @@ -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' diff --git a/src/django/api/serializers/__init__.py b/src/django/api/serializers/__init__.py index a632fa7f..5d9122a4 100644 --- a/src/django/api/serializers/__init__.py +++ b/src/django/api/serializers/__init__.py @@ -3,3 +3,4 @@ from .utility import UtilitySerializer from .state import StateIDSerializer from .boundary import BoundaryListSerializer, BoundaryDetailSerializer +from .shape import ShapeSerializer diff --git a/src/django/api/serializers/shape.py b/src/django/api/serializers/shape.py new file mode 100644 index 00000000..1f0d33e8 --- /dev/null +++ b/src/django/api/serializers/shape.py @@ -0,0 +1,32 @@ +from rest_framework.serializers import Serializer +from rest_framework.fields import ListField, FloatField +from django.contrib.gis.geos import Polygon + + +class ShapeSerializer(Serializer): + coordinates = ListField( + child=ListField( + child=ListField(child=FloatField(), min_length=2, max_length=2), + min_length=3, + ), + min_length=1, + max_length=1, + ) + + def to_internal_value(self, data): + validated_data = super().to_internal_value(data) + coordinates = validated_data['coordinates'] + + if not ShapeSerializer.coordinates_are_closed(coordinates): + coordinates = ShapeSerializer.get_closed_coordinates(coordinates) + + return Polygon(*coordinates) + + @staticmethod + def coordinates_are_closed(coordinates): + return coordinates[0][-1] == coordinates[0][0] + + @staticmethod + def get_closed_coordinates(coordinates): + coordinates[0].append(coordinates[0][0]) + return coordinates diff --git a/src/django/api/urls.py b/src/django/api/urls.py index 1fa3898c..1e1bf688 100644 --- a/src/django/api/urls.py +++ b/src/django/api/urls.py @@ -2,7 +2,7 @@ from rest_framework.urlpatterns import format_suffix_patterns from .views import Login, Logout -from .views.boundary import BoundaryDetailView, BoundaryListView +from .views.boundary import BoundaryDetailView, BoundaryListView, BoundaryShapeView from .views.reference_image import ReferenceImageList, ReferenceImageDetail urlpatterns = [ @@ -11,6 +11,7 @@ path("auth/", include("dj_rest_auth.urls")), path("boundaries/", BoundaryListView.as_view()), path("boundaries//", BoundaryDetailView.as_view()), + path("boundaries//shape/", BoundaryShapeView.as_view()), path( "boundaries//reference-images/", ReferenceImageList.as_view(), diff --git a/src/django/api/views/boundary.py b/src/django/api/views/boundary.py index 3992b5d6..74391a90 100644 --- a/src/django/api/views/boundary.py +++ b/src/django/api/views/boundary.py @@ -4,10 +4,17 @@ from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.permissions import IsAuthenticated +from rest_framework.status import HTTP_204_NO_CONTENT -from ..models import Boundary, Roles, Submission -from ..serializers import BoundaryListSerializer, BoundaryDetailSerializer +from ..models.boundary import BOUNDARY_STATUS +from ..serializers import ( + BoundaryListSerializer, + BoundaryDetailSerializer, + ShapeSerializer, +) +from ..models import Boundary, Roles, Submission +from ..exceptions import ForbiddenException, BadRequestException def get_boundary_queryset_for_user(user): @@ -70,3 +77,32 @@ def get(self, request, id, format=None): boundary = get_object_or_404(boundary_set, pk=id) return Response(BoundaryDetailSerializer(boundary).data) + + +class BoundaryShapeView(APIView): + permission_classes = [IsAuthenticated] + + def put(self, request, id, format=None): + boundary_set = get_boundary_queryset_for_user(request.user) + boundary_set = boundary_set.prefetch_related('submissions') + boundary = get_object_or_404(boundary_set, pk=id) + + if request.user.role not in [Roles.CONTRIBUTOR, Roles.ADMINISTRATOR]: + raise ForbiddenException( + 'Only contributors and administrators can edit boundaries.' + ) + + if boundary.status != BOUNDARY_STATUS.DRAFT: + raise BadRequestException( + 'Cannot update shape of boundary with status: {}'.format( + boundary.status + ), + ) + + serializer = ShapeSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + boundary.latest_submission.shape = serializer.validated_data + boundary.latest_submission.save() + + return Response(status=HTTP_204_NO_CONTENT) From 8d3772f2146a8650861a7325900d269d6f3a6114 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Tue, 18 Oct 2022 12:54:39 -0500 Subject: [PATCH 4/8] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ea29212..2325b266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 From c824157823160c28a4b777e5651a747d57ae018a Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Wed, 19 Oct 2022 14:13:07 -0500 Subject: [PATCH 5/8] Add useEndpointToastError hook This could be useful in many components. --- src/app/src/hooks.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/app/src/hooks.js b/src/app/src/hooks.js index d4b92517..d1cd29c2 100644 --- a/src/app/src/hooks.js +++ b/src/app/src/hooks.js @@ -1,6 +1,7 @@ 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 { @@ -174,3 +175,18 @@ export function useReverseQueue({ queryArgs, endpointName }) { return { queue, clear, apply, push }; } + +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]); +} From 2b9520aadca58d1abd2fa5f3ffadcdba44f1dac8 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Wed, 19 Oct 2022 14:13:25 -0500 Subject: [PATCH 6/8] Add LoadingModal This could be useful in many components. --- src/app/src/components/LoadingModal.js | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/app/src/components/LoadingModal.js diff --git a/src/app/src/components/LoadingModal.js b/src/app/src/components/LoadingModal.js new file mode 100644 index 00000000..d9f2b68f --- /dev/null +++ b/src/app/src/components/LoadingModal.js @@ -0,0 +1,28 @@ +import { + Flex, + Modal, + ModalBody, + ModalContent, + ModalFooter, + ModalHeader, + ModalOverlay, + Spinner, +} from '@chakra-ui/react'; + +export default function LoadingModal({ isOpen, title }) { + return ( + + + + {title ?? ''} + + + + + + + + + + ); +} From 551e61d7e2424e6ede5dff726fb8acedd9e4ac07 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Wed, 19 Oct 2022 14:14:02 -0500 Subject: [PATCH 7/8] Move boundary details query inside When the query was outside of the component, it would cause re-render of the map each time the query data was updated. --- src/app/src/components/DrawTools/DrawTools.js | 47 +++++++++++++++- src/app/src/pages/Draw.js | 53 ++----------------- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/app/src/components/DrawTools/DrawTools.js b/src/app/src/components/DrawTools/DrawTools.js index 64131a99..ead6ee54 100644 --- a/src/app/src/components/DrawTools/DrawTools.js +++ b/src/app/src/components/DrawTools/DrawTools.js @@ -1,5 +1,5 @@ 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'; @@ -7,14 +7,45 @@ 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 ; + } + + if (error || typeof details !== 'object') { + return null; + } + + const mode = getDrawMode({ status: details.status, userRole: user.role }); -export default function DrawTools({ details }) { + return ; +} + +function DrawTools({ mode, details }) { const dispatch = useDispatch(); // Add the polygon indicated by `details` to the state @@ -45,6 +76,18 @@ export default function DrawTools({ details }) { ); } +function getDrawMode({ status, userRole }) { + if (userRole === ROLES.VALIDATOR && status === BOUNDARY_STATUS.IN_REVIEW) { + 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 (