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
This commit is contained in:
Christina Lin 2024-09-13 13:58:33 -04:00 коммит произвёл GitHub
Родитель 6744fdcc77
Коммит c6026f6d89
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
22 изменённых файлов: 416 добавлений и 14 удалений

Просмотреть файл

@ -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'),
]

Просмотреть файл

@ -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),
),
]

Просмотреть файл

@ -1,7 +1,9 @@
import json import json
import os
import uuid import uuid
from collections import defaultdict from collections import defaultdict
from copy import copy from copy import copy
from datetime import datetime
from inspect import isclass from inspect import isclass
from django.apps import apps from django.apps import apps
@ -20,6 +22,7 @@ from olympia.access.models import Group
from olympia.addons.models import Addon from olympia.addons.models import Addon
from olympia.amo.fields import IPAddressBinaryField, PositiveAutoField from olympia.amo.fields import IPAddressBinaryField, PositiveAutoField
from olympia.amo.models import BaseQuerySet, LongNameIndex, ManagerBase, ModelBase 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.bandwagon.models import Collection
from olympia.blocklist.models import Block from olympia.blocklist.models import Block
from olympia.constants.activity import _LOG from olympia.constants.activity import _LOG
@ -39,6 +42,20 @@ MAX_TOKEN_USE_COUNT = 100
GENERIC_USER_NAME = gettext('Add-ons Review Team') 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 GenericMozillaUser(UserProfile):
class Meta: class Meta:
proxy = True proxy = True
@ -262,6 +279,24 @@ class RatingLog(ModelBase):
ordering = ('-created',) 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): class DraftComment(ModelBase):
"""A model that allows us to draft comments for reviews before we have """A model that allows us to draft comments for reviews before we have
an ActivityLog instance ready. an ActivityLog instance ready.

Просмотреть файл

@ -2,6 +2,8 @@ from ipaddress import IPv4Address
from unittest.mock import Mock from unittest.mock import Mock
from uuid import UUID from uuid import UUID
from django.core.files.base import ContentFile
from pyquery import PyQuery as pq from pyquery import PyQuery as pq
from olympia import amo, core from olympia import amo, core
@ -11,10 +13,12 @@ from olympia.activity.models import (
ActivityLog, ActivityLog,
ActivityLogToken, ActivityLogToken,
AddonLog, AddonLog,
AttachmentLog,
DraftComment, DraftComment,
GenericMozillaUser, GenericMozillaUser,
IPLog, IPLog,
ReviewActionReasonLog, ReviewActionReasonLog,
attachment_upload_path,
) )
from olympia.addons.models import Addon, AddonUser from olympia.addons.models import Addon, AddonUser
from olympia.amo.tests import ( from olympia.amo.tests import (
@ -507,6 +511,13 @@ class TestActivityLog(TestCase):
ActivityLog.objects.create(action=amo.LOG.ADD_TAG.id).log == amo.LOG.ADD_TAG 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): class TestDraftComment(TestCase):
def test_default_requirements(self): def test_default_requirements(self):

Просмотреть файл

@ -4,13 +4,19 @@ from datetime import datetime, timedelta
from unittest import mock from unittest import mock
from django.conf import settings from django.conf import settings
from django.core.files.base import ContentFile
from django.test.utils import override_settings from django.test.utils import override_settings
from rest_framework.exceptions import ErrorDetail from rest_framework.exceptions import ErrorDetail
from rest_framework.test import APIRequestFactory from rest_framework.test import APIRequestFactory
from olympia import amo 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_serializers import LogMixin
from olympia.activity.tests.test_utils import sample_message_content from olympia.activity.tests.test_utils import sample_message_content
from olympia.activity.views import InboundEmailIPPermission, inbound_email from olympia.activity.views import InboundEmailIPPermission, inbound_email
@ -20,6 +26,7 @@ from olympia.addons.models import (
AddonUser, AddonUser,
) )
from olympia.addons.utils import generate_addon_guid from olympia.addons.utils import generate_addon_guid
from olympia.amo.reverse import reverse
from olympia.amo.tests import ( from olympia.amo.tests import (
APITestClientSessionID, APITestClientSessionID,
TestCase, TestCase,
@ -660,3 +667,33 @@ class TestEmailApi(TestCase):
res = inbound_email(req) res = inbound_email(req)
assert not _mock.called assert not _mock.called
assert res.status_code == 403 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='admin@mozilla.com')
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)

