Merge branch 'change_draft'

metabrainz · Apr 6, 2022 · fd72c34 · fd72c34
2 parents ca0b2a3 + b78f056
commit fd72c34
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 4 deletions.
diff --git a/critiquebrainz/frontend/views/review.py b/critiquebrainz/frontend/views/review.py
@@ -364,9 +364,15 @@ def edit(id):
         # Can't change license if review is published.
         del form.license_choice
 
-    # Check if contents of the review are updated
-    if form.text.data == review['text'] and form.rating.data == review['rating']:
-        form.errors['edit'] = ["You must edit either text or rating to update the review."]
+    # The purpose of the check is to not create unnecessary revisions. So updating a draft review
+    # without changes or editing a published review without changes is not allowed. But publishing
+    # a draft review without changes is allowed.
+    if form.state.data == "draft" and review["is_draft"] \
+            and form.text.data == review["text"] and form.rating.data == review["rating"]:
+        form.errors["edit"] = ["You must edit either text or rating to update the draft."]
+    elif not review["is_draft"] and form.text.data == review["text"] \
+        and form.rating.data == review["rating"]:
+        form.errors["edit"] = ["You must edit either text or rating to update the review."]
     elif form.validate_on_submit():
         if review["is_draft"]:
             license_choice = form.license_choice.data

diff --git a/critiquebrainz/frontend/views/test/test_review.py b/critiquebrainz/frontend/views/test/test_review.py
@@ -145,6 +145,41 @@ def test_create_duplicate(self):
                                    follow_redirects=True)
         self.assertIn("You have already published a review for this entity", str(response.data))
 
+    def test_edit_draft_without_changes(self):
+        self.temporary_login(self.user)
+
+        review = self.create_dummy_review(is_draft=True)
+        # test saving draft review without changes results in warning
+        data = {
+            review["entity_type"]: review["entity_id"],
+            "state": "draft",
+            "license_choice": self.license["id"],
+            "language": 'en',
+            "agreement": 'True',
+            "text": review["text"],
+            "rating": review["rating"]
+        }
+
+        response = self.client.post(
+            "/review/%s/edit" % review["id"],
+            data=data,
+            query_string=data,
+            follow_redirects=True
+        )
+        self.assert200(response)
+        self.assertIn('You must edit either text or rating to update the review.', str(response.data))
+
+        # test publishing draft review without changes does not cause error
+        data["state"] = "publish"
+        response = self.client.post(
+            "/review/%s/edit" % review["id"],
+            data=data,
+            query_string=data,
+            follow_redirects=True
+        )
+        self.assert200(response)
+        self.assertNotIn('You must edit either text or rating to update the review.', str(response.data))
+
     def test_edit(self):
         updated_text = "The text has now been updated"
         data = dict(
@@ -164,7 +199,7 @@ def test_edit(self):
         self.assert200(response)
         self.assertIn(updated_text, str(response.data))
 
-        # edit once again with the same test and check for error
+        # test editing published review without changing errors
         response = self.client.post('/review/%s/edit' % review['id'], data=data,
                                     query_string=data, follow_redirects=True)
         self.assert200(response)