21931 update validation (bcgov#2853)

PaulGarewal · Jul 23, 2024 · 4594c29 · 4594c29
1 parent 3fe951b
commit 4594c29
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 18 deletions.
diff --git a/legal-api/src/legal_api/resources/v2/admin/reviews.py b/legal-api/src/legal_api/resources/v2/admin/reviews.py
@@ -56,23 +56,27 @@ def save_review(review_id: int):
         return jsonify({'message': 'Review not found.'}), HTTPStatus.NOT_FOUND
 
     json_input = request.get_json()
-    status = json_input.get('status')
-    comment = json_input.get('comment')
-    if not status or not comment:
-        return jsonify({'message': 'Status and Comment are required.'}), HTTPStatus.BAD_REQUEST
+    if status := json_input.get('status'):
+        if not ((status := ReviewStatus[status]) and
+                (status in [ReviewStatus.CHANGE_REQUESTED,
+                            ReviewStatus.APPROVED,
+                            ReviewStatus.REJECTED])):
+            return jsonify({'message': 'Invalid Status.'}), HTTPStatus.BAD_REQUEST
+
+    else:
+        return jsonify({'message': 'Status is required.'}), HTTPStatus.BAD_REQUEST
 
-    if not ((status := ReviewStatus[status]) and
-            (status in [ReviewStatus.CHANGE_REQUESTED,
-                        ReviewStatus.APPROVED,
-                        ReviewStatus.REJECTED])):
-        return jsonify({'message': 'Invalid Status.'}), HTTPStatus.BAD_REQUEST
+    comment = json_input.get('comment')
+    if (status in [ReviewStatus.CHANGE_REQUESTED, ReviewStatus.REJECTED]
+            and not comment):
+        return jsonify({'message': 'Comment is required.'}), HTTPStatus.BAD_REQUEST
 
     filing = Filing.find_by_id(review.filing_id)
 
     review_result = ReviewResult()
     review_result.reviewer_id = user.id
     review_result.status = status
-    review_result.comments = Sanitizer().sanitize(comment)
+    review_result.comments = Sanitizer().sanitize(comment) if comment else None
     review.review_results.append(review_result)
     review.status = status
     review.save()

diff --git a/legal-api/tests/unit/resources/v2/test_reviews.py b/legal-api/tests/unit/resources/v2/test_reviews.py
@@ -128,18 +128,18 @@ def test_get_nonexistent_review(app, session, client, jwt):
     assert rv.json['message'] == 'Review not found.'
 
 
-@pytest.mark.parametrize('status', [
-    ReviewStatus.CHANGE_REQUESTED,
-    ReviewStatus.APPROVED,
-    ReviewStatus.REJECTED,
+@pytest.mark.parametrize('status, comment', [
+    (ReviewStatus.CHANGE_REQUESTED, 'Upload all documents'),
+    (ReviewStatus.APPROVED, None),
+    (ReviewStatus.REJECTED, 'Upload all documents'),
 ])
-def test_save_review(app, session, client, jwt, mocker, status):
+def test_save_review(app, session, client, jwt, mocker, status, comment):
     """Assert that a review can be saved."""
     review = create_review('T1z3a567')
 
     data = {
         'status': status.name,
-        'comment': 'Upload all documents'
+        'comment': comment
     }
 
     subjects_in_queue = {}
@@ -158,7 +158,7 @@ def publish_json(payload, subject):
     result = review.review_results.all()[0]
     assert result
     assert result.status == status
-    assert result.comments == data['comment']
+    assert result.comments == comment
 
     status_mapping = {
         ReviewStatus.CHANGE_REQUESTED: Filing.Status.CHANGE_REQUESTED.value,
@@ -176,7 +176,9 @@ def publish_json(payload, subject):
 
 
 @pytest.mark.parametrize('data, message, response_code', [
-    ({'status': ReviewStatus.APPROVED.name}, 'Status and Comment are required.', HTTPStatus.BAD_REQUEST),
+    ({'comment': 'all docs'}, 'Status is required.', HTTPStatus.BAD_REQUEST),
+    ({'status': ReviewStatus.CHANGE_REQUESTED.name}, 'Comment is required.', HTTPStatus.BAD_REQUEST),
+    ({'status': ReviewStatus.REJECTED.name}, 'Comment is required.', HTTPStatus.BAD_REQUEST),
     ({'status': ReviewStatus.RESUBMITTED.name, 'comment': 'all docs'}, 'Invalid Status.', HTTPStatus.BAD_REQUEST),
 ])
 def test_save_review_validation(app, session, client, jwt, mocker, data, message, response_code):