Merge pull request #186 from azavea/ms/add-support-for-approving-boun…

…daries

Add support for approving/unapprove boundaries

CHANGELOG.md

@@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add unapproving to data model [#178](https://github.com/azavea/iow-boundary-tool/pull/178)
 - Add ability to submit reviews [#181](https://github.com/azavea/iow-boundary-tool/pull/181)
 - Add ability to resubmit boundaries [#185](https://github.com/azavea/iow-boundary-tool/pull/185)
+- Add support for approving/unapproving boundaries [#186](https://github.com/azavea/iow-boundary-tool/pull/186)
 
 ### Changed
 

doc/arch/adr-002-rest-api.md

@@ -33,8 +33,8 @@ Also present are `/auth/password/reset/` and `/auth/password/reset/confirm/` end
 | `/boundaries/{id}/review/annotations/{id}/` | PUT    | `{ annotation }` | Validator      | 200 OK,<br />404 Not Found                                 | Updates an annotation in the latest review. Older reviews are read-only, so no need to specify review id.          |
 | `/boundaries/{id}/review/annotations/{id}/` | DELETE | -                | Validator      | 204 NO Content,<br />404 Not Found                         | Deletes an annotation in the latest review. Older reviews are read-only, so no need to specify review id.          |
 | `/boundaries/{id}/draft/`                   | POST   | -                | Contributor    | 204 No Content,<br />404 Not Found                         | Creates a new draft submission for a boundary after a review                                                       |
-| `/boundaries/{id}/approve/`                 | POST   | -                | Validator      | 200 OK,<br />404 Not Found                                 | Approves a boundary                                                                                                |
-| `/boundaries/{id}/unapprove/`               | POST   | -                | Validator      | 200 OK,<br />404 Not Found                                 | Unapproves a boundary                                                                                              |
+| `/boundaries/{id}/approve/`                 | POST   | -                | Validator      | 204 No Content,<br />404 Not Found                         | Approves a boundary                                                                                                |
+| `/boundaries/{id}/unapprove/`               | POST   | -                | Validator      | 204 No Content,<br />404 Not Found                         | Unapproves a boundary                                                                                              |
 
 ### User
 

src/app/src/api/boundaries.js

@@ -167,6 +167,22 @@ const boundaryApi = api.injectEndpoints({
             }),
             invalidatesTags: getUpdateItemTagInvalidator(TAGS.BOUNDARY),
         }),
+
+        approveBoundary: build.mutation({
+            query: boundaryId => ({
+                url: `/boundaries/${boundaryId}/approve/`,
+                method: 'POST',
+            }),
+            invalidatesTags: getUpdateItemTagInvalidator(TAGS.BOUNDARY),
+        }),
+
+        unapproveBoundary: build.mutation({
+            query: boundaryId => ({
+                url: `/boundaries/${boundaryId}/unapprove/`,
+                method: 'POST',
+            }),
+            invalidatesTags: getUpdateItemTagInvalidator(TAGS.BOUNDARY),
+        }),
     }),
 });
 
@@ -179,4 +195,6 @@ export const {
     useDeleteBoundaryShapeMutation,
     useSubmitBoundaryMutation,
     useCreateDraftMutation,
+    useApproveBoundaryMutation,
+    useUnapproveBoundaryMutation,
 } = boundaryApi;

src/app/src/components/Submissions/Detail/Detail.js

@@ -10,7 +10,11 @@ import {
 } from '@chakra-ui/react';
 import { ArrowLeftIcon } from '@heroicons/react/outline';
 import { useNavigate, useParams } from 'react-router-dom';
-import { useGetBoundaryDetailsQuery } from '../../../api/boundaries';
+import {
+    useApproveBoundaryMutation,
+    useGetBoundaryDetailsQuery,
+    useUnapproveBoundaryMutation,
+} from '../../../api/boundaries';
 import CenteredSpinner from '../../CenteredSpinner';
 import { CheckCircleIcon } from '@heroicons/react/outline';
 
