Reviewer Tools Pastebin Re-implementation (#22640)

* Added pastebin reimplementation on reviewer tools side * make lint * Moved attachment upload box to own section (and available for all actions) * Fixed css for textareas, returned datetime-wrapper * Missing assignment * Updated naming for files, limited to .txt and .zip, temporarily updated query number assertion * added waffle enable-activity-log-attachments for user facing menus; * testing * corrected test * Removed zip allowance * migration conflict * test (temp) * import * remove git_extraction.py from pr * ??? * numQueries * review changes * lint * prettier * review changes * lol lint * review comment
mozilla · Sep 13, 2024 · c6026f6 · c6026f6
1 parent 6744fdc
commit c6026f6
Show file tree

Hide file tree

Showing 22 changed files with 416 additions and 14 deletions.
diff --git a/src/olympia/activity/api_urls.py b/src/olympia/activity/api_urls.py
@@ -0,0 +1,8 @@
+from django.urls import re_path
+
+from . import views
+
+
+urlpatterns = [
+    re_path(r'^mail/', views.inbound_email, name='inbound-email-api'),
+]
diff --git a/src/olympia/activity/migrations/0026_alter_activitylog_action_attachmentlog.py b/src/olympia/activity/migrations/0026_alter_activitylog_action_attachmentlog.py
@@ -0,0 +1,37 @@
+# Generated by Django 4.2.15 on 2024-09-04 15:46
+
+from django.db import migrations, models
+import django.db.models.deletion
+import django.utils.timezone
+import olympia.activity.models
+import olympia.amo.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('activity', '0025_alter_activitylog_action_cinderpolicylog'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='activitylog',
+            name='action',
+            field=models.SmallIntegerField(choices=[(1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8), (9, 9), (12, 12), (16, 16), (17, 17), (18, 18), (19, 19), (20, 20), (21, 21), (22, 22), (23, 23), (24, 24), (25, 25), (26, 26), (27, 27), (28, 28), (29, 29), (31, 31), (32, 32), (33, 33), (34, 34), (35, 35), (36, 36), (37, 37), (38, 38), (39, 39), (40, 40), (41, 41), (42, 42), (43, 43), (44, 44), (45, 45), (46, 46), (47, 47), (48, 48), (49, 49), (53, 53), (60, 60), (61, 61), (62, 62), (98, 98), (99, 99), (100, 100), (101, 101), (102, 102), (103, 103), (104, 104), (105, 105), (106, 106), (107, 107), (108, 108), (109, 109), (110, 110), (120, 120), (121, 121), (128, 128), (130, 130), (131, 131), (132, 132), (133, 133), (134, 134), (135, 135), (136, 136), (137, 137), (138, 138), (139, 139), (140, 140), (141, 141), (142, 142), (143, 143), (144, 144), (145, 145), (146, 146), (147, 147), (148, 148), (149, 149), (150, 150), (151, 151), (152, 152), (153, 153), (154, 154), (155, 155), (156, 156), (157, 157), (158, 158), (159, 159), (160, 160), (161, 161), (162, 162), (163, 163), (164, 164), (165, 165), (166, 166), (167, 167), (168, 168), (169, 169), (170, 170), (171, 171), (172, 172), (173, 173), (174, 174), (175, 175), (176, 176), (177, 177), (178, 178), (179, 179), (180, 180), (181, 181), (182, 182), (183, 183), (184, 184), (185, 185), (186, 186), (187, 187), (188, 188), (189, 189), (190, 190), (191, 191), (192, 192)]),
+        ),
+        migrations.CreateModel(
+            name='AttachmentLog',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', models.DateTimeField(blank=True, default=django.utils.timezone.now, editable=False)),
+                ('modified', models.DateTimeField(auto_now=True)),
+                ('file', models.FileField(blank=True, null=True, storage=olympia.activity.models.activity_attachment_storage, upload_to=olympia.activity.models.attachment_upload_path)),
+                ('activity_log', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='activity.activitylog')),
+            ],
+            options={
+                'db_table': 'log_activity_attachment',
+                'ordering': ('-created',),
+            },
+            bases=(olympia.amo.models.SaveUpdateMixin, models.Model),
+        ),
+    ]
diff --git a/src/olympia/activity/models.py b/src/olympia/activity/models.py
@@ -1,7 +1,9 @@
 import json
+import os
 import uuid
 from collections import defaultdict
 from copy import copy
+from datetime import datetime
 from inspect import isclass
 
 from django.apps import apps
