have (single) listed as a select choice

Author: eviljeff

src/olympia/devhub/forms.py

@@ -1603,6 +1603,7 @@ class RollbackVersionForm(forms.Form):
         coerce=int,
         widget=forms.RadioSelect(),
     )
+    listed_version = forms.TypedChoiceField(choices=(), coerce=int, required=False)
     unlisted_version = forms.ModelChoiceField(
         queryset=Version.objects.none(), empty_label=gettext('Choose version')
     )
@@ -1611,13 +1612,19 @@ class RollbackVersionForm(forms.Form):
     def __init__(self, *args, **kwargs):
         self.addon = kwargs.pop('addon')
         super().__init__(*args, **kwargs)
+        listed = self.fields['listed_version']
         unlisted = self.fields['unlisted_version']
         channel = self.fields['channel']
 
         listed_queryset = self.addon.rollbackable_versions_qs(amo.CHANNEL_LISTED)
         self.listed_count = listed_queryset.count()
         # We currently only allow rolling back to a single listed version.
-        self.listed_version = listed_queryset.first() if self.listed_count else None
+        if self.listed_count:
+            listed_version_obj = listed_queryset.first()
+            listed.choices = ((listed_version_obj.id, listed_version_obj),)
+        else:
+            listed.choices = ((None, 'No appropriate version available'),)
+
         unlisted.queryset = self.addon.rollbackable_versions_qs(amo.CHANNEL_UNLISTED)
         self.unlisted_count = unlisted.queryset.count()
 
@@ -1652,10 +1659,15 @@ def clean_new_version_string(self):
             raise forms.ValidationError(error)
         return new_version_string
 
+    def clean_listed_version(self):
+        version_pk, version_obj = self.fields['listed_version'].choices[0]
+        self.cleaned_data['listed_version'] = version_obj if version_pk else None
+        return self.cleaned_data['listed_version']
+
     def clean(self):
         data = super().clean()
         data['version'] = (
-            self.listed_version
+            data.get('listed_version')
             if data.get('channel') == amo.CHANNEL_LISTED
             else data.get('unlisted_version')
             if data.get('channel') == amo.CHANNEL_UNLISTED

src/olympia/devhub/templates/devhub/includes/modal_rollback_version.html

@@ -20,9 +20,10 @@ <h3>{{ _('Roll back to a previous version') }}</h3>
         {% endif %}
         {% if has_listed_versions %}
         <tr id="listed-version-row">
-        <td><label>{{ _('Version') }}</label></td>
+        <td><label for="id_listed_version">{{ _('Version') }}</label></td>
         <td>
-            <span id="listed_version_for_rollback">{{ rollback_form.listed_version or _('No appropriate version available') }}</span>
+            {{ rollback_form.listed_version }}
+            {{ rollback_form.listed_version.errors }}
         </td>
         </tr>
         {% endif %}

src/olympia/devhub/tests/test_forms.py

@@ -1468,7 +1468,9 @@ def test_no_listed_version_choices(self):
             v2.version,
             v1.version,
         ]
