Skip to content

Commit

Permalink
Migrate from BlockVersion.soft to BlockVersion.block_type
Browse files Browse the repository at this point in the history
This patch does the following:

- Introduce `PositiveTinyIntegerField` to support `tinyint unsigned` fields
- Rename `soft` to `block_type` via 2 separate migrations (add + remove)
- Introduce `BlockType` enum and use it instead of the `soft` boolean
- Unify the string versions of enum values
  • Loading branch information
bakulf committed Nov 4, 2024
1 parent a83aead commit ee25d84
Show file tree
Hide file tree
Showing 19 changed files with 207 additions and 128 deletions.
5 changes: 5 additions & 0 deletions src/olympia/amo/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,8 @@ def formfield(self, **kwargs):
defaults = {'form_class': fields.CharField, 'validators': self.validators}
defaults.update(kwargs)
return super().formfield(**defaults)


class PositiveTinyIntegerField(models.PositiveSmallIntegerField):
def db_type(self, connection):
return 'tinyint unsigned'
6 changes: 3 additions & 3 deletions src/olympia/amo/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
from olympia.api.tests import JWTAuthKeyTester
from olympia.applications.models import AppVersion
from olympia.bandwagon.models import Collection
from olympia.blocklist.models import Block, BlockVersion
from olympia.blocklist.models import Block, BlockType, BlockVersion
from olympia.constants.categories import CATEGORIES
from olympia.files.models import File
from olympia.promoted.models import (
Expand Down Expand Up @@ -989,13 +989,13 @@ def version_factory(file_kw=None, **kw):
return ver


def block_factory(*, version_ids=None, soft=False, **kwargs):
def block_factory(*, version_ids=None, block_type=BlockType.BLOCKED, **kwargs):
block = Block.objects.create(**kwargs)
if version_ids is None and block.addon:
version_ids = list(block.addon.versions.values_list('id', flat=True))
if version_ids is not None:
BlockVersion.objects.bulk_create(
BlockVersion(block=block, version_id=version_id, soft=soft)
BlockVersion(block=block, version_id=version_id, block_type=block_type)
for version_id in version_ids
)
return block
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/blocklist/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def users(self, obj):

def blocked_versions(self, obj):
return ', '.join(
f'{version.version} ({version.get_soft_display()})'
f'{version.version} ({version.get_block_type_display()})'
for version in obj.blockversion_set.all()
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Generated by Django 4.2.16 on 2024-10-31 13:26

from django.db import models, 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')
# block_type=1 -> soft=True
# block_type=0 -> soft=False
BlockVersion.objects.filter(block_type=1).update(soft=True)
BlockVersion.objects.filter(block_type=0).update(soft=False)


class Migration(migrations.Migration):

dependencies = [
('blocklist', '0036_blocklistsubmission_block_type_and_more'),
]

operations = [
migrations.AddField(
model_name='blockversion',
name='block_type',
field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0, null=True),
),
migrations.AlterField(
model_name='blockversion',
name='soft',
field=models.BooleanField(default=False, null=True),
),
migrations.AlterField(
model_name='blocklistsubmission',
name='block_type',
field=olympia.amo.fields.PositiveTinyIntegerField(choices=[(0, 'Blocked'), (1, 'Restricted')], default=0),
),
migrations.RunPython(convert_soft_to_block_type, convert_block_type_to_soft),
]
4 changes: 2 additions & 2 deletions src/olympia/blocklist/mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import olympia.core.logger
from olympia.amo.utils import SafeStorage
from olympia.blocklist.models import BlockVersion
from olympia.blocklist.models import BlockType, BlockVersion
from olympia.blocklist.utils import datetime_to_ts


Expand Down Expand Up @@ -61,7 +61,7 @@ class MLBFDataType(Enum):

def fetch_blocked_from_db():
qs = BlockVersion.objects.filter(
version__file__is_signed=True, soft=False
version__file__is_signed=True, block_type=BlockType.BLOCKED
).values_list('block__guid', 'version__version', 'version_id', named=True)
return set(qs)

Expand Down
42 changes: 17 additions & 25 deletions src/olympia/blocklist/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@
from django.urls import reverse
from django.utils.functional import cached_property
from django.utils.html import format_html
from django.utils.translation import gettext_lazy as _

from extended_choices import Choices
from multidb import get_replica

from olympia import amo
from olympia.addons.models import Addon
from olympia.amo.fields import PositiveTinyIntegerField
from olympia.amo.models import BaseQuerySet, ManagerBase, ModelBase
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.utils import chunked
from olympia.constants.blocklist import BLOCK_TYPE_END_USERS_CHOICES
from olympia.users.models import UserProfile
from olympia.versions.models import Version

Expand Down Expand Up @@ -157,24 +157,29 @@ def get_blocks_from_guids(cls, guids):
return blocks


class BlockType(models.IntegerChoices):
BLOCKED = 0, '🛑 Hard-Blocked'
SOFT_BLOCKED = 1, '⚠️ Soft-Blocked'


class BlockVersion(ModelBase):
BLOCK_TYPE_CHOICES = Choices(
('BLOCKED', 0, '🛑 Hard-Blocked'),
('SOFT_BLOCKED', 1, '⚠️ Soft-Blocked'),
)
version = models.OneToOneField(Version, on_delete=models.CASCADE)
block = models.ForeignKey(Block, on_delete=models.CASCADE)
soft = models.BooleanField(default=False, choices=BLOCK_TYPE_CHOICES)
block_type = PositiveTinyIntegerField(
default=BlockType.BLOCKED,
choices=BlockType.choices,
null=True,
)

def __str__(self) -> str:
return (
f'Block.id={self.block_id} ({self.get_soft_display()}) '
f'Block.id={self.block_id} ({self.get_block_type_display()}) '
f'-> Version.id={self.version_id}'
)

def get_user_facing_block_type_display(self):
"""Like get_soft_display(), but using strings meant for end-users."""
return BLOCK_TYPE_END_USERS_CHOICES.for_value(int(self.soft)).display
"""Like get_block_type_display(), but using strings meant for end-users."""
return _('Blocked') if self.block_type == BlockType.BLOCKED else _('Restricted')


class BlocklistSubmissionQuerySet(BaseQuerySet):
Expand Down Expand Up @@ -216,18 +221,6 @@ class BlocklistSubmission(ModelBase):
ACTION_ADDCHANGE: 'Add/Change',
ACTION_DELETE: 'Delete',
}
BLOCK_TYPE_CHOICES = Choices(
(
BlockVersion.BLOCK_TYPE_CHOICES.BLOCKED.constant,
BlockVersion.BLOCK_TYPE_CHOICES.BLOCKED.value,
'🛑 Hard-Block',
),
(
BlockVersion.BLOCK_TYPE_CHOICES.SOFT_BLOCKED.constant,
BlockVersion.BLOCK_TYPE_CHOICES.SOFT_BLOCKED.value,
'⚠️ Soft-Block',
),
)
FakeBlockAddonVersion = namedtuple(
'FakeBlockAddonVersion',
(
Expand Down Expand Up @@ -276,9 +269,8 @@ class BlocklistSubmission(ModelBase):
help_text='The submission will not be published into blocks before this time.',
)
disable_addon = models.BooleanField(default=True)
block_type = models.IntegerField(
default=BLOCK_TYPE_CHOICES.BLOCKED,
choices=BLOCK_TYPE_CHOICES,
block_type = PositiveTinyIntegerField(
default=BlockType.BLOCKED, choices=BlockType.choices
)

objects = BlocklistSubmissionManager()
Expand Down
14 changes: 7 additions & 7 deletions src/olympia/blocklist/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from olympia.api.utils import is_gate_active
from olympia.versions.models import Version

from .models import Block
from .models import Block, BlockType


class BlockSerializer(AMOModelSerializer):
Expand All @@ -31,24 +31,24 @@ class Meta:
'is_all_versions',
)

def _get_blocked_for(self, obj, *, is_soft):
def _get_blocked_for(self, obj, *, block_type):
if not hasattr(obj, '_blockversion_set_qs_values_list'):
obj._blockversion_set_qs_values_list = sorted(
obj.blockversion_set.order_by('version__version').values_list(
'version__version', 'soft'
'version__version', 'block_type'
)
)
return [
version
for version, soft in obj._blockversion_set_qs_values_list
if soft == is_soft
for version, _block_type in obj._blockversion_set_qs_values_list
if _block_type == block_type
]

def get_blocked(self, obj):
return self._get_blocked_for(obj, is_soft=False)
return self._get_blocked_for(obj, block_type=BlockType.BLOCKED)

def get_soft_blocked(self, obj):
return self._get_blocked_for(obj, is_soft=True)
return self._get_blocked_for(obj, block_type=BlockType.SOFT_BLOCKED)

def get_is_all_versions(self, obj):
cannot_upload_new_versions = not obj.addon or obj.addon.status in (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_soft_display }}){% endif %}</label>
> {% if is_add_change %}Block{% else %}Unblock{% endif %} {{ version.version }} {% if version.blockversion %}({{ version.blockversion.get_block_type_display }}){% endif %}</label>
{% else %}
<span>
{{ version.version }} ({% if version.is_blocked %}{{ version.blockversion.get_soft_display }}{% else %}&#x1F7E2; Not Blocked{% endif %})
{{ version.version }} ({% if version.is_blocked %}{{ version.blockversion.get_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 %}
Expand Down
Loading

0 comments on commit ee25d84

Please sign in to comment.