Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add basic page to proceed/escalate held actions #22822

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/olympia/abuse/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def notify_owners(self, *, log_entry_id=None, extra_context=None):
'reference_id': reference_id,
'target': self.target,
'target_url': target_url,
'type': self.decision.get_target_type(),
'type': self.decision.get_target_display(),
'SITE_URL': settings.SITE_URL,
**(extra_context or {}),
}
Expand Down Expand Up @@ -189,7 +189,7 @@ def notify_reporters(self, *, reporter_abuse_reports, is_appeal=False):
'policy_document_url': POLICY_DOCUMENT_URL,
'reference_id': reference_id,
'target_url': absolutify(self.target.get_url_path()),
'type': self.decision.get_target_type(),
'type': self.decision.get_target_display(),
'SITE_URL': settings.SITE_URL,
}
if is_appeal:
Expand Down
38 changes: 15 additions & 23 deletions src/olympia/abuse/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def process_decision(
uuid__in=policy_ids
).without_parents_if_their_children_are_present()
cinder_decision.policies.add(*policies)
cinder_decision.process_action(overridden_action)
cinder_decision.process_action(overridden_action=overridden_action)

def process_queue_move(self, *, new_queue, notes):
CinderQueueMove.objects.create(cinder_job=self, notes=notes, to_queue=new_queue)
Expand Down Expand Up @@ -1026,6 +1026,14 @@ def target(self):
else:
return self.collection

def get_target_display(self):
if self.addon_id:
return self.addon.get_type_display()
elif self.user_id:
return _('User profile')
else:
return self.target.__class__.__name__

@property
def is_third_party_initiated(self):
return hasattr(self, 'cinder_job') and bool(self.cinder_job.all_abuse_reports)
Expand Down Expand Up @@ -1277,10 +1285,11 @@ def notify_reviewer_decision(
},
)

def process_action(self, overridden_action=None):
def process_action(self, *, overridden_action=None, release_hold=False):
"""currently only called by decisions from cinder.
see https://mozilla-hub.atlassian.net/browse/AMOENG-1125
"""
assert not self.action_date # we should not be attempting to process twice
appealed_action = (
getattr(self.cinder_job.appealed_decisions.first(), 'action', None)
if hasattr(self, 'cinder_job')
Expand All @@ -1291,7 +1300,7 @@ def process_action(self, overridden_action=None):
overridden_action=overridden_action,
appealed_action=appealed_action,
)
if not action_helper.should_hold_action():
if release_hold or not action_helper.should_hold_action():
self.action_date = datetime.now()
log_entry = action_helper.process_action()
if cinder_job := getattr(self, 'cinder_job', None):
Expand All @@ -1302,29 +1311,12 @@ def process_action(self, overridden_action=None):
action_helper.hold_action()

def get_target_review_url(self):
return (
reverse('reviewers.review', args=(self.target.id,))
if isinstance(self.target, Addon)
else ''
)

def get_target_type(self):
match self.target:
case target if isinstance(target, Addon):
return target.get_type_display()
case target if isinstance(target, UserProfile):
return _('User profile')
case target if isinstance(target, Collection):
return _('Collection')
case target if isinstance(target, Rating):
return _('Rating')
case target:
return target.__class__.__name__
return reverse('reviewers.held_action_review', kwargs={'decision_id': self.id})

def get_target_name(self):
return str(
_('"{}" for {}').format(self.target, self.target.addon.name)
if isinstance(self.target, Rating)
_('"{}" for {}').format(self.rating, self.rating.addon.name)
if self.rating
else getattr(self.target, 'name', self.target)
)

Expand Down
69 changes: 45 additions & 24 deletions src/olympia/abuse/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3070,6 +3070,14 @@ def test_notify_reviewer_decision_rejection_addon_already_disabled(self):
not in mail.outbox[0].body
)

def _test_process_action_ban_user_outcome(self, decision):
self.assertCloseToNow(decision.action_date)
self.assertCloseToNow(decision.user.reload().banned)
assert (
ActivityLog.objects.filter(action=amo.LOG.ADMIN_USER_BANNED.id).count() == 1
)
assert 'appeal' in mail.outbox[0].body

def test_process_action_ban_user_held(self):
user = user_factory(email='[email protected]')
decision = ContentDecision.objects.create(
Expand All @@ -3087,18 +3095,22 @@ def test_process_action_ban_user_held(self):
)
assert len(mail.outbox) == 0

decision.process_action(release_hold=True)
self._test_process_action_ban_user_outcome(decision)

def test_process_action_ban_user(self):
user = user_factory()
decision = ContentDecision.objects.create(
user=user, action=DECISION_ACTIONS.AMO_BAN_USER
)
assert decision.action_date is None
decision.process_action()
self._test_process_action_ban_user_outcome(decision)

