Apply comments

src/olympia/blocklist/migrations/0037_alter_blockversion_block_type_and_more.py

@@ -4,16 +4,6 @@
 import olympia.amo.fields
 
 
-def convert_soft_to_block_type(apps, schema_editor):
-    BlockVersion = apps.get_model('blocklist', 'BlockVersion')
-    # soft=True -> block_type=1
-    # soft=False -> block_type=0
-    BlockVersion.objects.filter(soft=True).update(block_type=1)
-    BlockVersion.objects.filter(soft=False).update(block_type=0)
-
-
-
-
 class Migration(migrations.Migration):
 
     dependencies = [
@@ -24,12 +14,11 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='blockversion',
             name='block_type',
-            field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0),
+            field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0, null=True),
         ),
         migrations.AlterField(
             model_name='blocklistsubmission',
             name='block_type',
-            field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0),
+            field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0, null=True),
         ),
-        migrations.RunPython(convert_soft_to_block_type),
     ]

src/olympia/blocklist/migrations/0038_remove_blockversion_soft.py

@@ -3,6 +3,13 @@
 from django.db import migrations
 import olympia.amo.fields
 
+def convert_soft_to_block_type(apps, schema_editor):
+    BlockVersion = apps.get_model('blocklist', 'BlockVersion')
+    # soft=True -> block_type=1
+    # soft=False -> block_type=0
+    BlockVersion.objects.filter(soft=True).update(block_type=1)
+    BlockVersion.objects.filter(soft=False).update(block_type=0)
+
 
 def convert_block_type_to_soft(apps, schema_editor):
     BlockVersion = apps.get_model('blocklist', 'BlockVersion')
@@ -19,7 +26,7 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
-        migrations.RunPython(migrations.RunPython.noop, convert_block_type_to_soft),
+        migrations.RunPython(convert_soft_to_block_type, convert_block_type_to_soft),
         migrations.RemoveField(
             model_name='blockversion',
             name='soft',

src/olympia/blocklist/models.py

@@ -175,6 +175,9 @@ def __str__(self) -> str:
             f'-> Version.id={self.version_id}'
         )
 
+    def get_admin_facing_block_type_display(self) -> str:
+        return '🛑 Hard-Blocked' if self.block_type == BlockType.BLOCKED else '⚠️ Soft-Blocked'
+
 
 class BlocklistSubmissionQuerySet(BaseQuerySet):
     def delayed(self):

src/olympia/blocklist/templates/admin/blocklist/widgets/blocks.html

@@ -29,10 +29,10 @@ <h3>{{ blocks|length|intcomma }} Add-on GUIDs with {{ total_adu|intcomma }} user
               name="changed_version_ids"
               value="{{ version.id }}"
               {% if version.id in widget.value %}checked{% endif %}
-            > {% if is_add_change %}Block{% else %}Unblock{% endif %} {{ version.version }} {% if version.blockversion %}({{ version.blockversion.get_block_type_display }}){% endif %}</label>
+            > {% if is_add_change %}Block{% else %}Unblock{% endif %} {{ version.version }} {% if version.blockversion %}({{ version.blockversion.get_admin_facing_block_type_display }}){% endif %}</label>
           {% else %}
             <span>
-              {{ version.version }} ({% if version.is_blocked %}{{ version.blockversion.get_block_type_display }}{% else %}&#x1F7E2; Not Blocked{% endif %})
+              {{ version.version }} ({% if version.is_blocked %}{{ version.blockversion.get_admin_facing_block_type_display }}{% else %}&#x1F7E2; Not Blocked{% endif %})
               {% if version.blocklist_submission_id %}
                 [<a href="{% url 'admin:blocklist_blocklistsubmission_change' version.blocklist_submission_id %}">Edit Submission</a>]
               {% endif %}

src/olympia/blocklist/tests/test_admin.py

@@ -201,7 +201,7 @@ def test_view_versions(self):
         assert response.status_code == 200
         doc = pq(response.content.decode('utf-8'))
         assert doc('.field-blocked_versions').text() == (
-            'Blocked versions:\n2.0 (Blocked), 3.0 (Restricted)'
+            'Blocked versions:\n2.0 (🛑 Hard-Blocked), 3.0 (⚠️ Soft-Blocked)'
         )
 
 
@@ -338,12 +338,12 @@ def test_version_checkboxes(self):
 
         # not a checkbox because blocked already and this is an add action
         assert doc(f'li[data-version-id="{ver_block.id}"]').text() == (
-            f'{ver_block.version} (Blocked)'
+            f'{ver_block.version} (🛑 Hard-Blocked)'
         )
 
         # not a checkbox because (soft-)blocked already and this is an add action
         assert doc(f'li[data-version-id="{ver_soft_block.id}"]').text() == (
-            f'{ver_soft_block.version} (Restricted)'
+            f'{ver_soft_block.version} (⚠️ Soft-Blocked)'
         )
 
         # Now with an existing submission
@@ -1144,7 +1144,7 @@ def test_edit_with_blocklist_create(self):
         doc = pq(response.content.decode('utf-8'))
         # Can't change block type when approving
         assert not doc('.field_block_type select')
-        assert doc('.field-block_type .readonly').text() == 'Blocked'
+        assert doc('.field-block_type .readonly').text() == '🛑 Hard-Block'
         buttons = doc('.submit-row input')
         assert buttons[0].attrib['value'] == 'Update'
         assert len(buttons) == 1
@@ -2394,12 +2394,12 @@ def test_version_checkboxes(self):
         check_checkbox(checkboxes[0], ver_block)
         assert (
             checkboxes[0].getparent().text_content().strip()
-            == f'Unblock {ver_block.version} (Blocked)'
+            == f'Unblock {ver_block.version} (🛑 Hard-Blocked)'
         )
         check_checkbox(checkboxes[1], ver_soft_block)
         assert (
             checkboxes[1].getparent().text_content().strip()
-            == f'Unblock {ver_soft_block.version} (Restricted)'
+            == f'Unblock {ver_soft_block.version} (⚠️ Soft-Blocked)'
         )
 
         # not a checkbox because in a submission, green circle because not blocked
@@ -2408,7 +2408,7 @@ def test_version_checkboxes(self):
         )
         # not a checkbox because in a submission, red hexagon because hard blocked
         assert doc(f'li[data-version-id="{ver_del_subm.id}"]').text() == (
-            f'{ver_del_subm.version} (Blocked) [Edit Submission]'
+            f'{ver_del_subm.version} (🛑 Hard-Blocked) [Edit Submission]'
         )
         # not a checkbox because not blocked, and this is a delete action
         assert doc(f'li[data-version-id="{ver.id}"]').text() == (

src/olympia/blocklist/tests/test_models.py

@@ -26,15 +26,16 @@ def test_str(self):
             block=self.block, version=self.version, block_type=BlockType.BLOCKED
         )
         assert str(hard_block_version) == (
-            f'Block.id={self.block.id} ' f'(Blocked) -> Version.id={self.version.id}'
+            f'Block.id={self.block.id} '
+            f'(🛑 Hard-Blocked) -> Version.id={self.version.id}'
         )
 
         soft_block_version = BlockVersion.objects.create(
             block=self.block, version=self.version_2, block_type=BlockType.SOFT_BLOCKED
         )
         assert str(soft_block_version.reload()) == (
             f'Block.id={self.block.id} '
-            f'(Restricted) -> Version.id={self.version_2.id}'
+            f'(⚠️ Soft-Blocked) -> Version.id={self.version_2.id}'
         )