Skip to content
This repository was archived by the owner on Jan 29, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion normandy/classifier/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
11 changes: 10 additions & 1 deletion normandy/classifier/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -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('/'))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh man this is awesome

16 changes: 10 additions & 6 deletions normandy/classifier/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'}
34 changes: 29 additions & 5 deletions normandy/recipes/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand All @@ -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):
Expand Down Expand Up @@ -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']
31 changes: 31 additions & 0 deletions normandy/recipes/migrations/0016_auto_20160219_0101.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
55 changes: 55 additions & 0 deletions normandy/recipes/migrations/0017_auto_20160218_2024.py
Original file line number Diff line number Diff line change
@@ -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']},
),
]
52 changes: 52 additions & 0 deletions normandy/recipes/migrations/0018_countries.py
Original file line number Diff line number Diff line change
@@ -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),
]
79 changes: 65 additions & 14 deletions normandy/recipes/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit vague what this is matching against; perhaps we pass in the client and it extracts from the client the fields it's matching against?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to change this with the other matching refactors we talked about, since changing this sort of breaks check_many.

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."""
Expand All @@ -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
Expand All @@ -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):
Expand Down
Loading