Tests for update/delete review item also provided fix

portal/facade/reviews_facade.py

@@ -26,8 +26,8 @@ class ReviewsFacade(GeneralCRUDFacade):
             .create_review_items(author=author, items=items)
 
     def update_item(self, item: ReviewItem, **data):
-        return self._service(review=item.review).update_item(item, **data)
+        return self._service(item.review).update_item(item, **data)
 
     def delete_item(self, item: ReviewItem):
-        return self._service(review=item.review).delete(item)
+        return self._service(item.review).delete(item)
 

portal/rest/schemas.py

@@ -483,6 +483,14 @@ class Schemas:
     def review(self, **kwargs):
         return self.__get_schema(ReviewSchema, **kwargs)
 
+    @fn_name
+    def review_items(self, **kwargs):
+        return self.__get_schema(ReviewItemSchema, many=True, **kwargs)
+
+    @fn_name
+    def review_item(self, **kwargs):
+        return self.__get_schema(ReviewItemSchema, **kwargs)
+
     @fn_name
     def courses(self, **kwargs):
         return self.__get_schema(CourseSchema, many=True, **kwargs)

portal/rest/submissions.py

@@ -344,12 +344,12 @@ class SubmissionReviewItem(CustomResource):
         course = submission.project.course
         perm_service = self.permissions(course=course)
         perm_service.require.write_review_for_submission(submission)
-        data = rest_helpers.parse_request_data(action='update', resource='review')
+        data = rest_helpers.parse_request_data(action='update', resource='review_item')
         log.info(f"[REST] Update submission review item for {submission.log_name} by "
                  f"{self.client.log_name}: {data}")
         item = self.find.review_item(rid)
         review = self.facades.reviews.update_item(item=item, **data)
-        return SCHEMAS.dump('review', review), 201
+        return SCHEMAS.dump('review', review), 204
 
     @jwt_required
     @access_log
@@ -363,4 +363,4 @@ class SubmissionReviewItem(CustomResource):
                  f"{self.client.log_name}")
         item = self.find.review_item(rid)
         review = self.facades.reviews.delete_item(item=item)
-        return SCHEMAS.dump('review', review), 201
+        return SCHEMAS.dump('review', review), 204

tests/conftest.py

@@ -4,8 +4,7 @@ import pytest
 
 from management.data import DataManagement
 from portal import create_app, db
-from portal.database import ProjectConfig
-from portal.logger import Logging
+from portal.database import ProjectConfig, Review, Submission
 from tests.utils.ent_mocker import EntitiesMocker
 
 
@@ -85,6 +84,16 @@ def created_student(rest_service):
     return rest_service.find.user('student1')
 
 
+@pytest.fixture()
+def created_submission(rest_service, created_project) -> Submission:
+    return created_project.submissions[0]
+
+
+@pytest.fixture()
+def created_review(rest_service) -> Review:
+    return Review.query.all()[0]
+
+
 @pytest.fixture()
 def created_admin(rest_service):
     return rest_service.find.user('admin')

tests/rest/rest_tools.py

@@ -258,6 +258,7 @@ def assert_review_items(expected: list, actual: dict):
 
 
 def assert_review(expected: Review, actual: dict):
+    assert expected is not None
     assert expected.id == actual['id']
     assert expected.submission_id == actual['submission']['id']
     assert_review_items(expected.review_items, actual['review_items'])

tests/rest/test_submission.py

@@ -2,7 +2,6 @@ import pytest
 
 from portal import storage
 from portal.database.models import Review, ReviewItem, Submission, SubmissionState
-from portal.tools import paths
 from tests.rest.rest_tools import assert_response
 from . import rest_tools
 
@@ -217,9 +216,7 @@ def test_read_review(client):
     rest_tools.assert_review(expected=s.review, actual=review)
 
 
-def test_create_review(client):
-    s = Submission.query.filter_by(state=SubmissionState.IN_PROGRESS).first()
-    assert s
+def test_create_review(client, created_submission):
     len_reviews = len(Review.query.all())
     len_review_items = len(ReviewItem.query.all())
 
@@ -231,21 +228,45 @@ def test_create_review(client):
                 "content": "This is wrong!"
             },
             {
-                "file": "src.main.c",
+                "file": "src/main.c",
                 "line": 15,
                 "content": "Nice job!"
             }
         ]
     }
-
-    response = rest_tools.make_request(client, f'/submissions/{s.id}/review', json=request_dict,
+    url = f'/submissions/{created_submission.id}/review'
+    response = rest_tools.make_request(client, url, json=request_dict,
                                        method='post')
 
     assert_response(response, 201)
 
     review = rest_tools.extract_data(response)
-    s_updated = Submission.query.filter_by(project_id=s.project_id,
-                                           state=SubmissionState.IN_PROGRESS).first()
+    s_updated = Submission.query.filter_by(id=created_submission.id).first()
     assert len(Review.query.all()) == len_reviews + 1
     assert len(ReviewItem.query.all()) == len_review_items + 2
     rest_tools.assert_review(s_updated.review, review)
+
+
+def test_delete_review(created_review, client):
+    submission = created_review.submission
+    item = created_review.review_items[0]
+    url = f'/submissions/{submission.id}/review/{item.id}'
+    response = rest_tools.make_request(client, url, method='delete')
+    assert_response(response=response, code=204)
+    assert not ReviewItem.query.filter_by(id=item.id).all()
+
+
+def test_update_review(created_review, client):
+    submission = created_review.submission
+    item: ReviewItem = created_review.review_items[0]
+    url = f'/submissions/{submission.id}/review/{item.id}'
+    body = {
+        "content": "Updated content"
+    }
+    response = rest_tools.make_request(client, url, json=body, method='put')
+    assert_response(response=response, code=204)
+    updated: ReviewItem = ReviewItem.query.filter_by(id=item.id).one()
+    assert updated
+    assert updated.content == 'Updated content'
+    assert updated.line == item.line
+    assert updated.file == item.file