From 6d4192f42d66a79ec8c05bdd46f4604742d71f29 Mon Sep 17 00:00:00 2001 From: Christina Lin <44586776+chrstinalin@users.noreply.github.com> Date: Thu, 12 Dec 2024 09:31:40 -0500 Subject: [PATCH 1/5] Allow deleted versions to use "Confirm Multiple Versions" action --- src/olympia/reviewers/forms.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py index c87d186663a..f619b367b59 100644 --- a/src/olympia/reviewers/forms.py +++ b/src/olympia/reviewers/forms.py @@ -137,6 +137,7 @@ class VersionsChoiceWidget(forms.SelectMultiple): ], amo.STATUS_DISABLED: [ 'unreject_multiple_versions', + 'confirm_multiple_versions', 'reply', ], }, @@ -153,6 +154,7 @@ class VersionsChoiceWidget(forms.SelectMultiple): ], amo.STATUS_DISABLED: [ 'unreject_multiple_versions', + 'confirm_multiple_versions', 'reply', ], }, From ae3b87d2c6ec1b933fdf0a93fe32eec3b7e28ed6 Mon Sep 17 00:00:00 2001 From: Christina Lin <44586776+chrstinalin@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:10:13 -0500 Subject: [PATCH 2/5] don't include rejections, specifically in the case of auto approval --- src/olympia/reviewers/forms.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py index f619b367b59..a7b83509b8e 100644 --- a/src/olympia/reviewers/forms.py +++ b/src/olympia/reviewers/forms.py @@ -137,7 +137,6 @@ class VersionsChoiceWidget(forms.SelectMultiple): ], amo.STATUS_DISABLED: [ 'unreject_multiple_versions', - 'confirm_multiple_versions', 'reply', ], }, @@ -154,7 +153,6 @@ class VersionsChoiceWidget(forms.SelectMultiple): ], amo.STATUS_DISABLED: [ 'unreject_multiple_versions', - 'confirm_multiple_versions', 'reply', ], }, @@ -185,6 +183,12 @@ def create_option(self, *args, **kwargs): # for a version to require human review. if obj.file.status != amo.STATUS_DISABLED or obj.file.is_signed: actions.append('set_needs_human_review_multiple_versions') + + # If a version was auto-approved but deleted, we still want to + # allow confirmation of its auto-approval. + if obj.deleted and obj.was_auto_approved: + actions.append('confirm_multiple_versions') + option['attrs']['class'] = 'data-toggle' option['attrs']['data-value'] = ' '.join(actions) # Just in case, let's now force the label to be a string (it would be From 2ee09e91157ec7fac9100dab7d3543ef3fc34cb3 Mon Sep 17 00:00:00 2001 From: Christina Lin <44586776+chrstinalin@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:26:54 -0500 Subject: [PATCH 3/5] test --- src/olympia/reviewers/tests/test_views.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index 29fd71da6e0..c392a28f659 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -5241,6 +5241,25 @@ def test_data_value_attributes_unlisted(self): elm = doc('.data-toggle.review-delayed-rejection')[0] assert elm.attrib['data-value'] == 'reject_multiple_versions' + def test_deleted_autoapproval_unlisted(self): + self.version.update(channel=amo.CHANNEL_UNLISTED) + + AutoApprovalSummary.objects.create( + verdict=amo.AUTO_APPROVED, version=self.version + ) + self.grant_permission(self.reviewer, 'Addons:ReviewUnlisted') + unlisted_url = reverse('reviewers.review', args=['unlisted', self.addon.pk]) + + self.version.deleted = True + response = self.client.get(unlisted_url) + doc = pq(response.content) + assert response.status_code == 200 + + # Deleted version with an auto-approval should still be available to confirm. + option = doc('#id_versions option')[0] + assert option.attrib['value'] == str(self.version.id) + assert 'confirm_multiple_versions' in option.attrib['data-value'] + def test_no_data_value_attributes_unlisted_for_viewer(self): self.version.update(channel=amo.CHANNEL_UNLISTED) AutoApprovalSummary.objects.create( From ebdf4a6497085d42013667f7ec837c57befc8cd4 Mon Sep 17 00:00:00 2001 From: Christina Lin <44586776+chrstinalin@users.noreply.github.com> Date: Thu, 12 Dec 2024 15:44:48 -0500 Subject: [PATCH 4/5] better test --- src/olympia/reviewers/tests/test_forms.py | 23 ++++++++++++++++++++--- src/olympia/reviewers/tests/test_views.py | 19 ------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/olympia/reviewers/tests/test_forms.py b/src/olympia/reviewers/tests/test_forms.py index 39942d93e80..f442421a82d 100644 --- a/src/olympia/reviewers/tests/test_forms.py +++ b/src/olympia/reviewers/tests/test_forms.py @@ -797,6 +797,13 @@ def test_versions_queryset_contains_pending_files_for_unlisted( version_ids=[blocked_version.id], updated_by=user_factory(), ) + deleted_version = version_factory( + addon=self.addon, + channel=amo.CHANNEL_UNLISTED, + file_kw={'status': amo.STATUS_DISABLED}, + ) + deleted_version.delete() + self.version.update(channel=amo.CHANNEL_UNLISTED) # auto-approve everything for version in Version.unfiltered.all(): @@ -815,9 +822,9 @@ def test_versions_queryset_contains_pending_files_for_unlisted( assert not form.is_bound assert form.fields['versions'].required is False assert list(form.fields['versions'].queryset) == list( - self.addon.versions.all().order_by('pk') + Version.unfiltered_for_relations.filter(addon=self.addon).order_by('pk') ) - assert form.fields['versions'].queryset.count() == 4 + assert form.fields['versions'].queryset.count() == 5 content = str(form['versions']) doc = pq(content) @@ -830,7 +837,7 @@ def test_versions_queryset_contains_pending_files_for_unlisted( #