-        assert form.listed_version is None
+        assert form.fields['listed_version'].choices == [
+            (None, 'No appropriate version available')
+        ]
 
     def test_no_unlisted_version_choices(self):
         """No listed version choices should default the channel to unlisted, and make
@@ -1479,7 +1481,7 @@ def test_no_unlisted_version_choices(self):
         form = forms.RollbackVersionForm(addon=addon)
         assert form.fields['channel'].initial == amo.CHANNEL_LISTED
         assert form.fields['unlisted_version'].required is False
-        assert form.listed_version == v2
+        assert form.fields['listed_version'].choices == [(v2.id, v2)]
 
     def test_listed_and_unlisted_version_choices(self):
         """Both listed and unlisted version choices should be available."""
@@ -1492,7 +1494,7 @@ def test_listed_and_unlisted_version_choices(self):
         form = forms.RollbackVersionForm(addon=addon)
         assert form.fields['channel'].initial is amo.CHANNEL_UNLISTED
         assert form.fields['unlisted_version'].required is True
-        assert form.listed_version == lv2
+        assert form.fields['listed_version'].choices == [(lv2.id, lv2)]
         assert [str(lab) for _, lab in form.fields['unlisted_version'].choices] == [
             'Choose version',
             uv2.version,
@@ -1585,17 +1587,27 @@ def test_clean_adds_version(self):
         }
         # test we're selecting the listed channel
         form = forms.RollbackVersionForm(data, addon=addon)
-        assert form.listed_version == lv1
+        assert form.fields['listed_version'].choices == [(lv1.id, lv1)]
         assert form.is_valid(), form.errors
-        # the listed version is added as the chosen version
-        assert form.clean() == {**data, 'unlisted_version': uv1, 'version': lv1}
+        # the listed version is added, and as the chosen version
+        assert form.clean() == {
+            **data,
+            'unlisted_version': uv1,
+            'listed_version': lv1,
+            'version': lv1,
+        }
 
         # repeat with unlisted
         data['channel'] = amo.CHANNEL_UNLISTED
         form = forms.RollbackVersionForm(data, addon=addon)
         assert form.is_valid(), form.errors
         # the selected unlisted version is added as the chosen version
-        assert form.clean() == {**data, 'unlisted_version': uv1, 'version': uv1}
+        assert form.clean() == {
+            **data,
+            'unlisted_version': uv1,
+            'listed_version': lv1,
+            'version': uv1,
+        }
 
         # and when we select listed but there is no listed version availble: error
         data['channel'] = amo.CHANNEL_LISTED

src/olympia/devhub/tests/test_views_versions.py

@@ -842,7 +842,8 @@ def test_version_rollback_form_listed_only(self):
         # we hide the channel selector, because there are no other channels
         assert modal('input[name="channel"]').attr('type') == 'hidden'
         assert modal('input[name="channel"]').attr('value') == str(amo.CHANNEL_LISTED)
-        assert modal('#listed_version_for_rollback').text() == first_version.version
+        assert modal('select option').length == 1
+        assert modal('select option')[0].text == first_version.version
         # and the select for unlisted versions is not present
         assert 'Choose version' not in modal.html()
 
@@ -861,7 +862,7 @@ def test_version_rollback_form_unlisted_only(self):
         channel_input = modal('input[name="channel"]')
         assert channel_input.attr('type') == 'hidden'
         assert channel_input.attr('value') == str(amo.CHANNEL_UNLISTED)
-        assert modal('#listed_version_for_rollback').length == 0
+        assert modal('#id_listed_version').length == 0
         assert 'Choose version' in modal.html()
         assert modal('select option').length == 2
         assert modal('select option')[0].text == 'Choose version'
@@ -891,12 +892,13 @@ def test_version_rollback_form_listed_but_not_appropriate(self):
         # and disable it
         assert modal(f'{channel_selector}[disabled]').length == 2
         assert modal(channel_selector).length == 2
+        assert modal('#id_listed_version option').length == 1
         assert (
-            modal('#listed_version_for_rollback').text()
+            modal('#id_listed_version option').text()
             == 'No appropriate version available'
         )
         assert 'Choose version' in modal.html()
-        assert modal('select option').length == 2
+        assert modal('#id_unlisted_version option').length == 2
 
     @override_switch('version-rollback', active=True)
     def test_version_rollback_form_both_channels(self):
@@ -914,7 +916,8 @@ def test_version_rollback_form_both_channels(self):
         # this time we show the channel selector, because there are both channels
         channel_selector = 'input[name="channel"]'
         assert modal(channel_selector).attr('type') == 'radio'
-        assert not modal(f'{channel_selector}[value="{amo.CHANNEL_UNLISTED}"]').attr(
+        # and preselect unlisted because it's the most recent channel for a version
+        assert modal(f'{channel_selector}[value="{amo.CHANNEL_UNLISTED}"]').attr(
             'checked'
         )
         assert not modal(f'{channel_selector}[value="{amo.CHANNEL_LISTED}"]').attr(
@@ -923,8 +926,10 @@ def test_version_rollback_form_both_channels(self):
         # and they're enabled
         assert modal(f'{channel_selector}[disabled]').length == 0
         assert modal(channel_selector).length == 2
-        assert modal('#listed_version_for_rollback').text() == listed_version.version
+        assert modal('#id_listed_version option').length == 1
+        assert modal('#id_listed_version option').text() == listed_version.version
         assert 'Choose version' in modal.html()
+        assert modal('#id_unlisted_version option').length == 2
 
     @override_switch('version-rollback', active=True)
     def test_version_rollback_submit(self):