Reset the editor/reviewer comments when not specified

CHANGELOG.rst

@@ -10,6 +10,10 @@ This document describes changes between each past release.
 
 - Removed ability to resign using ``to-sign`` twice. Use to ``to-resign`` instead.
 
+**Bug fixes**
+
+- Reset the editor/reviewer comments when not specified.
+
 
 8.0.1 (2021-02-23)
 ------------------

kinto_signer/listeners.py

@@ -166,6 +166,10 @@ def sign_collection_data(event, resources, **kwargs):
             review_event_kw["changes_count"] = changes_count
 
         elif new_status == STATUS.TO_REVIEW:
+            # Make sure we reset the review comments if none is specified.
+            new_collection.setdefault("last_reviewer_comment", "")
+            comment = new_collection.setdefault("last_editor_comment", "")
+
             if has_preview_collection:
                 # If preview collection: update and sign preview collection
                 updater.destination = resource["preview"]
@@ -180,7 +184,7 @@ def sign_collection_data(event, resources, **kwargs):
                 changes_count = None
             review_event_cls = signer_events.ReviewRequested
             review_event_kw["changes_count"] = changes_count
-            review_event_kw["comment"] = new_collection.get("last_editor_comment", "")
+            review_event_kw["comment"] = comment
 
         elif old_status == STATUS.TO_REVIEW and new_status == STATUS.WORK_IN_PROGRESS:
             review_event_cls = signer_events.ReviewRejected

kinto_signer/updater.py

@@ -223,7 +223,11 @@ def rollback_changes(self, request, refresh_last_edit=True, refresh_signature=Fa
         if refresh_last_edit:
             current_userid = request.prefixed_userid
             current_date = datetime.datetime.now(datetime.timezone.utc).isoformat()
-            attrs = {"status": STATUS.SIGNED.value}
+            attrs = {
+                "status": STATUS.SIGNED.value,
+                "last_editor_comment": "",
+                "last_reviewer_comment": "",
+            }
             attrs[TRACKING_FIELDS.LAST_EDIT_BY.value] = current_userid
             attrs[TRACKING_FIELDS.LAST_EDIT_DATE.value] = current_date
             self._update_source_attributes(request, **attrs)

tests/test_signoff_flow.py

@@ -289,6 +289,26 @@ def test_editor_cannot_be_reviewer(self):
         resp = self.app.get(self.source_collection, headers=self.headers)
         assert resp.json["data"]["status"] == "signed"
 
+    def test_comments_are_reset_on_review(self):
+        self.app.patch_json(
+            self.source_collection,
+            {
+                "data": {
+                    "last_editor_comment": "please check that",
+                    "last_reviewer_comment": "looks good",
+                }
+            },
+            headers=self.headers,
+        )
+
+        self.app.patch_json(
+            self.source_collection, {"data": {"status": "to-review"}}, headers=self.headers
+        )
+
+        resp = self.app.get(self.source_collection, headers=self.headers)
+        assert resp.json["data"]["last_editor_comment"] == ""
+        assert resp.json["data"]["last_reviewer_comment"] == ""
+
 
 class RefreshSignatureTest(SignoffWebTest, unittest.TestCase):
     @classmethod
@@ -585,6 +605,26 @@ def test_tracking_fields_are_updated(self):
         after_date = resp.json["data"]["last_edit_date"]
         assert before_date != after_date
 
+    def test_comments_are_reset(self):
+        self.app.patch_json(
+            self.source_collection,
+            {
+                "data": {
+                    "last_editor_comment": "please check that",
+                    "last_reviewer_comment": "looks good",
+                }
+            },
+            headers=self.headers,
+        )
+
+        self.app.patch_json(
+            self.source_collection, {"data": {"status": "to-rollback"}}, headers=self.headers
+        )
+
+        resp = self.app.get(self.source_collection, headers=self.headers)
+        assert resp.json["data"]["last_editor_comment"] == ""
+        assert resp.json["data"]["last_reviewer_comment"] == ""
+
     def test_recreates_deleted_record(self):
         resp = self.app.delete(
             self.source_collection + "/records?_limit=1&_sort=last_modified", headers=self.headers