From 3b196919d3ce7539efc50de813d5dd2cd4b0e91c Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 15 Jun 2021 10:36:40 +0200 Subject: [PATCH] Reset the editor/reviewer comments when not specified --- CHANGELOG.rst | 4 ++++ kinto_signer/listeners.py | 6 +++++- kinto_signer/updater.py | 6 +++++- tests/test_signoff_flow.py | 40 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1d0d43e2..0616fb62 100644 --- a/CHANGELOG.rst +++ b/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) ------------------ diff --git a/kinto_signer/listeners.py b/kinto_signer/listeners.py index 5ff0b32d..cc18c5fb 100644 --- a/kinto_signer/listeners.py +++ b/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 diff --git a/kinto_signer/updater.py b/kinto_signer/updater.py index aaf8ac16..c0124166 100644 --- a/kinto_signer/updater.py +++ b/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) diff --git a/tests/test_signoff_flow.py b/tests/test_signoff_flow.py index 052a061c..4a2c39b1 100644 --- a/tests/test_signoff_flow.py +++ b/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