Просмотреть файл

@ -4,5 +4,9 @@ from . import views
urlpatterns = [ 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',
)
] ]

Просмотреть файл

@ -1,4 +1,8 @@
import os
from django import http
from django.conf import settings from django.conf import settings
from django.db.transaction import non_atomic_requests
from django.shortcuts import get_object_or_404 from django.shortcuts import get_object_or_404
from django.utils.translation import gettext, gettext_lazy as _ from django.utils.translation import gettext, gettext_lazy as _
@ -27,6 +31,7 @@ from olympia.activity.utils import (
log_and_notify, log_and_notify,
) )
from olympia.addons.views import AddonChildMixin from olympia.addons.views import AddonChildMixin
from olympia.amo.utils import HttpResponseXSendFile
from olympia.api.permissions import ( from olympia.api.permissions import (
AllowAddonAuthor, AllowAddonAuthor,
AllowListedViewerOrReviewer, AllowListedViewerOrReviewer,
@ -166,3 +171,26 @@ def inbound_email(request):
spam_rating = request.data.get('SpamScore', 0.0) spam_rating = request.data.get('SpamScore', 0.0)
process_email.apply_async((message, spam_rating)) process_email.apply_async((message, spam_rating))
return Response(data=validation_response, status=status.HTTP_201_CREATED) 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

Просмотреть файл

@ -59,13 +59,13 @@ v3_api_urls = [
re_path(r'^reviews/', include(ratings_v3.urls)), re_path(r'^reviews/', include(ratings_v3.urls)),
re_path(r'^reviewers/', include('olympia.reviewers.api_urls')), re_path(r'^reviewers/', include('olympia.reviewers.api_urls')),
re_path(r'^', include('olympia.signing.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 = [ v4_api_urls = [
re_path(r'^abuse/', include('olympia.abuse.api_urls')), re_path(r'^abuse/', include('olympia.abuse.api_urls')),
re_path(r'^accounts/', include(accounts_v4)), 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'^addons/', include(addons_v4)),
re_path(r'^applications/', include('olympia.applications.api_urls')), re_path(r'^applications/', include('olympia.applications.api_urls')),
re_path(r'^blocklist/', include('olympia.blocklist.urls')), re_path(r'^blocklist/', include('olympia.blocklist.urls')),
@ -81,7 +81,7 @@ v4_api_urls = [
v5_api_urls = [ v5_api_urls = [
re_path(r'^abuse/', include('olympia.abuse.api_urls')), re_path(r'^abuse/', include('olympia.abuse.api_urls')),
re_path(r'^accounts/', include(accounts_v4)), 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'^addons/', include(addons_v5)),
re_path(r'^applications/', include('olympia.applications.api_urls')), re_path(r'^applications/', include('olympia.applications.api_urls')),
re_path(r'^blocklist/', include('olympia.blocklist.urls')), re_path(r'^blocklist/', include('olympia.blocklist.urls')),

Просмотреть файл

@ -264,6 +264,7 @@ SUPPORTED_NONAPPS = (
'abuse', 'abuse',
'admin', 'admin',
'apps', 'apps',
'activity',
'contribute.json', 'contribute.json',
'developer_agreement', 'developer_agreement',
'developers', 'developers',
@ -289,6 +290,7 @@ DEFAULT_APP = 'firefox'
# This needs to be kept in sync with addons-frontend's validLocaleUrlExceptions # This needs to be kept in sync with addons-frontend's validLocaleUrlExceptions
# https://github.com/mozilla/addons-frontend/blob/master/config/default-amo.js # https://github.com/mozilla/addons-frontend/blob/master/config/default-amo.js
SUPPORTED_NONLOCALES = ( SUPPORTED_NONLOCALES = (
'activity',
'contribute.json', 'contribute.json',
'google1f3e37b7351799a5.html', 'google1f3e37b7351799a5.html',
'google231a41e803e464e9.html', 'google231a41e803e464e9.html',

Просмотреть файл

@ -10,6 +10,7 @@ from django.forms.models import (
modelformset_factory, modelformset_factory,
) )
from django.utils.html import format_html, format_html_join from django.utils.html import format_html, format_html_join
from django.utils.translation import gettext
import markupsafe import markupsafe
@ -41,6 +42,8 @@ ACTION_FILTERS = (
ACTION_DICT = dict(approved=amo.LOG.APPROVE_RATING, deleted=amo.LOG.DELETE_RATING) ACTION_DICT = dict(approved=amo.LOG.APPROVE_RATING, deleted=amo.LOG.DELETE_RATING)
VALID_ATTACHMENT_EXTENSIONS = ('.txt',)
class RatingModerationLogForm(forms.Form): class RatingModerationLogForm(forms.Form):
start = forms.DateField(required=False, label='View entries between') start = forms.DateField(required=False, label='View entries between')
@ -299,6 +302,19 @@ class ActionChoiceWidget(forms.RadioSelect):
return option 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): class ReviewForm(forms.Form):
# Hack to restore behavior from pre Django 1.10 times. # Hack to restore behavior from pre Django 1.10 times.
# Django 1.10 enabled `required` rendering for required widgets. That # Django 1.10 enabled `required` rendering for required widgets. That
@ -363,6 +379,11 @@ class ReviewForm(forms.Form):
required=True, required=True,
widget=ReasonsChoiceWidget, 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) version_pk = forms.IntegerField(required=False, min_value=1)
cinder_jobs_to_resolve = WidgetRenderedModelMultipleChoiceField( cinder_jobs_to_resolve = WidgetRenderedModelMultipleChoiceField(
label='Outstanding DSA related reports to resolve:', label='Outstanding DSA related reports to resolve:',
@ -413,6 +434,10 @@ class ReviewForm(forms.Form):
# If the user select a different type of job before changing actions there could # 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 # 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. # 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': if self.cleaned_data.get('action') == 'resolve_appeal_job':
self.cleaned_data['cinder_jobs_to_resolve'] = [ self.cleaned_data['cinder_jobs_to_resolve'] = [
job job

Просмотреть файл

@ -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')
]

Просмотреть файл

@ -14,4 +14,15 @@
{% endif %} {% endif %}
{% endif %} {% endif %}
</td> </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> </tr>

Просмотреть файл

@ -102,7 +102,7 @@
</form> </form>
{% endif %} {% endif %}
<form method="POST" action="#review-actions" class="review-form"> <form method="POST" enctype="multipart/form-data" action="#review-actions" class="review-form">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="version_pk" value="{{ version.pk }}"/> <input type="hidden" name="version_pk" value="{{ version.pk }}"/>
@ -261,6 +261,23 @@
</div> </div>
</div> </div>
</div> </div>
{% if switch_is_active('enable-activity-log-attachments') %}
<div class="review-actions-section review-actions-attachment-reply">
<label>Attachment:</label>
<div id="attachment-type-toggle">
<button type="button" id="toggle_attachment_file">Upload</button>
or <a id="toggle_attachment_input">Paste from Clipboard</a>
</div>
<div id="attachment_input_wrapper" class="hidden">
{{ form.attachment_input }}
</div>
<div id="attachment_file_wrapper" class="hidden">
{{ form.attachment_file }}
</div>
{{ form.attachment_input.errors }}
{{ form.attachment_file.errors }}
</div>
{% endif %}
<div class="review-actions-section review-actions-save"> <div class="review-actions-section review-actions-save">
<span class="currently_viewing_warning"> <span class="currently_viewing_warning">
{% trans %} {% trans %}

Просмотреть файл

@ -1,6 +1,7 @@
import uuid import uuid
from datetime import datetime from datetime import datetime
from django.core.files.base import ContentFile
from django.utils.encoding import force_str from django.utils.encoding import force_str
from pyquery import PyQuery as pq from pyquery import PyQuery as pq
@ -48,9 +49,10 @@ class TestReviewForm(TestCase):
self.request = FakeRequest() self.request = FakeRequest()
self.file = self.version.file self.file = self.version.file
def get_form(self, data=None): def get_form(self, data=None, files=None):
return ReviewForm( return ReviewForm(
data=data, data=data,
files=files,
helper=ReviewHelper( helper=ReviewHelper(
addon=self.addon, version=self.version, user=self.request.user addon=self.addon, version=self.version, user=self.request.user
), ),
@ -1134,3 +1136,25 @@ class TestReviewForm(TestCase):
# only policies that are expose_in_reviewer_tools=True should be included # only policies that are expose_in_reviewer_tools=True should be included
policy_exposed policy_exposed
] ]
def test_upload_attachment(self):
self.grant_permission(self.request.user, 'Addons:Review')
attachment = ContentFile('Pseudo File', name='attachment.txt')
data = {
'action': 'reply',
'comments': 'lol',
}
files = {'attachment_file': attachment}
form = self.get_form(data=data, files=files)
assert form.is_valid()
assert not form.errors
data['attachment_input'] = 'whee'
form = self.get_form(data=data)
assert form.is_valid()
assert not form.errors
form = self.get_form(data=data, files=files)
assert not form.is_valid()
assert form.errors == {'__all__': ['Cannot upload both a file and input.']}

Просмотреть файл

@ -5,6 +5,7 @@ from unittest.mock import call, patch
from django.conf import settings from django.conf import settings
from django.core import mail from django.core import mail
from django.core.files.base import ContentFile
from django.core.files.storage import default_storage as storage from django.core.files.storage import default_storage as storage
from django.test.utils import override_settings from django.test.utils import override_settings
from django.urls import reverse from django.urls import reverse
@ -17,6 +18,7 @@ from olympia.abuse.models import AbuseReport, CinderDecision, CinderJob, CinderP
from olympia.activity.models import ( from olympia.activity.models import (
ActivityLog, ActivityLog,
ActivityLogToken, ActivityLogToken,
AttachmentLog,
CinderPolicyLog, CinderPolicyLog,
ReviewActionReasonLog, ReviewActionReasonLog,
) )
@ -1162,6 +1164,30 @@ class TestReviewHelper(TestReviewHelperBase):
assert logs.count() == 1 assert logs.count() == 1
assert logs[0].user == task_user assert logs[0].user == task_user
def test_log_action_attachment_input(self):
assert AttachmentLog.objects.count() == 0
data = self.get_data()
text = 'This is input'
data['attachment_input'] = 'This is input'
self.helper.set_data(data)
self.helper.handler.log_action(amo.LOG.REJECT_VERSION)
assert AttachmentLog.objects.count() == 1
attachment_log = AttachmentLog.objects.first()
file_content = attachment_log.file.read().decode('utf-8')
assert file_content == text
def test_log_action_attachment_file(self):
assert AttachmentLog.objects.count() == 0
text = "I'm a text file"
data = self.get_data()
data['attachment_file'] = ContentFile(text, name='attachment.txt')
self.helper.set_data(data)
self.helper.handler.log_action(amo.LOG.REJECT_VERSION)
assert AttachmentLog.objects.count() == 1
attachment_log = AttachmentLog.objects.first()
file_content = attachment_log.file.read().decode('utf-8')
assert file_content == text
@patch('olympia.reviewers.utils.notify_addon_decision_to_cinder.delay') @patch('olympia.reviewers.utils.notify_addon_decision_to_cinder.delay')
@patch('olympia.reviewers.utils.resolve_job_in_cinder.delay') @patch('olympia.reviewers.utils.resolve_job_in_cinder.delay')
def test_notify_decision_calls_resolve_job_in_cinder( def test_notify_decision_calls_resolve_job_in_cinder(

Просмотреть файл

@ -10,7 +10,7 @@ from django.conf import settings
from django.core import mail from django.core import mail
from django.core.cache import cache from django.core.cache import cache
from django.core.files import temp from django.core.files import temp
from django.core.files.base import File as DjangoFile from django.core.files.base import ContentFile, File as DjangoFile
from django.db import connection, reset_queries from django.db import connection, reset_queries
from django.template import defaultfilters from django.template import defaultfilters
from django.test.client import RequestFactory from django.test.client import RequestFactory
@ -30,7 +30,7 @@ from olympia.abuse.models import AbuseReport, CinderDecision, CinderJob, CinderP
from olympia.access import acl from olympia.access import acl
from olympia.access.models import Group, GroupUser from olympia.access.models import Group, GroupUser
from olympia.accounts.serializers import BaseUserSerializer from olympia.accounts.serializers import BaseUserSerializer
from olympia.activity.models import ActivityLog, DraftComment from olympia.activity.models import ActivityLog, AttachmentLog, DraftComment
from olympia.addons.models import ( from olympia.addons.models import (
UPCOMING_DUE_DATE_CUT_OFF_DAYS_CONFIG_DEFAULT, UPCOMING_DUE_DATE_CUT_OFF_DAYS_CONFIG_DEFAULT,
Addon, Addon,
@ -2611,6 +2611,65 @@ class TestReview(ReviewBase):
assert len(mail.outbox) == 1 assert len(mail.outbox) == 1
self.assertTemplateUsed(response, 'activity/emails/from_reviewer.txt') self.assertTemplateUsed(response, 'activity/emails/from_reviewer.txt')
def test_attachment_input(self):
self.client.post(
self.url,
{'action': 'reply', 'comments': 'hello sailor'},
)
# A regular reply does not create an AttachmentLog.
assert AttachmentLog.objects.count() == 0
text = 'babys first build log'
response = self.client.post(
self.url,
{
'action': 'reply',
'comments': 'hello sailor',
'attachment_input': text,
},
)
assert response.status_code == 302
assert AttachmentLog.objects.count() == 1
attachment_log = AttachmentLog.objects.first()
file_content = attachment_log.file.read().decode('utf-8')
assert file_content == text
def test_attachment_valid_upload(self):
assert AttachmentLog.objects.count() == 0
text = "I'm a text file"
attachment = ContentFile(text, name='attachment.txt')
response = self.client.post(
self.url,
{
'action': 'reply',
'comments': 'hello sailor',
'attachment_file': attachment,
},
)
assert response.status_code == 302
assert AttachmentLog.objects.count() == 1
attachment_log = AttachmentLog.objects.first()
file_content = attachment_log.file.read().decode('utf-8')
assert file_content == text
@override_switch('enable-activity-log-attachments', active=True)
def test_attachment_invalid_upload(self):
assert AttachmentLog.objects.count() == 0
attachment = ContentFile("I'm not a text file", name='attachment.png')
response = self.client.post(
self.url,
{
'action': 'reply',
'comments': 'hello sailor',
'attachment_file': attachment,
},
)
assert response.status_code != 302
assert AttachmentLog.objects.count() == 0
self.assertIn(
'Unsupported file type, please upload a file (.txt)',
response.content.decode('utf-8'),
)
def test_page_title(self): def test_page_title(self):
response = self.client.get(self.url) response = self.client.get(self.url)
assert response.status_code == 200 assert response.status_code == 200
@ -2704,7 +2763,7 @@ class TestReview(ReviewBase):
str(author.get_role_display()), str(author.get_role_display()),
self.addon, self.addon,
) )
with self.assertNumQueries(56): with self.assertNumQueries(57):
# FIXME: obviously too high, but it's a starting point. # FIXME: obviously too high, but it's a starting point.
# Potential further optimizations: # Potential further optimizations:
# - Remove trivial... and not so trivial duplicates # - Remove trivial... and not so trivial duplicates
@ -2769,6 +2828,7 @@ class TestReview(ReviewBase):
# 54. cinder policies for the policy dropdown # 54. cinder policies for the policy dropdown
# 55. select users by role for this add-on (?) # 55. select users by role for this add-on (?)
# 56. unreviewed versions in other channel # 56. unreviewed versions in other channel
# 57. attachmentlog
response = self.client.get(self.url) response = self.client.get(self.url)
assert response.status_code == 200 assert response.status_code == 200
doc = pq(response.content) doc = pq(response.content)
@ -2951,15 +3011,27 @@ class TestReview(ReviewBase):
in doc('#versions-history .review-files .listing-header .light').text() in doc('#versions-history .review-files .listing-header .light').text()
) )
@override_switch('enable-activity-log-attachments', active=True)
def test_item_history_comment(self): def test_item_history_comment(self):
# Add Comment. # Add Comment.
self.client.post(self.url, {'action': 'comment', 'comments': 'hello sailor'}) self.client.post(self.url, {'action': 'comment', 'comments': 'hello sailor'})
# Add reply with an attachment.
self.client.post(
self.url,
{
'action': 'reply',
'comments': 'hello again sailor',
'attachment_input': 'build log',
},
)
response = self.client.get(self.url) response = self.client.get(self.url)
assert response.status_code == 200 assert response.status_code == 200
doc = pq(response.content)('#versions-history .review-files') doc = pq(response.content)('#versions-history .review-files')
assert doc('th').eq(1).text() == 'Commented' assert doc('th').eq(1).text() == 'Commented'
assert doc('.history-comment').text() == 'hello sailor' assert doc('.history-comment').eq(0).text() == 'hello sailor'
assert doc('.history-comment').eq(1).text() == 'hello again sailor'
assert len(doc('.download-reply-attachment')) == 1
def test_item_history_pending_rejection(self): def test_item_history_pending_rejection(self):
response = self.client.get(self.url) response = self.client.get(self.url)
@ -5438,7 +5510,7 @@ class TestReview(ReviewBase):
results={'matchedRules': [customs_rule.name]}, results={'matchedRules': [customs_rule.name]},
) )
with self.assertNumQueries(57): with self.assertNumQueries(58):
# See test_item_history_pagination() for more details about the # See test_item_history_pagination() for more details about the
# queries count. What's important here is that the extra versions # queries count. What's important here is that the extra versions
# and scanner results don't cause extra queries. # and scanner results don't cause extra queries.

Просмотреть файл

@ -3,6 +3,7 @@ from datetime import datetime, timedelta
from django.conf import settings from django.conf import settings
from django.contrib.humanize.templatetags.humanize import naturaltime from django.contrib.humanize.templatetags.humanize import naturaltime
from django.core.files.base import ContentFile
from django.db.models import Count, F, Q from django.db.models import Count, F, Q
from django.urls import reverse from django.urls import reverse
from django.utils.http import urlencode from django.utils.http import urlencode
@ -15,7 +16,7 @@ from olympia import amo
from olympia.abuse.models import CinderJob, CinderPolicy from olympia.abuse.models import CinderJob, CinderPolicy
from olympia.abuse.tasks import notify_addon_decision_to_cinder, resolve_job_in_cinder from olympia.abuse.tasks import notify_addon_decision_to_cinder, resolve_job_in_cinder
from olympia.access import acl from olympia.access import acl
from olympia.activity.models import ActivityLog from olympia.activity.models import ActivityLog, AttachmentLog
from olympia.activity.utils import notify_about_activity_log from olympia.activity.utils import notify_about_activity_log
from olympia.addons.models import Addon, AddonApprovalsCounter, AddonReviewerFlags from olympia.addons.models import Addon, AddonApprovalsCounter, AddonReviewerFlags
from olympia.constants.abuse import DECISION_ACTIONS from olympia.constants.abuse import DECISION_ACTIONS
@ -1017,6 +1018,17 @@ class ReviewBase:
kwargs = {'user': user or self.user, 'created': timestamp, 'details': details} kwargs = {'user': user or self.user, 'created': timestamp, 'details': details}
self.log_entry = ActivityLog.objects.create(action, *args, **kwargs) self.log_entry = ActivityLog.objects.create(action, *args, **kwargs)
attachment = None
if self.data.get('attachment_file'):
attachment = self.data.get('attachment_file')
elif self.data.get('attachment_input'):
# The name will be overridden later by attachment_upload_path.
attachment = ContentFile(
self.data['attachment_input'], name='attachment.txt'
)
if attachment is not None:
AttachmentLog.objects.create(activity_log=self.log_entry, file=attachment)
def reviewer_reply(self): def reviewer_reply(self):
# Default to reviewer reply action. # Default to reviewer reply action.
action = amo.LOG.REVIEWER_REPLY_VERSION action = amo.LOG.REVIEWER_REPLY_VERSION

Просмотреть файл

@ -527,7 +527,9 @@ def review(request, addon, channel=None):
human_review=True, human_review=True,
) )
form = ReviewForm( form = ReviewForm(
request.POST if request.method == 'POST' else None, helper=form_helper request.POST if request.method == 'POST' else None,
request.FILES if request.method == 'POST' else None,
helper=form_helper,
) )
reports = Paginator(AbuseReport.objects.for_addon(addon), 5).page(1) reports = Paginator(AbuseReport.objects.for_addon(addon), 5).page(1)

Просмотреть файл

@ -43,6 +43,8 @@ urlpatterns = [
re_path(r'^uploads/', include(upload_patterns)), re_path(r'^uploads/', include(upload_patterns)),
# Downloads. # Downloads.
re_path(r'^downloads/', include(download_patterns)), re_path(r'^downloads/', include(download_patterns)),
# Activity.
re_path(r'activity/', include('olympia.activity.urls')),
# Users # Users
re_path(r'', include('olympia.users.urls')), re_path(r'', include('olympia.users.urls')),
# Developer Hub. # Developer Hub.

Просмотреть файл

@ -896,6 +896,11 @@ form.review-form .data-toggle {
height: 200px; height: 200px;
} }
#review-actions-form #id_attachment_input {
height: 100px;
width: 30%;
}
#review-actions-form .review-actions-desc { #review-actions-form .review-actions-desc {
text-align: center; text-align: center;
} }
@ -1406,3 +1411,12 @@ table.abuse_reports {
margin-bottom: 0; margin-bottom: 0;
} }
} }
#attachment-type-toggle, #id_attachment_file {
padding: 0.5em;
}
.review-actions-attachment-reply {
display: flex;
flex-direction: column;
}

