Merge pull request #41 from mythmon/version-filtering-1248265

Release channels and M2M for country and locale

normandy/classifier/models.py

@@ -10,9 +10,12 @@
 
 class Client(object):
     """A client attempting to fetch a set of recipes."""
-    def __init__(self, request, locale=None):
+    def __init__(self, request, locale=None, release_channel=None, country=None):
         self.request = request
         self.locale = locale
+        self.release_channel = release_channel
+        if country is not None:
+            self.country = country
 
     @cached_property
     def country(self):

normandy/classifier/tests/__init__.py

@@ -1,8 +1,17 @@
 import factory
 
-from normandy.classifier.models import Bundle
+from django.test import RequestFactory
+
+from normandy.classifier.models import Bundle, Client
 
 
 class BundleFactory(factory.Factory):
     class Meta:
         model = Bundle
+
+
+class ClientFactory(factory.Factory):
+    class Meta:
+        model = Client
+
+    request = factory.LazyAttribute(lambda o: RequestFactory().get('/'))

normandy/classifier/tests/test_api.py

@@ -2,7 +2,7 @@
 from rest_framework.reverse import reverse
 
 from normandy.base.tests import Whatever
-from normandy.recipes.tests import RecipeActionFactory, RecipeFactory
+from normandy.recipes.tests import RecipeActionFactory, RecipeFactory, LocaleFactory
 
 
 @pytest.mark.django_db
@@ -38,13 +38,17 @@ def test_it_serves_recipes(self, client):
         }]
 
     def test_it_filters_by_locale(self, client):
-        RecipeFactory(name='english', enabled=True, locale='en-US')
-        RecipeFactory(name='german', enabled=True, locale='de')
-        RecipeFactory(name='any', enabled=True, locale='')
-        RecipeFactory(name='disabled', enabled=False, locale='de')
+        english = LocaleFactory(code='en-US')
+        german = LocaleFactory(code='de')
+
+        RecipeFactory(name='english', enabled=True, locales=[english])
+        RecipeFactory(name='german', enabled=True, locales=[german])
+        RecipeFactory(name='any', enabled=True, locales=[])
+        RecipeFactory(name='both', enabled=True, locales=[english, german])
+        RecipeFactory(name='disabled', enabled=False, locales=[german])
 
         res = client.post('/api/v1/fetch_bundle/', {'locale': 'de'})
         assert res.status_code == 200
 
         recipe_names = set(r['name'] for r in res.data['recipes'])
-        assert recipe_names == {'german', 'any'}
+        assert recipe_names == {'german', 'any', 'both'}

normandy/recipes/admin.py

@@ -19,10 +19,16 @@ class RecipeActionInline(SortableTabularInline):
 
 @admin.register(models.Recipe)
 class RecipeAdmin(NonSortableParentAdmin):
-    list_display = ['name', 'enabled', 'locale', 'country', 'start_time', 'end_time']
-    list_filter = ['enabled', 'locale', 'country']
-    search_fields = ['name', 'locale', 'country']
+    list_display = ['name', 'enabled', 'get_locales', 'get_countries', 'start_time', 'end_time']
+    search_fields = ['name', 'locales', 'countries']
     inlines = [RecipeActionInline]
