Skip to content

Commit

Permalink
Allow deleted versions to use "Confirm Multiple Versions" action (#22942
Browse files Browse the repository at this point in the history
)
  • Loading branch information
chrstinalin authored Jan 9, 2025
1 parent f993f4e commit c276784
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
6 changes: 6 additions & 0 deletions src/olympia/reviewers/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,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
Expand Down
24 changes: 21 additions & 3 deletions src/olympia/reviewers/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand 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)
Expand All @@ -830,7 +837,7 @@ def test_versions_queryset_contains_pending_files_for_unlisted(

# <option>s should as well, and the value depends on which version:
# the approved one and the pending one should have different values.
assert len(doc('option')) == 4
assert len(doc('option')) == 5
option1 = doc('option[value="%s"]' % self.version.pk)[0]
assert option1.attrib.get('class') == 'data-toggle'
assert option1.attrib.get('data-value').split(' ') == [
Expand Down Expand Up @@ -873,6 +880,17 @@ def test_versions_queryset_contains_pending_files_for_unlisted(
assert option4.attrib.get('data-value') == 'reply'
assert option4.attrib.get('value') == str(blocked_version.pk)

option5 = doc('option[value="%s"]' % deleted_version.pk)[0]
assert option5.attrib.get('class') == 'data-toggle'
assert option5.attrib.get('data-value').split(' ') == [
'unreject_multiple_versions',
'reply',
# The deleted auto-approved version can still have
# its auto-approval confirmed.
'confirm_multiple_versions',
]
assert option5.attrib.get('value') == str(deleted_version.pk)

def test_set_needs_human_review_presence(self):
self.grant_permission(self.request.user, 'Addons:Review')
deleted_but_signed = version_factory(
Expand Down

0 comments on commit c276784

Please sign in to comment.