@@ -51,6 +55,16 @@ export default function SubmissionDetail() {
         { isLoading: isCreatingDraft, error: createDraftMutationError },
     ] = useCreateDraftMutation();
 
+    const [
+        approveBoundary,
+        { isLoading: isApprovingBoundary, error: approveBoundaryError },
+    ] = useApproveBoundaryMutation();
+
+    const [
+        unapproveBoundary,
+        { isLoading: isUnpprovingBoundary, error: unapproveBoundaryError },
+    ] = useUnapproveBoundaryMutation();
+
     useEndpointToastError(
         startReviewError,
         'There was an error starting a review.'
@@ -61,15 +75,34 @@ export default function SubmissionDetail() {
         'There was an error creating a new submission.'
     );
 
-    if (isFetching || isStartingReview || isCreatingDraft) {
+    useEndpointToastError(
+        approveBoundaryError,
+        'There was an error approving the submission.'
+    );
+
+    useEndpointToastError(
+        unapproveBoundaryError,
+        'There was an error unapproving the submission'
+    );
+
+    if (
+        isFetching ||
+        isStartingReview ||
+        isCreatingDraft ||
+        isApprovingBoundary ||
+        isUnpprovingBoundary
+    ) {
         return <CenteredSpinner />;
     }
 
     if (!boundary) {
         return null;
     }
 
-    const { canApprove } = getBoundaryPermissions({ boundary, user });
+    const { canApprove, canUnapprove } = getBoundaryPermissions({
+        boundary,
+        user,
+    });
 
     return (
         <VStack
@@ -104,10 +137,21 @@ export default function SubmissionDetail() {
                             mb={4}
                             alignSelf='flex-end'
                             rightIcon={heroToChakraIcon(CheckCircleIcon)()}
+                            onClick={() => approveBoundary(boundary.id)}
                         >
                             Mark approved
                         </Button>
                     )}
+                    {canUnapprove && (
+                        <Button
+                            mb={4}
+                            alignSelf='flex-end'
+                            rightIcon={heroToChakraIcon(CheckCircleIcon)()}
+                            onClick={() => unapproveBoundary(boundary.id)}
+                        >
+                            Mark unapproved
+                        </Button>
+                    )}
 
                     <Box
                         h='sm'

src/app/src/utils.js

@@ -152,7 +152,7 @@ export function getBoundaryPermissions({ boundary, user }) {
         Object.entries(getBoundaryPermissionForRole(role)).map(
             ([permissionType, validStatuses]) => [
                 permissionType,
-                validStatuses?.includes(status) ?? false,
+                validStatuses.includes(status),
             ]
         )
     );
@@ -176,7 +176,10 @@ function getBoundaryPermissionForRole(role) {
                     BOUNDARY_STATUS.SUBMITTED,
                     BOUNDARY_STATUS.IN_REVIEW,
                 ],
-                canApprove: [BOUNDARY_STATUS.SUBMITTED],
+                canApprove: [
+                    BOUNDARY_STATUS.SUBMITTED,
+                    BOUNDARY_STATUS.IN_REVIEW,
+                ],
                 canUnapprove: [BOUNDARY_STATUS.APPROVED],
             };
 

src/django/api/models/submission.py

@@ -121,6 +121,10 @@ def clean(self):
     def revoked(self):
         return self.unapproved_at is not None
 
+    def unapprove(self, unapproved_by):
+        self.unapproved_at = datetime.now(tz=timezone.utc)
+        self.unapproved_by = unapproved_by
+
 
 class Annotation(models.Model):
     review = models.ForeignKey(

src/django/api/permissions.py

@@ -25,3 +25,13 @@ def has_permission(self, request, view):
             request.user.role == Roles.CONTRIBUTOR
             or request.user.role == Roles.ADMINISTRATOR
         )
+
+
+class UserCanUnapproveBoundaries(BasePermission):
+    message = 'You are not able to unapprove boundaries.'
+
+    def has_permission(self, request, view):
+        return request_is_read_only(request) or (
+            request.user.role == Roles.VALIDATOR
+            or request.user.role == Roles.ADMINISTRATOR
+        )

src/django/api/urls.py

@@ -8,6 +8,8 @@
     BoundaryShapeView,
     BoundarySubmitView,
     BoundaryDraftView,
+    BoundaryApproveView,
+    BoundaryUnapproveView,
 )
 from .views.reference_image import ReferenceImageList, ReferenceImageDetail
 from .views.review import ReviewCreateView, ReviewFinishView