def _test_process_action_disable_addon_outcome(self, decision):
self.assertCloseToNow(decision.action_date)
self.assertCloseToNow(user.reload().banned)
assert (
ActivityLog.objects.filter(action=amo.LOG.ADMIN_USER_BANNED.id).count() == 1
)
assert decision.addon.reload().status == amo.STATUS_DISABLED
assert ActivityLog.objects.filter(action=amo.LOG.FORCE_DISABLE.id).count() == 1
assert 'appeal' in mail.outbox[0].body

def test_process_action_disable_addon_held(self):
Expand All @@ -3119,16 +3131,25 @@ def test_process_action_disable_addon_held(self):
)
assert len(mail.outbox) == 0

decision.process_action(release_hold=True)
self._test_process_action_disable_addon_outcome(decision)

def test_process_action_disable_addon(self):
addon = addon_factory(users=[user_factory()])
decision = ContentDecision.objects.create(
addon=addon, action=DECISION_ACTIONS.AMO_DISABLE_ADDON
)
assert decision.action_date is None
decision.process_action()
self._test_process_action_disable_addon_outcome(decision)

def _test_process_action_delete_collection_outcome(self, decision):
self.assertCloseToNow(decision.action_date)
assert addon.reload().status == amo.STATUS_DISABLED
assert ActivityLog.objects.filter(action=amo.LOG.FORCE_DISABLE.id).count() == 1
assert decision.collection.reload().deleted
assert (
ActivityLog.objects.filter(action=amo.LOG.COLLECTION_DELETED.id).count()
== 1
)
assert 'appeal' in mail.outbox[0].body

def test_process_action_delete_collection_held(self):
Expand All @@ -3148,19 +3169,22 @@ def test_process_action_delete_collection_held(self):
)
assert len(mail.outbox) == 0

decision.process_action(release_hold=True)
self._test_process_action_delete_collection_outcome(decision)

def test_process_action_delete_collection(self):
collection = collection_factory(author=user_factory())
decision = ContentDecision.objects.create(
collection=collection, action=DECISION_ACTIONS.AMO_DELETE_COLLECTION
)
assert decision.action_date is None
decision.process_action()
self._test_process_action_delete_collection_outcome(decision)

def _test_process_action_delete_rating_outcome(self, decision):
self.assertCloseToNow(decision.action_date)
assert collection.reload().deleted
assert (
ActivityLog.objects.filter(action=amo.LOG.COLLECTION_DELETED.id).count()
== 1
)
assert decision.rating.reload().deleted
assert ActivityLog.objects.filter(action=amo.LOG.DELETE_RATING.id).count() == 1
assert 'appeal' in mail.outbox[0].body

def test_process_action_delete_rating_held(self):
Expand Down Expand Up @@ -3192,47 +3216,44 @@ def test_process_action_delete_rating_held(self):
)
assert len(mail.outbox) == 0

decision.process_action(release_hold=True)
self._test_process_action_delete_rating_outcome(decision)

def test_process_action_delete_rating(self):
rating = Rating.objects.create(addon=addon_factory(), user=user_factory())
decision = ContentDecision.objects.create(
rating=rating, action=DECISION_ACTIONS.AMO_DELETE_RATING
)
assert decision.action_date is None
decision.process_action()
self.assertCloseToNow(decision.action_date)
assert rating.reload().deleted
assert ActivityLog.objects.filter(action=amo.LOG.DELETE_RATING.id).count() == 1
assert 'appeal' in mail.outbox[0].body
self._test_process_action_delete_rating_outcome(decision)

def test_get_target_review_url(self):
addon = addon_factory()
decision = ContentDecision.objects.create(
addon=addon, action=DECISION_ACTIONS.AMO_DISABLE_ADDON
)
assert decision.get_target_review_url() == reverse(
'reviewers.review', args=(addon.id,)
'reviewers.held_action_review', args=(decision.id,)
)

decision.update(addon=None, user=user_factory())
assert decision.get_target_review_url() == ''

def test_get_target_type(self):
def test_get_target_display(self):
decision = ContentDecision.objects.create(
addon=addon_factory(), action=DECISION_ACTIONS.AMO_DISABLE_ADDON
)
assert decision.get_target_type() == 'Extension'
assert decision.get_target_display() == 'Extension'

decision.update(addon=None, user=user_factory())
assert decision.get_target_type() == 'User profile'
assert decision.get_target_display() == 'User profile'

decision.update(user=None, collection=collection_factory())
assert decision.get_target_type() == 'Collection'
assert decision.get_target_display() == 'Collection'

decision.update(
collection=None,
rating=Rating.objects.create(addon=addon_factory(), user=user_factory()),
)
assert decision.get_target_type() == 'Rating'
assert decision.get_target_display() == 'Rating'

def test_get_target_name(self):
decision = ContentDecision.objects.create(
Expand Down
28 changes: 27 additions & 1 deletion src/olympia/reviewers/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ def create_option(
for text, is_reporter in appeals
),
]
print(subtexts_gen)