+    filter_horizontal = ['locales', 'countries']
+
+    list_filter = [
+        ('enabled', admin.BooleanFieldListFilter),
+        ('locales', admin.RelatedOnlyFieldListFilter),
+        ('countries', admin.RelatedOnlyFieldListFilter),
+    ]
 
     fieldsets = [
         [None, {
@@ -31,15 +37,28 @@ class RecipeAdmin(NonSortableParentAdmin):
         ['Delivery Rules', {
             'fields': [
               'enabled',
-              'locale',
-              'country',
+              'locales',
+              'countries',
+              'release_channels',
               'sample_rate',
               'start_time',
               'end_time',
             ]
         }],
     ]
 
+    def get_locales(self, obj):
+        val = ', '.join(l.code for l in obj.locales.all())
+        if not val:
+            val = self.get_empty_value_display()
+        return val
+
+    def get_countries(self, obj):
+        val = ', '.join(l.name for l in obj.countries.all())
+        if not val:
+            val = self.get_empty_value_display()
+        return val
+
 
 @admin.register(models.Action)
 class ActionAdmin(admin.ModelAdmin):
@@ -74,3 +93,8 @@ def in_use(self, action):
         """
         return action.in_use
     in_use.boolean = True
+
+
+@admin.register(models.ReleaseChannel)
+class ReleaseChannelAdmin(admin.ModelAdmin):
+    fields = ['name', 'slug']

normandy/recipes/migrations/0016_auto_20160219_0101.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9 on 2016-02-19 01:01
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('recipes', '0015_auto_20160217_1819'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='ReleaseChannel',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('slug', models.SlugField(max_length=255, unique=True)),
+                ('name', models.CharField(max_length=255)),
+            ],
+            options={
+                'ordering': ['slug'],
+            },
+        ),
+        migrations.AddField(
+            model_name='recipe',
+            name='release_channels',
+            field=models.ManyToManyField(blank=True, to='recipes.ReleaseChannel'),
+        ),
+    ]

normandy/recipes/migrations/0017_auto_20160218_2024.py

@@ -0,0 +1,55 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9 on 2016-02-18 20:24
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('recipes', '0016_auto_20160219_0101'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Country',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('code', models.CharField(max_length=255, unique=True)),
+                ('name', models.CharField(max_length=255)),
+                ('order', models.IntegerField()),
+            ],
+            options={
+                'ordering': ['order', 'name'],
+            },
+        ),
+        migrations.AddField(
+            model_name='locale',
+            name='order',
+            field=models.IntegerField(default=100),
+            preserve_default=False,
+        ),
+        migrations.RemoveField(
+            model_name='recipe',
+            name='country',
+        ),
+        migrations.RemoveField(
+            model_name='recipe',
+            name='locale',
+        ),
+        migrations.AddField(
+            model_name='recipe',
+            name='locales',
+            field=models.ManyToManyField(blank=True, to='recipes.Locale'),
+        ),
+        migrations.AddField(
+            model_name='recipe',
+            name='countries',
+            field=models.ManyToManyField(blank=True, to='recipes.Country'),
+        ),
+        migrations.AlterModelOptions(
+            name='locale',
+            options={'ordering': ['order', 'code']},
+        ),
+    ]

normandy/recipes/migrations/0018_countries.py

@@ -0,0 +1,52 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9 on 2016-02-18 19:03
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+from django_countries import countries
+
+
+def add_countries(apps, schema_editor):
+    Country = apps.get_model('recipes', 'Country')
+    for (code, name) in countries:
+        if code == 'US':
+            order = 0
+        else:
+            order = 100
+
+        Country.objects.update_or_create(code=code, defaults={
+            'name': name,
+            'order': order,
+        })
+
+
+def remove_countries(apps, schema_editor):
+    Country = apps.get_model('recipes', 'Country')
+    Country.objects.all().delete()
+
+
+def set_locale_sort_order(apps, schema_editor):
+    Locale = apps.get_model('recipes', 'Locale')
+    try:
+        english = Locale.objects.get(code='en-US')
+        english.order = 0
+        english.save()
+    except Locale.DoesNotExist:
+        pass
+
+
+def noop(apps, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('recipes', '0017_auto_20160218_2024'),
+    ]
+
+    operations = [
+        migrations.RunPython(add_countries, remove_countries),
+        migrations.RunPython(set_locale_sort_order, noop),
+    ]

normandy/recipes/models.py

@@ -5,7 +5,6 @@
 from django.db import models
 
 from adminsortable.models import SortableMixin
-from django_countries.fields import CountryField
 from rest_framework.reverse import reverse
 
 from normandy.recipes import utils
@@ -21,13 +20,48 @@ class Locale(models.Model):
     code = models.CharField(max_length=255, unique=True)
     english_name = models.CharField(max_length=255, blank=True)
     native_name = models.CharField(max_length=255, blank=True)
+    order = models.IntegerField(default=100)
 
     class Meta:
-        ordering = ['code']
+        ordering = ['order', 'code']
 
     def __str__(self):
         return '{self.code} ({self.english_name})'.format(self=self)
 
+    def matches(self, other):
+        return self.code.lower() == other.lower()
+
+
+class ReleaseChannel(models.Model):
+    """Release channel of Firefox"""
+    slug = models.SlugField(max_length=255, unique=True)
+    name = models.CharField(max_length=255)
+
+    class Meta:
+        ordering = ['slug']
+
+    def __str__(self):
+        return self.name
+
+    def matches(self, other):
+        return self.slug == other or self.name == other
+
+
+class Country(models.Model):
+    """Database table for countries from django_countries."""
+    code = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255)
+    order = models.IntegerField(default=100)
+
+    class Meta:
+        ordering = ['order', 'name']
+
+    def __str__(self):
+        return '{self.name} ({self.code})'.format(self=self)
+
+    def matches(self, other):
+        return self.code == other or self.name == other
+
 
 class Recipe(models.Model):
     """A set of actions to be fetched and executed by users."""
@@ -36,33 +70,41 @@ class Recipe(models.Model):
 
     # Fields that determine who this recipe is sent to.
     enabled = models.BooleanField(default=False)
-    locale = models.ForeignKey(Locale, blank=True, null=True)
-    country = CountryField(blank=True, null=True, default=None)
+    locales = models.ManyToManyField(Locale, blank=True)
+    countries = models.ManyToManyField(Country, blank=True)
     start_time = models.DateTimeField(blank=True, null=True, default=None)
     end_time = models.DateTimeField(blank=True, null=True, default=None)
     sample_rate = PercentField(default=100)
+    release_channels = models.ManyToManyField(ReleaseChannel, blank=True)
 
     def log_rejection(self, msg):
         logger.debug('{} rejected: {}'.format(self, msg))
 
+    def check_many(self, name, field_qs, client_val):
+        field_vals = list(field_qs.all())
+
+        if field_vals == []:
+            return True
+
+        for val in field_vals:
+            if val.matches(client_val):
+                return True
+
+        field_vals_str = ', '.join(str(o) for o in field_vals)
+        self.log_rejection('client {name} ({client_val}) does not match recipe '
+                           '(choices are {field_vals})'
+                           .format(name=name, client_val=client_val, field_vals=field_vals_str))
+        return False
+
     def matches(self, client):
         """
         Return whether this Recipe should be sent to the given client.
         """
+        # This should be ordered roughly by performance cost
         if not self.enabled:
             self.log_rejection('not enabled')
             return False
 
-        if self.locale and client.locale and self.locale.code.lower() != client.locale.lower():
-            self.log_rejection('recipe locale ({self.locale!r}) != '
-                               'client locale ({client.locale!r})')
-            return False
-
-        if self.country and self.country != client.country:
-            self.log_rejection('recipe country ({self.country!r}) != '
-                               'client country ({client.country!r})')
-            return False
-
         if self.start_time and self.start_time > client.request_time:
             self.log_rejection('start time not met ({})'.format(self.start_time))
             return False
@@ -77,6 +119,15 @@ def matches(self, client):
                 self.log_rejection('did not match sample')
                 return False
 
+        if not self.check_many('locale', self.locales, client.locale):
+            return False
+
+        if not self.check_many('country', self.countries, client.country):
+            return False
+
+        if not self.check_many('channel', self.release_channels, client.release_channel):
+            return False
+
         return True
 
     def __repr__(self):