aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--patchwork/api/event.py6
-rw-r--r--patchwork/api/patch.py18
-rw-r--r--patchwork/fixtures/default_states.xml10
-rw-r--r--patchwork/migrations/0038_state_slug.py68
-rw-r--r--patchwork/models.py8
-rw-r--r--patchwork/tests/api/test_patch.py4
-rw-r--r--patchwork/tests/utils.py1
7 files changed, 92 insertions, 23 deletions
diff --git a/patchwork/api/event.py b/patchwork/api/event.py
index e00ce05..a066faa 100644
--- a/patchwork/api/event.py
+++ b/patchwork/api/event.py
@@ -8,6 +8,7 @@ from collections import OrderedDict
from rest_framework.generics import ListAPIView
from rest_framework.serializers import ModelSerializer
from rest_framework.serializers import SerializerMethodField
+from rest_framework.serializers import SlugRelatedField
from patchwork.api.embedded import CheckSerializer
from patchwork.api.embedded import CoverLetterSerializer
@@ -16,7 +17,6 @@ from patchwork.api.embedded import ProjectSerializer
from patchwork.api.embedded import SeriesSerializer
from patchwork.api.embedded import UserSerializer
from patchwork.api.filters import EventFilterSet
-from patchwork.api.patch import StateField
from patchwork.models import Event
@@ -27,8 +27,8 @@ class EventSerializer(ModelSerializer):
patch = PatchSerializer(read_only=True)
series = SeriesSerializer(read_only=True)
cover = CoverLetterSerializer(read_only=True)
- previous_state = StateField()
- current_state = StateField()
+ previous_state = SlugRelatedField(slug_field='slug', read_only=True)
+ current_state = SlugRelatedField(slug_field='slug', read_only=True)
previous_delegate = UserSerializer()
current_delegate = UserSerializer()
created_check = SerializerMethodField()
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index d1c9904..a29a1ab 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -5,6 +5,7 @@
import email.parser
+from django.utils.text import slugify
from django.utils.translation import ugettext_lazy as _
from rest_framework.generics import ListAPIView
from rest_framework.generics import RetrieveUpdateAPIView
@@ -28,10 +29,7 @@ from patchwork.parser import clean_subject
class StateField(RelatedField):
"""Avoid the need for a state endpoint.
- NOTE(stephenfin): This field will only function for State names consisting
- of alphanumeric characters, underscores and single spaces. In Patchwork
- 2.0+, we should consider adding a slug field to the State object and make
- use of the SlugRelatedField in DRF.
+ TODO(stephenfin): Consider switching to SlugRelatedField for the v2.0 API.
"""
default_error_messages = {
'required': _('This field is required.'),
@@ -41,19 +39,13 @@ class StateField(RelatedField):
'{data_type}.'),
}
- @staticmethod
- def format_state_name(state):
- return ' '.join(state.split('-'))
-
def to_internal_value(self, data):
+ data = slugify(data.lower())
try:
- data = self.format_state_name(data)
- return self.get_queryset().get(name__iexact=data)
+ return self.get_queryset().get(slug=data)
except State.DoesNotExist:
self.fail('invalid_choice', name=data, choices=', '.join([
- self.format_state_name(x.name) for x in self.get_queryset()]))
- except (TypeError, ValueError):
- self.fail('incorrect_type', data_type=type(data).__name__)
+ x.slug for x in self.get_queryset()]))
def to_representation(self, obj):
return obj.slug
diff --git a/patchwork/fixtures/default_states.xml b/patchwork/fixtures/default_states.xml
index 86e1105..1bbd919 100644
--- a/patchwork/fixtures/default_states.xml
+++ b/patchwork/fixtures/default_states.xml
@@ -4,51 +4,61 @@
<!-- default states -->
<object pk="1" model="patchwork.state">
<field type="CharField" name="name">New</field>
+ <field type="SlugField" name="name">new</field>
<field type="IntegerField" name="ordering">0</field>
<field type="BooleanField" name="action_required">True</field>
</object>
<object pk="2" model="patchwork.state">
<field type="CharField" name="name">Under Review</field>
+ <field type="SlugField" name="name">under-review</field>
<field type="IntegerField" name="ordering">1</field>
<field type="BooleanField" name="action_required">True</field>
</object>
<object pk="3" model="patchwork.state">
<field type="CharField" name="name">Accepted</field>
+ <field type="SlugField" name="name">accepted</field>
<field type="IntegerField" name="ordering">2</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="4" model="patchwork.state">
<field type="CharField" name="name">Rejected</field>
+ <field type="SlugField" name="name">rejected</field>
<field type="IntegerField" name="ordering">3</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="5" model="patchwork.state">
<field type="CharField" name="name">RFC</field>
+ <field type="SlugField" name="name">rfc</field>
<field type="IntegerField" name="ordering">4</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="6" model="patchwork.state">
<field type="CharField" name="name">Not Applicable</field>
+ <field type="SlugField" name="name">not-applicable</field>
<field type="IntegerField" name="ordering">5</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="7" model="patchwork.state">
<field type="CharField" name="name">Changes Requested</field>
+ <field type="SlugField" name="name">changes-requested</field>
<field type="IntegerField" name="ordering">6</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="8" model="patchwork.state">
<field type="CharField" name="name">Awaiting Upstream</field>
+ <field type="SlugField" name="name">awaiting-upstream</field>
<field type="IntegerField" name="ordering">7</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="9" model="patchwork.state">
<field type="CharField" name="name">Superseded</field>
+ <field type="SlugField" name="name">superseded</field>
<field type="IntegerField" name="ordering">8</field>
<field type="BooleanField" name="action_required">False</field>
</object>
<object pk="10" model="patchwork.state">
<field type="CharField" name="name">Deferred</field>
+ <field type="SlugField" name="name">deferred</field>
<field type="IntegerField" name="ordering">9</field>
<field type="BooleanField" name="action_required">False</field>
</object>
diff --git a/patchwork/migrations/0038_state_slug.py b/patchwork/migrations/0038_state_slug.py
new file mode 100644
index 0000000..500624d
--- /dev/null
+++ b/patchwork/migrations/0038_state_slug.py
@@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import unicode_literals
+
+from django.db import migrations, models, transaction
+from django.utils.text import slugify
+
+
+def validate_uniqueness(apps, schema_editor):
+ """Ensure all State.name entries are unique.
+
+ We need to do this before enforcing a uniqueness constraint.
+ """
+
+ State = apps.get_model('patchwork', 'State')
+
+ total_count = State.objects.count()
+ slugs_count = len(set([
+ slugify(state.name) for state in State.objects.all()]))
+
+ if slugs_count != total_count:
+ raise Exception(
+ 'You have non-unique States entries that need to be combined '
+ 'before you can run this migration. This migration must be done '
+ 'by hand. If you need assistance, please contact '
+ 'patchwork@ozlabs.org')
+
+
+def populate_slug_field(apps, schema_editor):
+
+ State = apps.get_model('patchwork', 'State')
+
+ with transaction.atomic():
+ for state in State.objects.all():
+ state.slug = slugify(state.name)
+ state.save()
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('patchwork', '0037_event_actor'),
+ ]
+
+ operations = [
+ # Ensure all 'State.name' entries are unique
+ migrations.RunPython(validate_uniqueness, migrations.RunPython.noop),
+ # Apply the unique constraint to 'State.name'
+ migrations.AlterField(
+ model_name='state',
+ name='name',
+ field=models.CharField(max_length=100, unique=True),
+ ),
+ # Add a 'State.slug' field but allow it to be nullable
+ migrations.AddField(
+ model_name='state',
+ name='slug',
+ field=models.SlugField(blank=True, max_length=100, null=True, unique=True),
+ ),
+ # Populate the 'State.slug' field
+ migrations.RunPython(populate_slug_field, migrations.RunPython.noop),
+ # Make the 'State.slug' field non-nullable
+ migrations.AlterField(
+ model_name='state',
+ name='slug',
+ field=models.SlugField(max_length=100, unique=True),
+ ),
+ ]
diff --git a/patchwork/models.py b/patchwork/models.py
index 7f0efd4..d95eb0d 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -215,14 +215,12 @@ models.signals.post_save.connect(_user_saved_callback, sender=User)
@python_2_unicode_compatible
class State(models.Model):
- name = models.CharField(max_length=100)
+ # Both of these fields should be unique
+ name = models.CharField(max_length=100, unique=True)
+ slug = models.SlugField(max_length=100, unique=True)
ordering = models.IntegerField(unique=True)
action_required = models.BooleanField(default=True)
- @property
- def slug(self):
- return '-'.join(self.name.lower().split())
-
def __str__(self):
return self.name
diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index bf3ef9f..ef418e2 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -300,7 +300,7 @@ class TestPatchAPI(utils.APITestCase):
self.client.force_authenticate(user=user)
resp = self.client.patch(self.api_url(patch.id),
- {'state': state.name, 'delegate': user.id})
+ {'state': state.slug, 'delegate': user.id})
self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
self.assertEqual(Patch.objects.get(id=patch.id).state, state)
self.assertEqual(Patch.objects.get(id=patch.id).delegate, user)
@@ -326,7 +326,7 @@ class TestPatchAPI(utils.APITestCase):
self.client.force_authenticate(user=user)
resp = self.client.patch(self.api_url(patch.id), {'state': 'foobar'})
self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
- self.assertContains(resp, 'Expected one of: %s.' % state.name,
+ self.assertContains(resp, 'Expected one of: %s.' % state.slug,
status_code=status.HTTP_400_BAD_REQUEST)
def test_update_legacy_delegate(self):
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 577183d..91bcbe1 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -131,6 +131,7 @@ def create_state(**kwargs):
values = {
'name': 'state_%d' % num,
+ 'slug': 'state-%d' % num,
'ordering': num,
'action_required': True,
}
generated by cgit v1.2.3 (git 2.50.1) at 2025-10-27 04:27:22 +0000