label = format_html(
'{}{}{}<details><summary>Show detail on {} reports</summary>'
Expand Down Expand Up @@ -659,3 +658,30 @@ def __init__(self, data, *args, **kw):
self.fields['due_date_reasons'].choices = [
(reason, labels.get(reason, reason)) for reason in due_date_reasons
]


class HeldActionReviewForm(forms.Form):
cinder_job = WidgetRenderedModelMultipleChoiceField(
label='Resolving Job:',
required=False,
queryset=CinderJob.objects.none(),
widget=CinderJobsWidget(),
disabled=True,
)
choice = forms.ChoiceField(
choices=(('yes', 'Proceed with action'), ('no', 'Approve content instead')),
widget=forms.RadioSelect,
)

def __init__(self, *args, **kw):
jobs_qs = kw.pop('cinder_jobs_qs')
super().__init__(*args, **kw)

if jobs_qs:
# Set the queryset for cinder_job
self.fields['cinder_job'].queryset = jobs_qs
self.fields['cinder_job'].initial = [job.id for job in jobs_qs]
if jobs_qs[0].target_addon:
self.fields['choice'].choices += (
('forward', 'Forward to Reviewer Tools'),
)
82 changes: 82 additions & 0 deletions src/olympia/reviewers/templates/reviewers/held_action_review.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{% extends "reviewers/base.html" %}

{% block js %}
{{ super() }}
{% endblock %}

{% block title %}
{{ decision.get_target_display() }}: {{ decision.get_target_name() }} – Add-ons for Firefox
{% endblock %}

{% block content %}

<div class="primary entity-type-{{ decision.get_target_display() }}"
role="main" data-id="{{ decision.target.id }}"
>
<h2>
<span class="app-icon ed-sprite-action-target-{{ decision.get_target_display() }}" title="{{ decision.get_target_display() }}"></span>
{{ decision.get_target_display() }} Decision for {{ decision.get_target_name() }}
</h2>
<table>
<tr class="entity-id">
<th>Target ID</th>
<td>{{ decision.get_target_display() }}: {{ decision.target.id }}</td>
</tr>
<tr class="entity-name">
<th>Target</th>
<td><a href="{{ decision.target.get_url_path() }}">{{ decision.get_target_name() }}</a></td>
</tr>
<tr class="decision-created">
<th>Decision</th>
<td>
<a href="{{ cinder_url }}">{{ decision.cinder_id }}</a>
on <time datetime="{{ decision.created|isotime }}">{{
decision.created|date }}</time>
</td>
</tr>
<tr class="decision-action">
<th>Held Action</th>
<td>
<strong>{{ decision.get_action_display() }}</strong>
</td>
</tr>
<tr class="decision-policies">
<th>Policies</th>
<td>
<ul>
{% for policy in decision.policies.all() %}
<li>{{ policy }}</li>
{% endfor %}
</ul>
</td>
</tr>
<tr class="decision-notes">
<th>Notes from moderator/reviewer</th>
<td>
{{ decision.notes }}
</td>
</tr>
</table>

<h3>Links</h3>
<ul>
{% if decision.addon %}
<li><a href="{{ url('reviewers.review', 'listed', decision.addon_id) }}">Listed Review page</a></li>
<li><a href="{{ url('reviewers.review', 'unlisted', decision.addon_id) }}">Unlisted Review page</a></li>
<li><a href="{{ url('admin:addons_addon_change', decision.addon_id) }}">Admin</a></li>
{% elif decision.user %}
<li><a href="{{ url('admin:users_userprofile_change', decision.user_id) }}">Admin</a></li>
{% elif decision.collection %}
<li><a href="{{ url('admin:bandwagon_collection_change', decision.collection_id) }}">Admin</a></li>
{% elif decision.rating %}
<li><a href="{{ url('admin:ratings_rating_change', decision.rating_id) }}">Admin</a></li>
{% endif %}
</ul>

<form method="POST" class="review-held-action-form">
{% csrf_token %}
{{ form }}
<input type="submit" value="Process" {{ 'disabled' if decision.action_date else '' }}>
</form>
</div> {# /#primary #}
{% endblock %}
2 changes: 1 addition & 1 deletion src/olympia/reviewers/templates/reviewers/queue.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ <h3>
<tbody>
{% for decision in page.object_list %}
<tr id="{{ decision.get_reference_id(short=True) }}" class="held-item">
<td><div class="app-icon ed-sprite-action-target-{{ decision.get_target_type() }}" title="{{ decision.get_target_type() }}"></div></td>
<td><div class="app-icon ed-sprite-action-target-{{ decision.get_target_display() }}" title="{{ decision.get_target_display() }}"></div></td>
<td>{{ decision.get_target_name() }}</td>
<td><a href="{{ decision.get_target_review_url() }}">{{ decision.get_action_display() }}</a></td>
<td>{{ decision.created|datetime }}</td>
Expand Down
Loading
Loading