Просмотреть файл

@ -724,6 +724,11 @@ th, td {
padding: 0.308em 0.537em 0.214em 0.231em; /* 4px 7px 3px 7px */ padding: 0.308em 0.537em 0.214em 0.231em; /* 4px 7px 3px 7px */
} }
div:has(> .download-reply-attachment) {
display: flex;
flex-direction: column;
text-align: right;
}
/** Button =Popups *********/ /** Button =Popups *********/
.install { .install {

Просмотреть файл

@ -301,6 +301,20 @@ function initExtraReviewActions() {
}), }),
); );
$('#toggle_attachment_file').on('click', function (e) {
e.preventDefault();
$('#attachment-type-toggle').addClass('hidden');
$('#attachment_input_wrapper').addClass('hidden');
$('#attachment_file_wrapper').removeClass('hidden');
});
$('#toggle_attachment_input').on('click', function (e) {
e.preventDefault();
$('#attachment-type-toggle').addClass('hidden');
$('#attachment_file_wrapper').addClass('hidden');
$('#attachment_input_wrapper').removeClass('hidden');
});
// One-off-style buttons. // One-off-style buttons.
$('.more-actions button.oneoff[data-api-url]').click( $('.more-actions button.oneoff[data-api-url]').click(
_pd(function () { _pd(function () {