Store a separate activity for private notes

Author: diox

src/olympia/abuse/actions.py

@@ -57,20 +57,28 @@ def __init__(self, decision):
             )
 
     def log_action(self, activity_log_action, *extra_args, extra_details=None):
+        user_kw = (
+            {'user': self.decision.reviewer_user} if self.decision.reviewer_user else {}
+        )
+        if self.decision.private_notes:
+            # If the decision contained private notes, add a separate action
+            # for them.
+            log_create(
+                amo.LOG.REVIEWER_PRIVATE_COMMENT,
+                self.target,
+                self.decision,
+                **user_kw,
+                details={
+                    'comments': self.decision.private_notes,
+                },
+            )
         return log_create(
             activity_log_action,
             self.target,
             self.decision,
             *(self.decision.policies.all()),
             *extra_args,
-            **(
-                {'user': self.decision.reviewer_user}
-                if self.decision.reviewer_user
-                else {}
-            ),
-            # FIXME: if the decision had private notes, either record them as
-            # extra details under a specific key, or as a separate activity log
-            # to be consistent with how we do it in reviewer tools.
+            **user_kw,
             details={
                 'comments': self.decision.reasoning,
                 **(

src/olympia/activity/tests/test_utils.py

@@ -536,7 +536,7 @@ def test_review_url_unlisted(self, send_mail_mock):
         self.grant_permission(self.reviewer, 'Addons:ReviewUnlisted', 'Addon Reviewers')
 
         # One from the reviewer.
-        self._create(amo.LOG.COMMENT_VERSION, self.reviewer)
+        self._create(amo.LOG.REVIEWER_PRIVATE_COMMENT, self.reviewer)
         # One from the developer.  So the developer is on the 'thread'
         self._create(amo.LOG.DEVELOPER_REPLY_VERSION, self.developer)
         action = amo.LOG.DEVELOPER_REPLY_VERSION

src/olympia/activity/tests/test_views.py

@@ -349,7 +349,7 @@ def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
 
     def test_admin_activity_hidden_from_developer(self):
         # Add an extra activity note but a type we don't show the developer.
-        self.log('sécrets', amo.LOG.COMMENT_VERSION, self.days_ago(0))
+        self.log('sécrets', amo.LOG.REVIEWER_PRIVATE_COMMENT, self.days_ago(0))
         self._login_developer()
         # _test_url will check only the 3 notes defined in setup are there.
         self._test_url()

src/olympia/constants/activity.py

@@ -242,9 +242,9 @@ class REQUEST_SUPER_REVIEW(_LOG):
     obsolete = True
 
 
-class COMMENT_VERSION(_LOG):
+class REVIEWER_PRIVATE_COMMENT(_LOG):
     id = 49
-    format = '{addon} {version} reviewer comment.'
+    format = 'Reviewer made a private comment.'
     short = 'Commented'
     keep = True
     review_queue = True

src/olympia/devhub/tests/test_feeds.py

@@ -292,7 +292,9 @@ def test_xss_versions_unlisted_addon(self):
 
     def test_hidden(self):
         version = Version.objects.create(addon=self.addon)
-        ActivityLog.objects.create(amo.LOG.COMMENT_VERSION, self.addon, version)
+        ActivityLog.objects.create(
+            amo.LOG.REVIEWER_PRIVATE_COMMENT, self.addon, version
+        )
         res = self.get_response(addon=self.addon.id)
         key = RssKey.objects.get()
         res = self.get_response(privaterss=key.key)

src/olympia/devhub/tests/test_views.py

@@ -639,7 +639,7 @@ def add_log(self, action=amo.LOG.ADD_RATING):
         core.set_user(self.action_user)
         ActivityLog.objects.create(action, self.addon, self.version)
 
-    def add_hidden_log(self, action=amo.LOG.COMMENT_VERSION):
+    def add_hidden_log(self, action=amo.LOG.REVIEWER_PRIVATE_COMMENT):
         self.add_log(action=action)
 
     def test_feed_hidden(self):

src/olympia/reviewers/tests/test_views.py

@@ -450,7 +450,7 @@ def test_addon_missing(self):
         )
 
     def test_comment_logs(self):
-        self.make_an_approval(amo.LOG.COMMENT_VERSION)
+        self.make_an_approval(amo.LOG.REVIEWER_PRIVATE_COMMENT)
         response = self.client.get(self.url)
         assert response.status_code == 200
         assert pq(response.content)('#log-listing tbody td').eq(1).html().strip() == (
@@ -2627,7 +2627,7 @@ def test_comment(self):
         assert response.status_code == 302
         assert len(mail.outbox) == 0
 
-        comment_version = amo.LOG.COMMENT_VERSION
+        comment_version = amo.LOG.REVIEWER_PRIVATE_COMMENT
         assert ActivityLog.objects.filter(action=comment_version.id).count() == 1
 
     @mock.patch('olympia.reviewers.utils.report_decision_to_cinder_and_notify.delay')

src/olympia/reviewers/utils.py

@@ -1208,7 +1208,7 @@ def sign_file(self):
             sign_file(self.file)
 
     def process_comment(self):
-        self.log_action(amo.LOG.COMMENT_VERSION)
+        self.log_action(amo.LOG.REVIEWER_PRIVATE_COMMENT)
 
     def resolve_reports_job(self):
         if self.data.get('cinder_jobs_to_resolve', ()):