Merge pull request #181 from Cadasta/bugfix/#162

Fixes #162: Add SlugModel

cadasta/core/fixtures.py

@@ -121,7 +121,7 @@ def add_test_projects(self):
 
         projs.append(ProjectFactory.create(
             name='Kibera Test Project',
-            project_slug='kibera',
+            slug='kibera',
             description="""This is a test project.  This is a test project.
             This is a test project.  This is a test project.  This is a test
             project.  This is a test project.  This is a test project.  This
@@ -137,7 +137,7 @@ def add_test_projects(self):
         ))
         projs.append(ProjectFactory.create(
             name='H4H Test Project',
-            project_slug='h4h-test-project',
+            slug='h4h-test-project',
             description="""This is a test project.  This is a test project.
             This is a test project.  This is a test project.  This is a test
             project.  This is a test project.  This is a test project.  This
@@ -153,7 +153,7 @@ def add_test_projects(self):
         ))
         projs.append(ProjectFactory.create(
             name='Cadasta Indonesia Test Project',
-            project_slug='cadasta-indonesia-test-project',
+            slug='cadasta-indonesia-test-project',
             description="""This is another test project.  This is another test
             project. This is another test project.  This is another test
             project. This is another test project.  This is a test project.
@@ -171,7 +171,7 @@ def add_test_projects(self):
         ))
         projs.append(ProjectFactory.create(
             name='Cadasta Myanmar Test Project',
-            project_slug='cadasta-myanmar-test-project',
+            slug='cadasta-myanmar-test-project',
             description=""""This is another test project.  This is another test
             project. This is another test project.  This is another test
             project. This is another test project.  This is a test project.
@@ -182,7 +182,7 @@ def add_test_projects(self):
         ))
         projs.append(ProjectFactory.create(
             name='London 1',
-            project_slug='london-1',
+            slug='london-1',
             description=""""This is another test project.  This is another test
             project. This is another test project.  This is another test
             project. This is another test project.  This is a test project.
@@ -204,7 +204,7 @@ def add_test_projects(self):
         ))
         projs.append(ProjectFactory.create(
             name='London 2',
-            project_slug='london-2',
+            slug='london-2',
             description=""""This is another test project.  This is another test
             project. This is another test project.  This is another test
             project. This is another test project.  This is a test project.

cadasta/core/models.py

@@ -1,3 +1,5 @@
+import itertools
+from django.utils.text import slugify
 from django.db import models
 
 from .util import random_id, ID_FIELD_LENGTH
@@ -23,3 +25,25 @@ def save(self, *args, **kwargs):
 
         else:
             super(RandomIDModel, self).save(*args, **kwargs)
+
+
+class SlugModel:
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.__original_slug = self.slug
+
+    def save(self, *args, **kwargs):
+        if not self.slug:
+            self.slug = slugify(self.name)
+
+        orig = self.slug
+
+        if self.__original_slug != self.slug:
+            for x in itertools.count(1):
+                if not type(self).objects.filter(slug=self.slug).exists():
+                    break
+                self.slug = '{}-{}'.format(orig, x)
+
+        self.__original_slug = self.slug
+
+        return super().save(*args, **kwargs)

cadasta/core/tests/test_models.py

@@ -1,9 +1,10 @@
 import random
+from django.db.models import SlugField, CharField, Model
 from django.test import TestCase
-from ..models import RandomIDModel
+from ..models import RandomIDModel, SlugModel
 
 
-class MyTestModel(RandomIDModel):
+class MyRandomIdModel(RandomIDModel):
     class Meta:
         app_label = 'core'
 
@@ -12,15 +13,85 @@ class RandomIDModelTest(TestCase):
     abstract_model = RandomIDModel
 
     def test_save(self):
-        instance = MyTestModel()
+        instance = MyRandomIdModel()
         instance.save()
         assert instance.id is not None
 
     def test_duplicate_ids(self):
         random.seed(a=10)
-        instance1 = MyTestModel()
+        instance1 = MyRandomIdModel()
         instance1.save()
         random.seed(a=10)
-        instance2 = MyTestModel()
+        instance2 = MyRandomIdModel()
         instance2.save()
         assert instance1.id != instance2.id
+
+
+class MySlugModel(SlugModel, Model):
+    slug = SlugField(max_length=50, unique=True)
+
+    class Meta:
+        app_label = 'core'
+
+
+class SlugModelTest(TestCase):
+    name = CharField(max_length=200)
+    abstract_model = SlugModel
+
+    def test_save(self):
+        instance = MySlugModel()
+        instance.name = 'Test Name'
+        instance.save()
+
+        instance.refresh_from_db()
+        assert instance.slug == 'test-name'
+
+    def test_save_slug_is_set(self):
+        instance = MySlugModel()
+        instance.name = 'Test Name'
+        instance.slug = 'slug'
+        instance.save()
+
+        instance.refresh_from_db()
+        assert instance.slug == 'slug'
+
+    def test_duplicate_slug(self):
+        instance1 = MySlugModel()
+        instance1.name = 'Test Name'
+        instance1.save()
+
+        instance2 = MySlugModel()
+        instance2.name = 'Test Name'
+        instance2.save()
+
+        instance1.refresh_from_db()
+        instance2.refresh_from_db()
+        assert instance1.slug != instance2.slug
+        assert instance2.slug == 'test-name-1'
+
+    def test_duplicate_slug_is_set(self):
+        instance1 = MySlugModel()
+        instance1.name = 'Test Name'
+        instance1.save()
+
+        instance2 = MySlugModel()
+        instance2.name = 'Some Name'
+        instance2.slug = instance1.slug
+        instance2.save()
+
+        instance1.refresh_from_db()
+        instance2.refresh_from_db()
+        assert instance1.slug != instance2.slug
+        assert instance2.slug == 'test-name-1'
+
+    def test_keep_slug(self):
+        instance = MySlugModel()
+        instance.name = 'Test Name'
+        instance.save()
+
+        instance.name = 'Other Name'
+        instance.save()
+
+        instance.refresh_from_db()
+        assert instance.name == 'Other Name'
+        assert instance.slug == 'test-name'

cadasta/organization/forms.py

@@ -5,7 +5,6 @@
 from django.contrib.postgres import forms as pg_forms
 from django.contrib.gis import forms as gisforms
 from django.utils.translation import ugettext as _
-from django.utils.text import slugify
 
 from leaflet.forms.widgets import LeafletWidget
 from tutelary.models import Role
@@ -49,15 +48,6 @@ def save(self, *args, **kwargs):
         instance = super(OrganizationForm, self).save(commit=False)
         create = not instance.id
 
-        # ensuring slug is unique
-        if not instance.slug:
-            instance.slug = orig = slugify(instance.name)
-            for x in itertools.count(1):
-                if not Organization.objects.filter(
-                        slug=instance.slug).exists():
-                    break
-                instance.slug = '{}-{}'.format(orig, x)
-
         instance.save()
 
         if create:

cadasta/organization/migrations/0001_initial.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Generated by Django 1.9.4 on 2016-05-04 06:50
+# Generated by Django 1.9.4 on 2016-05-03 14:24
 from __future__ import unicode_literals
 
 from django.conf import settings
@@ -55,7 +55,7 @@ class Migration(migrations.Migration):
             fields=[
                 ('id', models.CharField(max_length=24, primary_key=True, serialize=False)),
                 ('name', models.CharField(max_length=100)),
-                ('project_slug', models.SlugField(null=True, unique=True)),
+                ('slug', models.SlugField(null=True, unique=True)),
                 ('country', django_countries.fields.CountryField(max_length=2, null=True)),
                 ('description', models.TextField(blank=True, null=True)),
                 ('archived', models.BooleanField(default=False)),

cadasta/organization/models.py

@@ -6,10 +6,11 @@
 from django.utils.translation import ugettext as _
 import django.contrib.gis.db.models as gismodels
 
+
 from tutelary.decorators import permissioned_model
 from tutelary.models import Policy
 
-from core.models import RandomIDModel
+from core.models import RandomIDModel, SlugModel
 from geography.models import WorldBorder
 from .validators import validate_contact
 from .choices import ROLE_CHOICES
@@ -31,7 +32,7 @@ def get_policy_instance(policy_name, variables):
 
 
 @permissioned_model
-class Organization(RandomIDModel):
+class Organization(SlugModel, RandomIDModel):
     name = models.CharField(max_length=200)
     slug = models.SlugField(max_length=50, unique=True)
     description = models.TextField(null=True, blank=True)
@@ -138,13 +139,13 @@ def remove_project_membership(sender, instance, **kwargs):
 
 
 @permissioned_model
-class Project(RandomIDModel):
+class Project(SlugModel, RandomIDModel):
     ACCESS_CHOICES = [
         ("public", _("Public")),
         ("private", _("Private")),
     ]
     name = models.CharField(max_length=100)
-    project_slug = models.SlugField(max_length=50, unique=True, null=True)
+    slug = models.SlugField(max_length=50, unique=True, null=True)
     organization = models.ForeignKey(Organization, related_name='projects')
     country = CountryField(null=True)
     description = models.TextField(null=True, blank=True)
@@ -164,7 +165,7 @@ class Meta:
 
     class TutelaryMeta:
         perm_type = 'project'
-        path_fields = ('organization', 'project_slug')
+        path_fields = ('organization', 'slug')
         actions = (
             ('project.list',
              {'description': _("List existing projects in an organization"),
@@ -239,19 +240,19 @@ def assign_project_permissions(sender, instance, **kwargs):
 
     project_manager = get_policy_instance('project-manager', {
         'organization': instance.project.organization.slug,
-        'project': instance.project.project_slug
+        'project': instance.project.slug
     })
     is_manager = project_manager in assigned_policies
 
     project_user = get_policy_instance('project-user', {
         'organization': instance.project.organization.slug,
-        'project': instance.project.project_slug
+        'project': instance.project.slug
     })
     is_user = project_user in assigned_policies
 
     data_collector = get_policy_instance('data-collector', {
         'organization': instance.project.organization.slug,
-        'project': instance.project.project_slug
+        'project': instance.project.slug
     })
     is_collector = data_collector in assigned_policies
 

cadasta/organization/serializers.py

@@ -1,5 +1,4 @@
 from django.conf import settings
-from django.utils.text import slugify
 from django.db.models import Q
 from django.core.mail import send_mail
 from django.core.urlresolvers import reverse
@@ -23,16 +22,10 @@ class OrganizationSerializer(DetailSerializer, FieldSelectorSerializer,
     class Meta:
         model = Organization
         fields = ('id', 'slug', 'name', 'description', 'archived', 'urls',
-                  'contacts', 'users')
-        read_only_fields = ('id',)
+                  'contacts', 'users',)
+        read_only_fields = ('id', 'slug',)
         detail_only_fields = ('users',)
 
-    def to_internal_value(self, data):
-        if not data.get('slug'):
-            data['slug'] = slugify(data.get('name'))
-
-        return super(OrganizationSerializer, self).to_internal_value(data)
-
     def create(self, *args, **kwargs):
         org = super(OrganizationSerializer, self).create(*args, **kwargs)
 
@@ -53,8 +46,8 @@ class ProjectSerializer(DetailSerializer, serializers.ModelSerializer):
     class Meta:
         model = Project
         fields = ('id', 'organization', 'country', 'name', 'description',
-                  'archived', 'urls', 'contacts', 'users', 'access',)
-        read_only_fields = ('id', 'country',)
+                  'archived', 'urls', 'contacts', 'users', 'access', 'slug')
+        read_only_fields = ('id', 'country', 'slug')
         detail_only_fields = ('users',)
 
     def create(self, validated_data):
@@ -81,7 +74,7 @@ def get_url(self, object):
         return reverse(
             'organization:project-dashboard',
             kwargs={'organization': object.organization.slug,
-                    'project': object.project_slug})
+                    'project': object.slug})
 
 
 class EntityUserSerializer(serializers.Serializer):

cadasta/organization/tests/factories.py

@@ -30,7 +30,7 @@ class Meta:
         model = Project
 
     name = factory.Sequence(lambda n: "Project #%s" % n)
-    project_slug = factory.Sequence(lambda n: "project-%s" % n)
+    slug = factory.Sequence(lambda n: "project-%s" % n)
     organization = factory.SubFactory(OrganizationFactory)
     description = factory.Sequence(
         lambda n: "Project #%s description" % n)

cadasta/organization/tests/test_forms.py

@@ -25,6 +25,7 @@ def test_add_organization(self):
         }
         self._save(data)
         org = Organization.objects.first()
+
         assert org.slug == 'org'
         assert OrganizationRole.objects.filter(organization=org).count() == 1
 

cadasta/organization/tests/test_serializers.py

@@ -34,12 +34,6 @@ def test_slug_field_is_set(self):
         assert OrganizationRole.objects.filter(
             organization=org_instance).count() == 1
 
-    def test_slug_field_is_unique(self):
-        OrganizationFactory.create(slug='org-slug')
-        org_data = {'name': 'Org Slug', 'slug': 'org-slug'}
-        serializer = serializers.OrganizationSerializer(data=org_data)
-        assert not serializer.is_valid()
-
     def test_users_are_not_serialized(self):
         users = UserFactory.create_batch(2)
         org = OrganizationFactory.create(add_users=users)
@@ -122,7 +116,7 @@ def test_method_fields_work(self):
         assert test_data['properties']['url'] == reverse(
             'organization:project-dashboard',
             kwargs={'organization': project.organization.slug,
-                    'project': project.project_slug})
+                    'project': project.slug})
 
 
 class OrganizationUserSerializerTest(TestCase):

cadasta/organization/tests/test_views_api_organizations.py

@@ -71,6 +71,7 @@ def test_list_only_one_organization_is_authorized(self):
         """
         OrganizationFactory.create_from_kwargs([{}, {'slug': 'unauthorized'}])
         content = self._get(user=self.unauth_user, status=200, length=1)
+        print(content[0])
         assert content[0]['slug'] != 'unauthorized'
 
     def test_full_list_with_unauthorized_user(self):