split appeal job action from abuse job action (#22406)

* split appeal job action from abuse job action

* don't localise hidden activity log format labels
This commit is contained in:
Andrew Williamson 2024-06-25 22:56:51 +01:00 коммит произвёл GitHub
Родитель e7263ae0a6
Коммит f85937b394
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
11 изменённых файлов: 489 добавлений и 96 удалений

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

@ -1041,8 +1041,18 @@ class AUTO_REJECT_CONTENT_AFTER_DELAY_EXPIRED(_LOG):
class RESOLVE_CINDER_JOB_WITH_NO_ACTION(_LOG):
id = 191
format = '{addon} cinder job resolved with no action .'
short = _('Job Resolved as Ignore/Approve')
format = '{addon} abuse report job resolved with no action.'
short = 'Reports resolved as Ignore/Approve'
keep = True
review_queue = True
hide_developer = True
reviewer_review_action = True
class DENY_APPEAL_JOB(_LOG):
id = 192
format = '{addon} appeal job denied.'
short = 'Appeal denied'
keep = True
review_queue = True
hide_developer = True

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

@ -177,10 +177,10 @@ class VersionsChoiceWidget(forms.SelectMultiple):
return option
class ReasonsChoiceField(ModelMultipleChoiceField):
class WidgetRenderedModelMultipleChoiceField(ModelMultipleChoiceField):
"""
Widget to use together with ReasonsChoiceWidget to display checkboxes
with extra data for the canned responses.
Field to use together with a suitable Widget subclass to pass down the object to be
rendered.
"""
def label_from_instance(self, obj):
@ -191,14 +191,14 @@ class ReasonsChoiceField(ModelMultipleChoiceField):
class ReasonsChoiceWidget(forms.CheckboxSelectMultiple):
"""
Widget to use together with ReasonsChoiceField to display checkboxes
with extra data for the canned responses.
Widget to use together with a WidgetRenderedModelMultipleChoiceField to display
checkboxes with extra data for the canned responses.
"""
def create_option(self, *args, **kwargs):
option = super().create_option(*args, **kwargs)
# label_from_instance() on ReasonsChoiceField returns the full object,
# not a label, this is what makes this work.
# label_from_instance() on WidgetRenderedModelMultipleChoiceField returns the
# full object, not a label, this is what makes this work.
obj = option['label']
canned_response = (
obj.cinder_policy.full_text(obj.canned_response)
@ -210,11 +210,24 @@ class ReasonsChoiceWidget(forms.CheckboxSelectMultiple):
return option
class CinderJobChoiceField(ModelMultipleChoiceField):
def label_from_instance(self, obj):
class CinderJobsWidget(forms.CheckboxSelectMultiple):
"""
Widget to use together with a WidgetRenderedModelMultipleChoiceField to display
select elements with additional attribute to allow toggling.
"""
option_template_name = 'reviewers/includes/input_option_with_label_attrs.html'
def create_option(
self, name, value, label, selected, index, subindex=None, attrs=None
):
# label_from_instance() on WidgetRenderedModelMultipleChoiceField returns the
# full object, not a label, this is what makes this work.
obj = label
is_escalation = (
obj.decision and obj.decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON
)
is_appeal = obj.is_appeal
reports = obj.all_abuse_reports
reasons_set = {
(report.REASONS.for_value(report.reason).display,) for report in reports
@ -227,10 +240,10 @@ class CinderJobChoiceField(ModelMultipleChoiceField):
for report in reports
)
subtext = f'Reasoning: {obj.decision.notes}' if is_escalation else ''
return format_html(
label = format_html(
'{}{}{}<details><summary>Show detail on {} reports</summary>'
'<span>{}</span><ul>{}</ul></details>',
'[Appeal] ' if obj.is_appeal else '',
'[Appeal] ' if is_appeal else '',
'[Escalation] ' if is_escalation else '',
format_html_join(', ', '"{}"', reasons_set),
len(reports),
@ -238,6 +251,14 @@ class CinderJobChoiceField(ModelMultipleChoiceField):
format_html_join('', '<li>{}{}</li>', messages_gen),
)
attrs = attrs or {}
attrs['data-value'] = (
'resolve_appeal_job' if not is_appeal else 'resolve_reports_job'
)
return super().create_option(
name, value, label, selected, index, subindex, attrs
)
class ActionChoiceWidget(forms.RadioSelect):
"""
@ -313,7 +334,7 @@ class ReviewForm(forms.Form):
min_value=1,
max_value=99,
)
reasons = ReasonsChoiceField(
reasons = WidgetRenderedModelMultipleChoiceField(
label='Choose one or more reasons:',
# queryset is set later in __init__.
queryset=ReviewActionReason.objects.none(),
@ -321,17 +342,25 @@ class ReviewForm(forms.Form):
widget=ReasonsChoiceWidget,
)
version_pk = forms.IntegerField(required=False, min_value=1)
resolve_cinder_jobs = CinderJobChoiceField(
cinder_jobs_to_resolve = WidgetRenderedModelMultipleChoiceField(
label='Outstanding DSA related reports to resolve:',
required=False,
queryset=CinderJob.objects.none(),
widget=forms.CheckboxSelectMultiple,
widget=CinderJobsWidget(attrs={'class': 'data-toggle-hide'}),
)
cinder_policies = forms.ModelMultipleChoiceField(
# queryset is set later in __init__
queryset=CinderPolicy.objects.none(),
required=False,
label='Choose one or more policies:',
widget=widgets.CheckboxSelectMultiple,
)
appeal_action = forms.MultipleChoiceField(
required=False,
label='Choose how to resolve appeal:',
choices=(('deny', 'Deny Appeal(s)'),),
widget=widgets.CheckboxSelectMultiple,
)
def is_valid(self):
@ -344,12 +373,14 @@ class ReviewForm(forms.Form):
self.fields['versions'].required = True
if not action.get('requires_reasons', False):
self.fields['reasons'].required = False
if self.data.get('resolve_cinder_jobs'):
if self.data.get('cinder_jobs_to_resolve'):
# if a cinder job is being resolved we need a review reason or policy
if action.get('allows_reasons'):
self.fields['reasons'].required = True
if action.get('allows_policies'):
self.fields['cinder_policies'].required = True
if self.data.get('action') == 'resolve_appeal_job':
self.fields['appeal_action'].required = True
result = super().is_valid()
if result:
self.helper.set_data(self.cleaned_data)
@ -357,7 +388,22 @@ class ReviewForm(forms.Form):
def clean(self):
super().clean()
if self.cleaned_data.get('resolve_cinder_jobs') and self.cleaned_data.get(
# 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('action') == 'resolve_appeal_job':
self.cleaned_data['cinder_jobs_to_resolve'] = [
job
for job in self.cleaned_data.get('cinder_jobs_to_resolve', ())
if job.is_appeal
]
elif self.cleaned_data.get('action') == 'resolve_reports_job':
self.cleaned_data['cinder_jobs_to_resolve'] = [
job
for job in self.cleaned_data.get('cinder_jobs_to_resolve', ())
if not job.is_appeal
]
if self.cleaned_data.get('cinder_jobs_to_resolve') and self.cleaned_data.get(
'cinder_policies'
):
actions = self.helper.handler.get_cinder_actions_from_policies(
@ -449,12 +495,10 @@ class ReviewForm(forms.Form):
self.fields['action'].widget.actions = self.helper.actions
# Set the queryset for cinderjobs to resolve
self.fields['resolve_cinder_jobs'].queryset = (
CinderJob.objects.for_addon(self.helper.addon)
.unresolved()
.resolvable_in_reviewer_tools()
.prefetch_related('abusereport_set', 'appealed_decisions__cinder_job')
)
self.fields[
'cinder_jobs_to_resolve'
].queryset = self.helper.unresolved_cinderjob_qs
# Set the queryset for policies to show as options
self.fields['cinder_policies'].queryset = CinderPolicy.objects.filter(
expose_in_reviewer_tools=True

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

@ -85,7 +85,7 @@ class Command(BaseCommand):
).distinct()
helper.handler.data = {
'comments': log_details.get('comments', ''),
'resolve_cinder_jobs': cinder_jobs,
'cinder_jobs_to_resolve': cinder_jobs,
'versions': versions,
}
helper.handler.reject_multiple_versions()

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

@ -0,0 +1 @@
<label{% if widget.attrs.id %} for="{{ widget.attrs.id }}"{% endif %} {% include "django/forms/widgets/attrs.html" %}>{% include "django/forms/widgets/input.html" %} {{ widget.label }}</label>

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

@ -180,28 +180,38 @@
<div class="review-actions-reasons-select">
{{ form.reasons }}
</div>
<div class="review-actions-reasons-error">
{{ form.reasons.errors }}
</div>
{% if form.reasons.errors %}
<div class="review-actions-reasons-error">{{ form.reasons.errors }}</div>
{% endif %}
</div>
<div class="data-toggle review-actions-policies"
data-value="{{ actions_policies|join(' ') }}">
<label for="id_cinder_policies">{{ form.cinder_policies.label }}</label>
<div class="review-actions-policies-select">
{{ form.cinder_policies }}
data-value="{{ actions_policies|join(' ') }}">
<label for="id_cinder_policies">{{ form.cinder_policies.label }}</label>
<div class="review-actions-policies-select">
{{ form.cinder_policies }}
</div>
<div>
{{ form.cinder_policies.errors }}
</div>
</div>
<div>
{{ form.cinder_policies.errors }}
<div class="data-toggle"
data-value="resolve_appeal_job">
<label for="id_appeal_action">{{ form.appeal_action.label }}</label>
<div class="review-actions-policies-select">
{{ form.appeal_action }}
</div>
<div>
{{ form.appeal_action.errors }}
</div>
</div>
</div>
</div>
</div>
<div class="review-actions-section data-toggle review-resolve-abuse-reports"
data-value="{{ actions_resolves_abuse_reports|join(' ') }}">
<strong><label for="id_resolve_cinder_jobs">{{ form.resolve_cinder_jobs.label }}</label></strong>
{{ form.resolve_cinder_jobs }}
{{ form.resolve_cinder_jobs.errors }}
<strong><label for="id_cinder_jobs_to_resolve">{{ form.cinder_jobs_to_resolve.label }}</label></strong>
{{ form.cinder_jobs_to_resolve }}
{{ form.cinder_jobs_to_resolve.errors }}
</div>
<div class="review-actions-section review-actions-files data-toggle review-files"

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

@ -356,7 +356,7 @@ class TestReviewForm(TestCase):
reason = ReviewActionReason.objects.create(name='A reason', canned_response='a')
form = self.get_form()
assert not form.is_bound
data = {'action': 'public', 'comments': 'lol', 'resolve_cinder_jobs': [job]}
data = {'action': 'public', 'comments': 'lol', 'cinder_jobs_to_resolve': [job]}
form = self.get_form(data=data)
assert form.is_bound
assert not form.is_valid()
@ -384,9 +384,8 @@ class TestReviewForm(TestCase):
form = self.get_form()
assert not form.is_bound
data = {
'action': 'resolve_job',
'comments': 'lol',
'resolve_cinder_jobs': [job.id],
'action': 'resolve_reports_job',
'cinder_jobs_to_resolve': [job.id],
}
form = self.get_form(data=data)
assert form.is_bound
@ -399,6 +398,34 @@ class TestReviewForm(TestCase):
assert form.is_valid(), form.errors
assert not form.errors
def test_appeal_action_require_with_resolve_appeal_job(self):
self.grant_permission(self.request.user, 'Addons:Review')
self.addon.update(status=amo.STATUS_NOMINATED)
self.version.file.update(status=amo.STATUS_AWAITING_REVIEW)
job = CinderJob.objects.create(
job_id='1', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=job, addon=self.addon, action=DECISION_ACTIONS.AMO_DISABLE_ADDON
)
form = self.get_form()
assert not form.is_bound
data = {
'action': 'resolve_appeal_job',
'comments': 'lol',
'cinder_jobs_to_resolve': [job.id],
}
form = self.get_form(data=data)
assert form.is_bound
assert not form.is_valid()
assert form.errors == {'appeal_action': ['This field is required.']}
data['appeal_action'] = ['deny']
form = self.get_form(data=data)
assert form.is_bound
assert form.is_valid(), form.errors
assert not form.errors
def test_only_one_cinder_action_selected(self):
self.grant_permission(self.request.user, 'Addons:Review')
self.addon.update(status=amo.STATUS_NOMINATED)
@ -430,9 +457,8 @@ class TestReviewForm(TestCase):
form = self.get_form()
assert not form.is_bound
data = {
'action': 'resolve_job',
'comments': 'lol',
'resolve_cinder_jobs': [job.id],
'action': 'resolve_reports_job',
'cinder_jobs_to_resolve': [job.id],
'cinder_policies': [no_action_policy.id],
}
form = self.get_form(data=data)
@ -453,6 +479,58 @@ class TestReviewForm(TestCase):
assert form.is_valid()
assert not form.errors
def test_cinder_jobs_filtered_for_resolve_reports_job_and_resolve_appeal_job(self):
self.grant_permission(self.request.user, 'Addons:Review')
self.addon.update(status=amo.STATUS_NOMINATED)
self.version.file.update(status=amo.STATUS_AWAITING_REVIEW)
appeal_job = CinderJob.objects.create(
job_id='1', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=appeal_job,
addon=self.addon,
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
)
report_job = CinderJob.objects.create(
job_id='2', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
AbuseReport.objects.create(cinder_job=report_job, guid=self.addon.guid)
policy = CinderPolicy.objects.create(
uuid='a',
name='ignore',
expose_in_reviewer_tools=True,
default_cinder_action=DECISION_ACTIONS.AMO_IGNORE,
)
data = {
'action': 'resolve_appeal_job',
'comments': 'lol',
'appeal_action': ['deny'],
'cinder_jobs_to_resolve': [report_job.id],
}
form = self.get_form(data=data)
form.is_valid()
assert form.cleaned_data['cinder_jobs_to_resolve'] == []
data['cinder_jobs_to_resolve'] = [report_job, appeal_job]
form = self.get_form(data=data)
form.is_valid()
assert form.cleaned_data['cinder_jobs_to_resolve'] == [appeal_job]
data = {
'action': 'resolve_reports_job',
'cinder_policies': [policy.id],
'cinder_jobs_to_resolve': [appeal_job.id],
}
form = self.get_form(data=data)
form.is_valid()
assert form.cleaned_data['cinder_jobs_to_resolve'] == []
data['cinder_jobs_to_resolve'] = [report_job.id, appeal_job.id]
form = self.get_form(data=data)
form.is_valid()
assert form.cleaned_data['cinder_jobs_to_resolve'] == [report_job]
def test_boilerplate(self):
self.grant_permission(self.request.user, 'Addons:Review')
self.addon.update(status=amo.STATUS_NOMINATED)
@ -898,7 +976,7 @@ class TestReviewForm(TestCase):
form = self.get_form(data={**data, 'version_pk': self.version.pk})
assert form.is_valid(), form.errors
def test_resolve_cinder_jobs_choices(self):
def test_cinder_jobs_to_resolve_choices(self):
abuse_kw = {
'guid': self.addon.guid,
'location': AbuseReport.LOCATION.ADDON,
@ -977,7 +1055,7 @@ class TestReviewForm(TestCase):
)
form = self.get_form()
choices = form.fields['resolve_cinder_jobs'].choices
choices = form.fields['cinder_jobs_to_resolve'].choices
qs_list = list(choices.queryset)
assert qs_list == [
# Only unresolved, reviewer handled, jobs are shown
@ -986,9 +1064,10 @@ class TestReviewForm(TestCase):
cinder_job_2_reports,
]
content = str(form['resolve_cinder_jobs'])
content = str(form['cinder_jobs_to_resolve'])
doc = pq(content)
assert doc('label[for="id_resolve_cinder_jobs_0"]').text() == (
label_0 = doc('label[for="id_cinder_jobs_to_resolve_0"]')
assert label_0.text() == (
'[Escalation] "DSA: It violates Mozilla\'s Add-on Policies"\n'
'Show detail on 1 reports\n'
'Reasoning: Why o why\n'
@ -996,15 +1075,24 @@ class TestReviewForm(TestCase):
)
assert '<script>alert()</script>' not in content # should be escaped
assert '&lt;script&gt;alert()&lt;/script&gt' in content # should be escaped
assert doc('label[for="id_resolve_cinder_jobs_1"]').text() == (
label_1 = doc('label[for="id_cinder_jobs_to_resolve_1"]')
assert label_1.text() == (
'[Appeal] "DSA: It violates Mozilla\'s Add-on Policies"'
'\nShow detail on 1 reports\nv[1.2]: ccc'
)
assert doc('label[for="id_resolve_cinder_jobs_2"]').text() == (
label_2 = doc('label[for="id_cinder_jobs_to_resolve_2"]')
assert label_2.text() == (
'"DSA: It violates Mozilla\'s Add-on Policies"\n'
'Show detail on 2 reports\n<no message>\nbbb'
)
assert label_0.attr['class'] == 'data-toggle-hide'
assert label_0.attr['data-value'] == 'resolve_appeal_job'
assert label_1.attr['class'] == 'data-toggle-hide'
assert label_1.attr['data-value'] == 'resolve_reports_job'
assert label_2.attr['class'] == 'data-toggle-hide'
assert label_2.attr['data-value'] == 'resolve_appeal_job'
def test_cinder_policies_choices(self):
policy_exposed = CinderPolicy.objects.create(
uuid='1', name='foo', expose_in_reviewer_tools=True

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

@ -949,16 +949,34 @@ class TestReviewHelper(TestReviewHelperBase):
)
assert expected == actions
def test_actions_resolve_cinder_jobs(self):
def test_actions_cinder_jobs_to_resolve(self):
self.grant_permission(self.user, 'Addons:Review')
CinderJob.objects.create(
job = CinderJob.objects.create(
target_addon=self.addon, resolvable_in_reviewer_tools=True
)
expected = [
'reject_multiple_versions',
'set_needs_human_review_multiple_versions',
'reply',
'resolve_job',
'resolve_reports_job',
'comment',
]
actions = list(
self.get_review_actions(
addon_status=amo.STATUS_APPROVED,
file_status=amo.STATUS_APPROVED,
).keys()
)
assert expected == actions
CinderDecision.objects.create(
action=DECISION_ACTIONS.AMO_DISABLE_ADDON, addon=self.addon, appeal_job=job
)
expected = [
'reject_multiple_versions',
'set_needs_human_review_multiple_versions',
'reply',
'resolve_appeal_job',
'comment',
]
actions = list(
@ -1049,10 +1067,12 @@ class TestReviewHelper(TestReviewHelperBase):
uuid='z', default_cinder_action=DECISION_ACTIONS.AMO_IGNORE
),
],
'resolve_cinder_jobs': [cinder_job],
'cinder_jobs_to_resolve': [cinder_job],
}
self.helper.set_data(data)
self.helper.handler.review_action = self.helper.actions.get('resolve_job')
self.helper.handler.review_action = self.helper.actions.get(
'resolve_reports_job'
)
self.helper.handler.log_action(amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION)
assert ReviewActionReasonLog.objects.count() == 0
assert CinderPolicyLog.objects.count() == 2
@ -1063,6 +1083,71 @@ class TestReviewHelper(TestReviewHelperBase):
== 'AMO_IGNORE'
)
def test_log_action_override_policies(self):
self.grant_permission(self.user, 'Addons:Review')
cinder_job = CinderJob.objects.create(
target_addon=self.addon, resolvable_in_reviewer_tools=True
)
other_policy = CinderPolicy.objects.create(
uuid='y', default_cinder_action=DECISION_ACTIONS.AMO_DISABLE_ADDON
)
self.helper = self.get_helper()
data = {
'cinder_policies': [
CinderPolicy.objects.create(
uuid='z', default_cinder_action=DECISION_ACTIONS.AMO_IGNORE
),
],
'cinder_jobs_to_resolve': [cinder_job],
}
self.helper.set_data(data)
self.helper.handler.review_action = self.helper.actions.get(
'resolve_reports_job'
)
self.helper.handler.log_action(
amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION, policies=[other_policy]
)
assert ReviewActionReasonLog.objects.count() == 0
assert CinderPolicyLog.objects.count() == 1
assert CinderPolicyLog.objects.first().cinder_policy == other_policy
assert (
ActivityLog.objects.get(
action=amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION.id
).details['cinder_action']
== 'AMO_DISABLE_ADDON' # from other_policy
)
def test_log_action_override_action(self):
self.grant_permission(self.user, 'Addons:Review')
cinder_job = CinderJob.objects.create(
target_addon=self.addon, resolvable_in_reviewer_tools=True
)
self.helper = self.get_helper()
data = {
'cinder_policies': [
CinderPolicy.objects.create(
uuid='z', default_cinder_action=DECISION_ACTIONS.AMO_IGNORE
),
],
'cinder_jobs_to_resolve': [cinder_job],
}
self.helper.set_data(data)
self.helper.handler.review_action = self.helper.actions.get(
'resolve_reports_job'
)
self.helper.handler.log_action(
amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION,
cinder_action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
)
assert ReviewActionReasonLog.objects.count() == 0
assert CinderPolicyLog.objects.count() == 1
assert (
ActivityLog.objects.get(
action=amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION.id
).details['cinder_action']
== 'AMO_DISABLE_ADDON'
)
def test_log_action_override_user(self):
# ActivityLog.user will default to self.user in log_action.
self.helper.set_data(self.get_data())
@ -1089,16 +1174,16 @@ class TestReviewHelper(TestReviewHelperBase):
cinder_job1 = CinderJob.objects.create(job_id='1')
cinder_job2 = CinderJob.objects.create(job_id='2')
# without 'resolve_cinder_jobs', notify_addon_decision_to_cinder is called
# without 'cinder_jobs_to_resolve', notify_addon_decision_to_cinder is called
self.helper.set_data(self.get_data())
self.helper.handler.notify_decision()
mock_report.assert_called_once_with(
addon_id=self.addon.id, log_entry_id=log_entry.id
)
# with 'resolve_cinder_jobs', resolve_job_in_cinder is called instead
# with 'cinder_jobs_to_resolve', resolve_job_in_cinder is called instead
self.helper.set_data(
{**self.get_data(), 'resolve_cinder_jobs': [cinder_job1, cinder_job2]}
{**self.get_data(), 'cinder_jobs_to_resolve': [cinder_job1, cinder_job2]}
)
self.helper.handler.notify_decision()
@ -2249,7 +2334,7 @@ class TestReviewHelper(TestReviewHelperBase):
f'{settings.CINDER_SERVER_URL}jobs/1/decision',
callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})),
)
self._test_reject_multiple_versions({'resolve_cinder_jobs': [cinder_job]})
self._test_reject_multiple_versions({'cinder_jobs_to_resolve': [cinder_job]})
message = mail.outbox[0]
self.check_subject(message)
assert 'Extension Delicious Bookmarks was manually reviewed' in message.body
@ -2345,7 +2430,7 @@ class TestReviewHelper(TestReviewHelperBase):
callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})),
)
self._test_reject_multiple_versions_with_delay(
{'resolve_cinder_jobs': [cinder_job]}
{'cinder_jobs_to_resolve': [cinder_job]}
)
message = mail.outbox[0]
self.check_subject(message)
@ -3299,7 +3384,7 @@ class TestReviewHelper(TestReviewHelperBase):
self.helper.handler.data = {
'versions': [self.review_version],
'resolve_cinder_jobs': [CinderJob()],
'cinder_jobs_to_resolve': [CinderJob()],
}
resolves_actions = {
key: action
@ -3366,7 +3451,7 @@ class TestReviewHelper(TestReviewHelperBase):
'should_email': True,
'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON,
},
'resolve_job': {'should_email': False, 'cinder_action': None},
'resolve_reports_job': {'should_email': False, 'cinder_action': None},
}
)
self.setup_data(amo.STATUS_APPROVED, file_status=amo.STATUS_DISABLED)
@ -3385,7 +3470,7 @@ class TestReviewHelper(TestReviewHelperBase):
'should_email': True,
'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON,
},
'resolve_job': {'should_email': False, 'cinder_action': None},
'resolve_reports_job': {'should_email': False, 'cinder_action': None},
}
)
self.setup_data(amo.STATUS_DISABLED, file_status=amo.STATUS_DISABLED)
@ -3399,7 +3484,7 @@ class TestReviewHelper(TestReviewHelperBase):
'should_email': True,
'cinder_action': DECISION_ACTIONS.AMO_APPROVE_VERSION,
},
'resolve_job': {'should_email': False, 'cinder_action': None},
'resolve_reports_job': {'should_email': False, 'cinder_action': None},
}
)
@ -3436,7 +3521,7 @@ class TestReviewHelper(TestReviewHelperBase):
'should_email': True,
'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON,
},
'resolve_job': {'should_email': False, 'cinder_action': None},
'resolve_reports_job': {'should_email': False, 'cinder_action': None},
}
)
self.setup_data(amo.STATUS_DISABLED, file_status=amo.STATUS_DISABLED)
@ -3458,10 +3543,74 @@ class TestReviewHelper(TestReviewHelperBase):
'should_email': True,
'cinder_action': DECISION_ACTIONS.AMO_APPROVE_VERSION,
},
'resolve_job': {'should_email': False, 'cinder_action': None},
'resolve_reports_job': {'should_email': False, 'cinder_action': None},
}
)
def test_resolve_appeal_job(self):
policy_a = CinderPolicy.objects.create(uuid='a')
policy_b = CinderPolicy.objects.create(uuid='b')
policy_c = CinderPolicy.objects.create(uuid='c')
policy_d = CinderPolicy.objects.create(uuid='d')
appeal_job1 = CinderJob.objects.create(
job_id='1', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=appeal_job1,
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
addon=self.addon,
).policies.add(policy_a, policy_b)
CinderDecision.objects.create(
appeal_job=appeal_job1,
action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
addon=self.addon,
).policies.add(policy_a, policy_c)
responses.add_callback(
responses.POST,
f'{settings.CINDER_SERVER_URL}jobs/{appeal_job1.job_id}/decision',
callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})),
)
appeal_job2 = CinderJob.objects.create(
job_id='2', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=appeal_job2,
action=DECISION_ACTIONS.AMO_APPROVE,
addon=self.addon,
).policies.add(policy_d)
responses.add_callback(
responses.POST,
f'{settings.CINDER_SERVER_URL}jobs/{appeal_job2.job_id}/decision',
callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})),
)
self.grant_permission(self.user, 'Addons:Review')
self.file.update(status=amo.STATUS_AWAITING_REVIEW)
self.helper = self.get_helper()
data = {
'comments': 'Nope',
'cinder_jobs_to_resolve': [appeal_job1, appeal_job2],
'appeal_action': ['deny'],
}
self.helper.set_data(data)
self.helper.handler.resolve_appeal_job()
assert CinderPolicyLog.objects.count() == 4
activity_log_qs = ActivityLog.objects.filter(action=amo.LOG.DENY_APPEAL_JOB.id)
assert activity_log_qs.count() == 2
log2, log1 = list(activity_log_qs.all())
assert log1.details['cinder_action'] == 'AMO_DISABLE_ADDON'
assert log2.details['cinder_action'] == 'AMO_APPROVE'
assert set(appeal_job1.reload().decision.policies.all()) == {
policy_a,
policy_b,
policy_c,
}
assert set(appeal_job2.reload().decision.policies.all()) == {policy_d}
@override_settings(ENABLE_ADDON_SIGNING=True)
class TestReviewHelperSigning(TestReviewHelperBase):

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

@ -25,7 +25,7 @@ from rest_framework.test import APIRequestFactory
from waffle.testutils import override_switch
from olympia import amo, core, ratings
from olympia.abuse.models import AbuseReport, CinderJob, CinderPolicy
from olympia.abuse.models import AbuseReport, CinderDecision, CinderJob, CinderPolicy
from olympia.access import acl
from olympia.access.models import Group, GroupUser
from olympia.accounts.serializers import BaseUserSerializer
@ -2515,7 +2515,7 @@ class TestReview(ReviewBase):
assert ActivityLog.objects.filter(action=comment_version.id).count() == 1
@mock.patch('olympia.reviewers.utils.resolve_job_in_cinder.delay')
def test_resolve_cinder_job(self, resolve_mock):
def test_resolve_reports_job(self, resolve_mock):
cinder_job = CinderJob.objects.create(
job_id='123', target_addon=self.addon, resolvable_in_reviewer_tools=True
)
@ -2534,8 +2534,8 @@ class TestReview(ReviewBase):
response = self.client.post(
self.url,
{
'action': 'resolve_job',
'resolve_cinder_jobs': [cinder_job.id],
'action': 'resolve_reports_job',
'cinder_jobs_to_resolve': [cinder_job.id],
'cinder_policies': [policy.id],
},
)
@ -2548,6 +2548,49 @@ class TestReview(ReviewBase):
assert activity_log_qs[0].details['cinder_action'] == 'AMO_IGNORE'
resolve_mock.assert_called_once()
@mock.patch('olympia.reviewers.utils.resolve_job_in_cinder.delay')
def test_resolve_appeal_job(self, resolve_mock):
appeal_job1 = CinderJob.objects.create(
job_id='1', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=appeal_job1,
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
addon=self.addon,
)
CinderDecision.objects.create(
appeal_job=appeal_job1,
action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
addon=self.addon,
)
appeal_job2 = CinderJob.objects.create(
job_id='2', resolvable_in_reviewer_tools=True, target_addon=self.addon
)
CinderDecision.objects.create(
appeal_job=appeal_job2,
action=DECISION_ACTIONS.AMO_APPROVE,
addon=self.addon,
)
response = self.client.post(
self.url,
{
'action': 'resolve_appeal_job',
'comments': 'Nope',
'cinder_jobs_to_resolve': [appeal_job1.id, appeal_job2.id],
'appeal_action': ['deny'],
},
)
assert response.status_code == 302
activity_log_qs = ActivityLog.objects.filter(action=amo.LOG.DENY_APPEAL_JOB.id)
assert activity_log_qs.count() == 2
log1, log2 = list(activity_log_qs.all())
assert log1.details['cinder_action'] == 'AMO_DISABLE_ADDON'
assert log2.details['cinder_action'] == 'AMO_APPROVE'
assert resolve_mock.call_count == 2
def test_reviewer_reply(self):
reason = ReviewActionReason.objects.create(
name='reason 1', is_active=True, canned_response='reason'
@ -5634,7 +5677,7 @@ class TestReview(ReviewBase):
self.get_dict(
action='disable_addon',
reasons=[reason.id],
resolve_cinder_jobs=[cinder_job.id],
cinder_jobs_to_resolve=[cinder_job.id],
),
)
assert self.get_addon().status == amo.STATUS_DISABLED
@ -5679,7 +5722,7 @@ class TestReview(ReviewBase):
self.get_dict(
action='public',
reasons=[reason.id],
resolve_cinder_jobs=[cinder_job.id],
cinder_jobs_to_resolve=[cinder_job.id],
),
)

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

@ -12,7 +12,7 @@ import markupsafe
import olympia.core.logger
from olympia import amo
from olympia.abuse.models import CinderJob
from olympia.abuse.models import CinderJob, CinderPolicy
from olympia.abuse.tasks import notify_addon_decision_to_cinder, resolve_job_in_cinder
from olympia.access import acl
from olympia.activity.models import ActivityLog
@ -537,11 +537,18 @@ class ReviewHelper:
)
version_is_blocked = self.version and self.version.is_blocked
has_unresolved_abuse_report_jobs = (
self.unresolved_cinderjob_qs = (
CinderJob.objects.for_addon(self.addon)
.unresolved()
.resolvable_in_reviewer_tools()
.exists()
.prefetch_related('abusereport_set', 'appealed_decisions__cinder_job')
)
unresolved_cinder_jobs = list(self.unresolved_cinderjob_qs)
has_unresolved_abuse_report_jobs = any(
job for job in unresolved_cinder_jobs if not job.is_appeal
)
has_unresolved_appeal_jobs = any(
job for job in unresolved_cinder_jobs if job.is_appeal
)
# Special logic for availability of reject/approve multiple action:
@ -836,9 +843,9 @@ class ReviewHelper:
'requires_reasons': not is_static_theme,
'resolves_abuse_reports': True,
}
actions['resolve_job'] = {
'method': self.handler.resolve_job,
'label': 'Resolve Jobs',
actions['resolve_reports_job'] = {
'method': self.handler.resolve_reports_job,
'label': 'Resolve Reports',
'details': (
'Allows abuse report jobs to be resovled without an action on the '
'add-on or versions.'
@ -849,6 +856,18 @@ class ReviewHelper:
'resolves_abuse_reports': True,
'allows_policies': True,
}
actions['resolve_appeal_job'] = {
'method': self.handler.resolve_appeal_job,
'label': 'Resolve Appeals',
'details': (
'Allows abuse report jobs to be resovled without an action on the '
'add-on or versions.'
),
'minimal': True,
'available': is_reviewer and has_unresolved_appeal_jobs,
'comments': True,
'resolves_abuse_reports': True,
}
actions['comment'] = {
'method': self.handler.process_comment,
'label': 'Comment',
@ -943,7 +962,7 @@ class ReviewBase:
self.addon.promotedaddon.approve_for_version(version)
def notify_decision(self):
if cinder_jobs := self.data.get('resolve_cinder_jobs', ()):
if cinder_jobs := self.data.get('cinder_jobs_to_resolve', ()):
# with appeals and escalations there could be multiple jobs
for cinder_job in cinder_jobs:
resolve_job_in_cinder.delay(
@ -1017,29 +1036,31 @@ class ReviewBase:
timestamp=None,
user=None,
extra_details=None,
policies=None,
cinder_action=None,
):
cinder_action = getattr(action, 'cinder_action', None)
if self.review_action and self.review_action.get('allows_reasons'):
reasons = self.data.get('reasons', [])
reasons = (
self.data.get('reasons', [])
if self.review_action and self.review_action.get('allows_reasons')
else []
)
if policies is None:
policies = [
reason.cinder_policy
for reason in reasons
if getattr(reason, 'cinder_policy', None)
]
else:
reasons = []
policies = []
if self.review_action and self.review_action.get('allows_policies'):
policies.extend(self.data.get('cinder_policies', []))
if self.review_action and self.review_action.get('allows_policies'):
policies.extend(self.data.get('cinder_policies', []))
cinder_action = cinder_action or getattr(action, 'cinder_action', None)
if not cinder_action and policies:
cinder_action = (
# If there isn't a cinder_action from the activity action already, get
# it from the policy. There should only be one in the list as form
# validation raises for multiple cinder actions.
cinder_action
or (
(actions := self.get_cinder_actions_from_policies(policies))
and actions[0]
)
(actions := self.get_cinder_actions_from_policies(policies))
and actions[0]
)
details = {
@ -1097,11 +1118,33 @@ class ReviewBase:
def process_comment(self):
self.log_action(amo.LOG.COMMENT_VERSION)
def resolve_job(self):
if self.data.get('resolve_cinder_jobs', ()):
def resolve_reports_job(self):
if self.data.get('cinder_jobs_to_resolve', ()):
self.log_action(amo.LOG.RESOLVE_CINDER_JOB_WITH_NO_ACTION)
self.notify_decision() # notify cinder
def resolve_appeal_job(self):
# It's possible to have multiple appeal jobs, so handle them seperately.
for job in self.data.get('cinder_jobs_to_resolve', ()):
# collect all the policies we made decisions under
previous_policies = CinderPolicy.objects.filter(
cinderdecision__appeal_job=job
).distinct()
# we just need a single action for this appeal
# - use min() to favor AMO_DISABLE_ADDON over AMO_REJECT_VERSION_ADDON
previous_action_id = min(
decision.action for decision in job.appealed_decisions.all()
)
self.log_action(
amo.LOG.DENY_APPEAL_JOB,
policies=list(previous_policies),
cinder_action=DECISION_ACTIONS.for_value(previous_action_id),
)
# notify cinder
resolve_job_in_cinder.delay(
cinder_job_id=job.id, log_entry_id=self.log_entry.id
)
def approve_latest_version(self):
"""Approve the add-on latest version (potentially setting the add-on to
approved if it was awaiting its first review)."""

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

@ -1050,7 +1050,7 @@ form.review-form .data-toggle {
list-style: none;
}
.review-actions-section #id_resolve_cinder_jobs {
.review-actions-section #id_cinder_jobs_to_resolve {
details {
margin-left: 2em;
}

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

@ -63,6 +63,7 @@ function initReviewActions() {
var $element = $(element),
value = $element.find('input').val(),
$data_toggle = $('form.review-form').find('.data-toggle'),
$data_toggle_hide = $('form.review-form').find('.data-toggle-hide'),
$comments = $('#id_comments'),
boilerplate_text = $element.find('input').attr('data-value');
@ -87,6 +88,10 @@ function initReviewActions() {
// Hide everything, then show the ones containing the value we're interested in.
$data_toggle.hide();
$data_toggle.filter('[data-value~="' + value + '"]').show();
// For data_toggle_hide, the opposite - show everything, then hide the ones containing
// the value we're interested in.
$data_toggle_hide.show();
$data_toggle_hide.filter('[data-value~="' + value + '"]').hide();
}
$('#review-actions .action_nav #id_action > *:not(.disabled)').click(