From f2eea2dcba0c42c622ff107c3cc8f054c3a017da Mon Sep 17 00:00:00 2001 From: eviljeff Date: Wed, 15 Jul 2026 18:33:59 +0100 Subject: [PATCH] drop obsolete tested on fields in review form --- src/olympia/reviewers/forms.py | 11 ------ .../reviewers/templates/reviewers/review.html | 27 ------------- src/olympia/reviewers/tests/test_utils.py | 7 +--- src/olympia/reviewers/tests/test_views.py | 38 +------------------ src/olympia/reviewers/views.py | 7 ---- 5 files changed, 3 insertions(+), 87 deletions(-) diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py index 056fef7d6548..de0786d836ee 100644 --- a/src/olympia/reviewers/forms.py +++ b/src/olympia/reviewers/forms.py @@ -516,8 +516,6 @@ class ReviewForm(forms.Form): label='Versions:', ) # queryset is set later in __init__. - operating_systems = forms.CharField(required=False, label='Operating systems:') - applications = forms.CharField(required=False, label='Applications:') delayed_rejection = forms.NullBooleanField( initial=False, required=False, @@ -876,15 +874,6 @@ def __init__(self, *args, **kw): self.fields['policy_values'].queryset = self.fields['cinder_policies'].queryset - @property - def unreviewed_files(self): - return ( - (self.helper.version.file,) - if self.helper.version - and self.helper.version.file.status == amo.STATUS_AWAITING_REVIEW - else () - ) - class MOTDForm(forms.Form): motd = forms.CharField(required=True, widget=widgets.Textarea()) diff --git a/src/olympia/reviewers/templates/reviewers/review.html b/src/olympia/reviewers/templates/reviewers/review.html index fc82342097c1..4f2723a96d97 100644 --- a/src/olympia/reviewers/templates/reviewers/review.html +++ b/src/olympia/reviewers/templates/reviewers/review.html @@ -258,33 +258,6 @@

Enforcement actions:

{{ form.cinder_jobs_to_resolve.errors }} -
- - -
- -
- Tested on: - - {{ form.operating_systems }} - - {{ form.applications }} - {{ form.operating_systems.errors }} - {{ form.applications.errors }} -
{% if form.delayed_rejection and form.delayed_rejection_date %}
diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index c49103f92c13..86a0f0cfe818 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -123,12 +123,7 @@ def setup_data( self.helper.set_data(data) def get_data(self): - return { - 'comments': 'foo', - 'action': 'public', - 'operating_systems': 'osx', - 'applications': 'Firefox', - } + return {'comments': 'foo', 'action': 'public'} def get_helper(self, content_review=False, human_review=True): return ReviewHelper( diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index 2e8f8879002a..215b3597a4ca 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -2435,11 +2435,7 @@ def get_addon(self): return Addon.objects.get(pk=self.addon.pk) def get_dict(self, **kw): - data = { - 'operating_systems': 'win', - 'applications': 'some app', - 'comments': 'some comment', - } + data = {'comments': 'some comment'} data.update(kw) return data @@ -3450,12 +3446,7 @@ def check_entry(parent): check_entry(version_history) def test_files_in_item_history(self): - data = { - 'action': 'public', - 'operating_systems': 'win', - 'applications': 'something', - 'comments': 'something', - } + data = {'action': 'public', 'comments': 'something'} self.client.post(self.url, data) response = self.client.get(self.url) @@ -4142,8 +4133,6 @@ def review_version(self, version, url, mock_sign): data = { 'action': action, - 'operating_systems': 'win', - 'applications': 'some app', 'comments': 'some comment', 'cinder_policies': [policy.id], 'versions': [version.pk], @@ -5955,11 +5944,6 @@ def test_data_value_attributes(self): 'reject_multiple_versions', ] - # We don't have approve/reject actions so these have an empty - # data-value. - assert doc('.data-toggle.review-files')[0].attrib['data-value'] == '' - assert doc('.data-toggle.review-tested')[0].attrib['data-value'] == '' - def test_test_data_value_attributes_admin(self): AutoApprovalSummary.objects.create( verdict=amo.AUTO_APPROVED, version=self.version @@ -6075,11 +6059,6 @@ def test_data_value_attributes_unlisted(self): 'data-value' ].split(' ') == ['approve_multiple_versions', 'reject_multiple_versions'] - # We don't have approve/reject actions so these have an empty - # data-value. - assert doc('.data-toggle.review-files')[0].attrib['data-value'] == '' - assert doc('.data-toggle.review-tested')[0].attrib['data-value'] == '' - assert doc('.data-toggle.review-delayed-rejection')[0].attrib[ 'data-value' ].split(' ') == [ @@ -6105,8 +6084,6 @@ def test_no_data_value_attributes_unlisted_for_viewer(self): assert not doc('select#id_versions.data-toggle')[0] assert doc('.data-toggle.review-comments')[0].attrib['data-value'] == '' assert doc('.data-toggle.review-actions-policies')[0].attrib['data-value'] == '' - assert doc('.data-toggle.review-files')[0].attrib['data-value'] == '' - assert doc('.data-toggle.review-tested')[0].attrib['data-value'] == '' # Viewer won't see delayed rejection inputs, need a reviewer. assert not doc('.data-toggle.review-delayed-rejection') @@ -6143,14 +6120,6 @@ def test_data_value_attributes_unreviewed(self): 'request_legal_review', 'comment', ] - assert doc('.data-toggle.review-files')[0].attrib['data-value'].split(' ') == [ - 'public', - 'reject', - ] - assert doc('.data-toggle.review-tested')[0].attrib['data-value'].split(' ') == [ - 'public', - 'reject', - ] assert doc('.data-toggle.review-actions-policies')[0].attrib[ 'data-value' ].split(' ') == ['public', 'reject', 'reject_multiple_versions'] @@ -6191,12 +6160,9 @@ def test_data_value_attributes_static_theme(self): 'request_legal_review', 'comment', ] - # we don't show files, reasons, and tested with for any static theme actions - assert doc('.data-toggle.review-files')[0].attrib['data-value'] == '' assert doc('.data-toggle.review-actions-policies')[0].attrib[ 'data-value' ].split(' ') == ['reject', 'reject_multiple_versions'] - assert doc('.data-toggle.review-tested')[0].attrib['data-value'] == '' def test_post_review_ignore_disabled(self): # Though the latest version will be disabled, the add-on is public and diff --git a/src/olympia/reviewers/views.py b/src/olympia/reviewers/views.py index 54c19704dd3a..7492cd6991e8 100644 --- a/src/olympia/reviewers/views.py +++ b/src/olympia/reviewers/views.py @@ -581,12 +581,6 @@ def review(request, addon, channel=None): .order_by('-created') .first() ) - # The actions we shouldn't show a minimal form for. - # FIXME: the full/minimal distinction is obsolete - we don't use the extra fields - actions_full = [] - for key, action in actions: - if not (is_static_theme or action.get('minimal')): - actions_full.append(key) addons_sharing_same_guid = ( Addon.unfiltered.all() @@ -694,7 +688,6 @@ def review(request, addon, channel=None): and request.user.is_staff ), actions=actions, - actions_full=actions_full, addon=addon, addons_sharing_same_guid=addons_sharing_same_guid, approvals_info=approvals_info,