@@ -20,6 +22,7 @@
 from olympia.addons.models import Addon
 from olympia.amo.fields import IPAddressBinaryField, PositiveAutoField
 from olympia.amo.models import BaseQuerySet, LongNameIndex, ManagerBase, ModelBase
+from olympia.amo.utils import SafeStorage, id_to_path
 from olympia.bandwagon.models import Collection
 from olympia.blocklist.models import Block
 from olympia.constants.activity import _LOG
@@ -39,6 +42,20 @@
 GENERIC_USER_NAME = gettext('Add-ons Review Team')
 
 
+def attachment_upload_path(instance, filename):
+    ext = os.path.splitext(filename)[1]
+    timestamp = datetime.now().replace(microsecond=0)
+    return os.path.join(
+        'activity_attachment',
+        id_to_path(instance.activity_log.pk, breadth=1),
+        f'{timestamp}{ext}',
+    )
+
+
+def activity_attachment_storage():
+    return SafeStorage(root_setting='MEDIA_ROOT')
+
+
 class GenericMozillaUser(UserProfile):
     class Meta:
         proxy = True
@@ -262,6 +279,24 @@ class Meta:
         ordering = ('-created',)
 
 
+class AttachmentLog(ModelBase):
+    """
+    This table is for indexing the activity log by attachment.
+    """
+
+    activity_log = models.OneToOneField('ActivityLog', on_delete=models.CASCADE)
+    file = models.FileField(
+        upload_to=attachment_upload_path,
+        storage=activity_attachment_storage,
+        blank=True,
+        null=True,
+    )
+
+    class Meta:
+        db_table = 'log_activity_attachment'
+        ordering = ('-created',)
+
+
 class DraftComment(ModelBase):
     """A model that allows us to draft comments for reviews before we have
     an ActivityLog instance ready.

diff --git a/src/olympia/activity/tests/test_models.py b/src/olympia/activity/tests/test_models.py
@@ -2,6 +2,8 @@
 from unittest.mock import Mock
 from uuid import UUID
 
+from django.core.files.base import ContentFile
+
 from pyquery import PyQuery as pq
 
 from olympia import amo, core
@@ -11,10 +13,12 @@
     ActivityLog,
     ActivityLogToken,
     AddonLog,
+    AttachmentLog,
     DraftComment,
     GenericMozillaUser,
     IPLog,
     ReviewActionReasonLog,
+    attachment_upload_path,
 )
 from olympia.addons.models import Addon, AddonUser
 from olympia.amo.tests import (
@@ -507,6 +511,13 @@ def test_create_action_arg_or_kwarg(self):
             ActivityLog.objects.create(action=amo.LOG.ADD_TAG.id).log == amo.LOG.ADD_TAG
         )
 
+    def test_attachment_upload_path(self):
+        log = ActivityLog.objects.create(amo.LOG.CUSTOM_TEXT, 'Test Attachment Log')
+        attachment = ContentFile('Pseudo File', name='attachment.txt')
+        attachment_log = AttachmentLog.objects.create(activity_log=log, file=attachment)
+        uploaded_name = attachment_upload_path(attachment_log, attachment.name)
+        assert uploaded_name.endswith('.txt')
+
 
 class TestDraftComment(TestCase):
     def test_default_requirements(self):

diff --git a/src/olympia/activity/tests/test_views.py b/src/olympia/activity/tests/test_views.py
@@ -4,13 +4,19 @@
 from unittest import mock
 
 from django.conf import settings
+from django.core.files.base import ContentFile
 from django.test.utils import override_settings
 
 from rest_framework.exceptions import ErrorDetail
 from rest_framework.test import APIRequestFactory
 
 from olympia import amo
-from olympia.activity.models import GENERIC_USER_NAME, ActivityLog, ActivityLogToken
+from olympia.activity.models import (
+    GENERIC_USER_NAME,
+    ActivityLog,
+    ActivityLogToken,
+    AttachmentLog,
+)
 from olympia.activity.tests.test_serializers import LogMixin
 from olympia.activity.tests.test_utils import sample_message_content
 from olympia.activity.views import InboundEmailIPPermission, inbound_email
@@ -20,6 +26,7 @@
     AddonUser,
 )
 from olympia.addons.utils import generate_addon_guid
+from olympia.amo.reverse import reverse
 from olympia.amo.tests import (
     APITestClientSessionID,
     TestCase,
@@ -660,3 +667,33 @@ def test_validation_response_wrong_secret(self, _mock):
         res = inbound_email(req)
         assert not _mock.called
         assert res.status_code == 403
+
+
+class TestDownloadAttachment(TestCase):
+    def setUp(self):
+        super().setUp()
+        self.addon = addon_factory(
+            guid=generate_addon_guid(), name='My Addôn', slug='my-addon'
+        )
+        self.user = user_factory(email='[email protected]')
+        self.log = ActivityLog.objects.create(
+            user=self.user, action=amo.LOG.REVIEWER_REPLY_VERSION
+        )
+        self.attachment = AttachmentLog.objects.create(
+            activity_log=self.log,
+            file=ContentFile('Pseudo File', name='attachment.txt'),
+        )
+
+    def test_download_attachment_success(self):
+        self.client.force_login(self.user)
+        self.grant_permission(self.user, 'Addons:Review', 'Addon Reviewers')
+        url = reverse('activity.attachment', args=[self.log.pk])
+        response = self.client.get(url, follow=True)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn('.txt', response['Content-Disposition'])
+
+    def test_download_attachment_failure(self):
+        self.client.force_login(self.user)
+        url = reverse('activity.attachment', args=[self.log.pk])
+        response = self.client.get(url, follow=True)
+        self.assertEqual(response.status_code, 404)
diff --git a/src/olympia/activity/urls.py b/src/olympia/activity/urls.py
@@ -4,5 +4,9 @@
 
 
 urlpatterns = [
-    re_path(r'^mail/', views.inbound_email, name='inbound-email-api'),
+    re_path(
+        r'^attachment/(?P<log_id>\d+)',
+        views.download_attachment,
+        name='activity.attachment',
+    )
 ]
diff --git a/src/olympia/activity/views.py b/src/olympia/activity/views.py
@@ -1,4 +1,8 @@
+import os
+
+from django import http
 from django.conf import settings
+from django.db.transaction import non_atomic_requests
 from django.shortcuts import get_object_or_404
 from django.utils.translation import gettext, gettext_lazy as _
 
@@ -27,6 +31,7 @@
     log_and_notify,
 )
 from olympia.addons.views import AddonChildMixin
+from olympia.amo.utils import HttpResponseXSendFile
 from olympia.api.permissions import (
     AllowAddonAuthor,
     AllowListedViewerOrReviewer,
@@ -166,3 +171,26 @@ def check_content_length(request):
     spam_rating = request.data.get('SpamScore', 0.0)
     process_email.apply_async((message, spam_rating))
     return Response(data=validation_response, status=status.HTTP_201_CREATED)
+
+
+@non_atomic_requests
+def download_attachment(request, log_id):
+    """
+    Download attachment for a given activity log.
+    """
+    log = get_object_or_404(ActivityLog, pk=log_id)
+    attachmentlog = log.attachmentlog
+
+    is_reviewer = acl.action_allowed_for(request.user, amo.permissions.ADDONS_REVIEW)
+    if not is_reviewer:
+        raise http.Http404()
+
+    response = HttpResponseXSendFile(request, attachmentlog.file.path)
+    path = attachmentlog.file.path
+    if not isinstance(path, str):
+        path = path.decode('utf8')
+    name = os.path.basename(path.replace('"', ''))
+    disposition = f'attachment; filename="{name}"'.encode()
+    response['Content-Disposition'] = disposition
+    response['Access-Control-Allow-Origin'] = '*'
+    return response
diff --git a/src/olympia/api/urls.py b/src/olympia/api/urls.py
@@ -59,13 +59,13 @@ def get_versioned_api_routes(version, url_patterns):
     re_path(r'^reviews/', include(ratings_v3.urls)),
     re_path(r'^reviewers/', include('olympia.reviewers.api_urls')),
     re_path(r'^', include('olympia.signing.urls')),
-    re_path(r'^activity/', include('olympia.activity.urls')),
+    re_path(r'^activity/', include('olympia.activity.api_urls')),
 ]
 
 v4_api_urls = [
     re_path(r'^abuse/', include('olympia.abuse.api_urls')),
     re_path(r'^accounts/', include(accounts_v4)),
-    re_path(r'^activity/', include('olympia.activity.urls')),
+    re_path(r'^activity/', include('olympia.activity.api_urls')),
     re_path(r'^addons/', include(addons_v4)),
     re_path(r'^applications/', include('olympia.applications.api_urls')),
     re_path(r'^blocklist/', include('olympia.blocklist.urls')),
@@ -81,7 +81,7 @@ def get_versioned_api_routes(version, url_patterns):
 v5_api_urls = [
     re_path(r'^abuse/', include('olympia.abuse.api_urls')),
     re_path(r'^accounts/', include(accounts_v4)),
-    re_path(r'^activity/', include('olympia.activity.urls')),
+    re_path(r'^activity/', include('olympia.activity.api_urls')),
     re_path(r'^addons/', include(addons_v5)),
     re_path(r'^applications/', include('olympia.applications.api_urls')),
     re_path(r'^blocklist/', include('olympia.blocklist.urls')),

diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py
@@ -264,6 +264,7 @@ def get_db_config(environ_var, atomic_requests=True):
     'abuse',
     'admin',
     'apps',
+    'activity',
     'contribute.json',
     'developer_agreement',
     'developers',
@@ -289,6 +290,7 @@ def get_db_config(environ_var, atomic_requests=True):
 # This needs to be kept in sync with addons-frontend's validLocaleUrlExceptions
 # https://github.com/mozilla/addons-frontend/blob/master/config/default-amo.js
 SUPPORTED_NONLOCALES = (
+    'activity',
     'contribute.json',
     'google1f3e37b7351799a5.html',
     'google231a41e803e464e9.html',

diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py
@@ -10,6 +10,7 @@
     modelformset_factory,
 )
 from django.utils.html import format_html, format_html_join
+from django.utils.translation import gettext
 
 import markupsafe
 
@@ -41,6 +42,8 @@
 
 ACTION_DICT = dict(approved=amo.LOG.APPROVE_RATING, deleted=amo.LOG.DELETE_RATING)
 
+VALID_ATTACHMENT_EXTENSIONS = ('.txt',)
+
 
 class RatingModerationLogForm(forms.Form):
     start = forms.DateField(required=False, label='View entries between')
@@ -299,6 +302,19 @@ def create_option(self, *args, **kwargs):
         return option
 
 
+def validate_review_attachment(value):
+    if value:
+        if not value.name.endswith(VALID_ATTACHMENT_EXTENSIONS):
+            valid_extensions_string = '(%s)' % ', '.join(VALID_ATTACHMENT_EXTENSIONS)
+            raise forms.ValidationError(
+                gettext(
+                    'Unsupported file type, please upload a '
+                    'file {extensions}.'.format(extensions=valid_extensions_string)
+                )
+            )
+    return value
+
+
 class ReviewForm(forms.Form):
     # Hack to restore behavior from pre Django 1.10 times.
     # Django 1.10 enabled `required` rendering for required widgets. That
@@ -363,6 +379,11 @@ class ReviewForm(forms.Form):
         required=True,
         widget=ReasonsChoiceWidget,
     )
+    attachment_file = forms.FileField(
+        required=False, validators=[validate_review_attachment]
+    )
+    attachment_input = forms.CharField(required=False, widget=forms.Textarea())
+
     version_pk = forms.IntegerField(required=False, min_value=1)
     cinder_jobs_to_resolve = WidgetRenderedModelMultipleChoiceField(
         label='Outstanding DSA related reports to resolve:',
@@ -413,6 +434,10 @@ def clean(self):
         # If the user select a different type of job before changing actions there could
         # be non-appeal jobs selected as cinder_jobs_to_resolve under resolve_appeal_job
         # action, or appeal jobs under resolve_reports_job action. So filter them out.
+        if self.cleaned_data.get('attachment_input') and self.cleaned_data.get(
+            'attachment_file'
+        ):
+            raise ValidationError('Cannot upload both a file and input.')
         if self.cleaned_data.get('action') == 'resolve_appeal_job':
             self.cleaned_data['cinder_jobs_to_resolve'] = [
                 job

diff --git a/src/olympia/reviewers/migrations/0038_auto_20240909_1647.py b/src/olympia/reviewers/migrations/0038_auto_20240909_1647.py
@@ -0,0 +1,16 @@
+# Generated by Django 4.2.15 on 2024-09-09 16:47
+
+from django.db import migrations
+
+from olympia.core.db.migrations import CreateWaffleSwitch
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('reviewers', '0037_autoapprovalsummary_is_pending_rejection'),
+    ]
+
+    operations = [
+        CreateWaffleSwitch('enable-activity-log-attachments')
+    ]
diff --git a/src/olympia/reviewers/templates/reviewers/includes/history.html b/src/olympia/reviewers/templates/reviewers/includes/history.html
@@ -14,4 +14,15 @@
       {% endif %}
     {% endif %}
   </td>
+  {% if switch_is_active('enable-activity-log-attachments') %}
+    <td>
+      {% if record.attachmentlog %}
+        {% with attachment=record.attachmentlog.file %}
+        <div>
+          <a class="download-reply-attachment" href="{{ url('activity.attachment', record.pk) }}" download>Download Attachment</a> ({{attachment.size|filesizeformat}})
+        </div>
+        {% endwith %}
+      {% endif %}
+    </td>
+  {% endif %}
 </tr>