@@ -56,6 +58,16 @@
         BoundaryDraftView.as_view(),
         name='start_draft',
     ),
+    path(
+        "boundaries/<int:boundary_id>/approve/",
+        BoundaryApproveView.as_view(),
+        name='approve_boundary',
+    ),
+    path(
+        "boundaries/<int:boundary_id>/unapprove/",
+        BoundaryUnapproveView.as_view(),
+        name='unapprove_boundary',
+    ),
 ]
 
 urlpatterns = format_suffix_patterns(urlpatterns)

src/django/api/views/boundary.py

@@ -1,7 +1,6 @@
 import json
 
-from datetime import datetime
-from pytz import timezone
+from datetime import datetime, timezone
 
 from django.conf import settings
 from django.db.models import Prefetch, functions
@@ -24,7 +23,7 @@
 from ..models.boundary import BOUNDARY_STATUS, Boundary
 from ..models.submission import Approval
 from ..exceptions import BadRequestException
-from ..permissions import UserCanWriteBoundaries
+from ..permissions import UserCanWriteBoundaries, UserCanUnapproveBoundaries
 
 
 def get_boundary_queryset_for_user(user):
@@ -251,3 +250,58 @@ def post(self, request, boundary_id, format=None):
         )
 
         return Response(status=HTTP_204_NO_CONTENT)
+
+
+class BoundaryApproveView(BoundaryView):
+    permission_classes = [IsAuthenticated]
+
+    def post(self, request, boundary_id, format=None):
+        boundary = self.get_boundary(request, boundary_id)
+        self.check_boundary_is_approvable(boundary, request.user.role)
+
+        boundary.latest_submission.approvals.create(approved_by=request.user)
+
+        return Response(status=HTTP_204_NO_CONTENT)
+
+    def check_boundary_is_approvable(self, boundary, user_role):
+        if user_role == Roles.ADMINISTRATOR:
+            if boundary.status not in [
+                BOUNDARY_STATUS.SUBMITTED,
+                BOUNDARY_STATUS.IN_REVIEW,
+                BOUNDARY_STATUS.NEEDS_REVISIONS,
+            ]:
+                raise BadRequestException(
+                    'This boundary is not in an approvable state.',
+                )
+
+        elif user_role == Roles.VALIDATOR:
+            if boundary.status not in [
+                BOUNDARY_STATUS.SUBMITTED,
+                BOUNDARY_STATUS.IN_REVIEW,
+            ]:
+                raise BadRequestException(
+                    'This boundary must be submitted or in review to be approved.',
+                )
+
+        else:
+            raise BadRequestException('You are not able to approve boundaries.')
+
+
+class BoundaryUnapproveView(BoundaryView):
+    permission_classes = [IsAuthenticated, UserCanUnapproveBoundaries]
+
+    def post(self, request, boundary_id, format=None):
+        boundary = self.get_boundary(request, boundary_id)
+        self.check_boundary_is_unapprovable(boundary, request.user.role)
+
+        approval = boundary.latest_submission.latest_approval
+        approval.unapprove(request.user)
+        approval.save()
+
+        return Response(status=HTTP_204_NO_CONTENT)
+
+    def check_boundary_is_unapprovable(self, boundary, user_role):
+        if boundary.status != BOUNDARY_STATUS.APPROVED:
+            raise BadRequestException(
+                'Only approved boundaries can be unapproved',
+            )