From 79531cac13903fca11e11ac525adb9e527974838 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Sat, 17 Apr 2010 12:18:40 -0500 Subject: [PATCH] Adding smarter validation to relationship status objects on save, to prevent slug collisions between different status slugs. Migrating slug fields to char fields so 'slug' validation won't occur, allowing statuses to be made invisible by giving them an invalid slug. Also added RelationshipStatus to the admin. --- example/example.db | Bin 43008 -> 47104 bytes relationships/admin.py | 8 +- relationships/forms.py | 39 ++++++ .../migrations/0003_slugs_to_charfields.py | 121 ++++++++++++++++++ relationships/models.py | 7 +- relationships/tests.py | 65 ++++++++++ 6 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 relationships/forms.py create mode 100644 relationships/migrations/0003_slugs_to_charfields.py diff --git a/example/example.db b/example/example.db index 2a4e555ec75c49c60a1dfb74f9200863e0338462..d828d7992ac0b32506d11fce16cef365ce3d4c2e 100644 GIT binary patch delta 870 zcmZWoT}V_x6rPzgx<6{`s_bqm>)osETJ6fcyQ`~9fj{{JVJQ^SOUtUOUf4hHu2L_u z5rr9nIuG_>W=c;{L>Gx(q$eqYus2@<@x_9L@}(COnpM}JhWp)_bG|b(=X{6BY?91R zGFxRHKu4pVPdbj8Id_Mt4));}>_HZGVF$iyW;zJ-da6gY0uQOQX;}~{_x=FwpokWm zxRa7C1nbZT6=aJu9|$v6-C~QCakgG%#vyI9s-9Mc2EtKwB85x%h5|H&&=k&+1GoU6 zn3+adw-C@ilG9lG?C2nK?H0{dmo;f+6mVguvCw+M$G+6sG=JkJS*1PtRoY680PP4i zU>e%_rGDtT1O>=-u}zIc!)YZEPmL+blq5;MP%1hxnhK>8p`o#`I-*3P!zr&MOCHJZ zkpqGp2ul3(HUwIHjlO0!-F>d&glG!-{0%KFa#Kq)+vs-wLj;;7Z$N(2W7hLbby&0Y zXUj?pQ71y3_z~VAgJd6Od3mz7HhdXa<%8tPm`xMtHXQz#KwIbg3X0GgLTh-JJcWK9 z{9L=X=)>&W(p%QJq#4|T&U(EK-?_^6mIU_t$-Uy7f|xiFaS|)9{3XU=zAa?M-#L*V zcL$t0Yi}>U?_f67zWjhw@X;u4HIwu22dY1l#Z)mLbE7O@hHrbqHh3C7NBZ z7_lAl=Bouu8@7{UlTdurWS_??3tnGPY7REa*pBl&{LlNnc;zt_jx{Xgvzw!V!^3f& bL*#JqJhoxJf@*a6w~ib9DV+2)-`Md#{_Nik delta 1292 zcmb7EO>7%Q6rP#4Sw~KcaSGVAOpln8 z7r0?;H5>yTV{<$8I`|p=k?hZ)-)m#r?QjY)tY23?AavMMq(8V%>@pns zAGg_Tc8swk4VAJ*AN|-9!JNL~>1OtD<7z>hEn}N;uLUh2w1Dem3;N*-T!pK(ia-~* zkddDPTtF9U1H1ID&bl2gp-Z*OSL~rjS!W_luLS~hakSH9$V9EIb3!scx%8mCL0Ak+ z2$pmQQ-PCZ%qa5%X*tM;MOED|2qAU9mj;7z$61L}w@SuNNls zL;2XCA}E8|tdh^XH6h05WBKU3lAn-c<8fhfd?*(i9C+R;@&ez>2YUsccP81{jvk_kE zlY>%F4AaX8pEdw7#POks9E`}JzK|q~Vd?y-xyW=*;RL&~83}zzqa9;5UQHk8q$J zp2Smy()7_>F+*(d6i$~4h0?rXHTuY4H^Odw23~+Qa>q#YoDsoX?+UvuPDR1*#VNlW z#Xfqs+D(rvv($IGiz%wdQuMPf7t8tQQreVx+{5|PTIpD#T$s)Hxj9vv&Z=6s%8$f7fP92F`=d2 zoXu&eq@Vt@;%zg+&S<$g^?0gbTlOW~th20`f~9XPxy^ErgmC|8@h3bxVevj}YiRzV z=5#%1PD(3pYmQ+xMwq~D1h?Ui(aBBlw}2h{NE>)?TFa%1$>tz3rtmi5JNUdIz7ufL z%dp5CU?8NabVG245+zfKvh0N>$`HI04SGovCCuNTf*1dV!cH_OHbxq5>6TCKLPzr- vF@|6bjggx~#_!iE2lVx-gz4zgmvrrnPH&z5hD~^PR!nMCth8)7_i5{&L)%`g diff --git a/relationships/admin.py b/relationships/admin.py index eb86724..21df228 100644 --- a/relationships/admin.py +++ b/relationships/admin.py @@ -2,7 +2,8 @@ from django.contrib.auth.models import User from django.contrib.auth.admin import UserAdmin -from relationships.models import Relationship +from relationships.forms import RelationshipStatusAdminForm +from relationships.models import Relationship, RelationshipStatus class RelationshipInline(admin.TabularInline): model = Relationship @@ -13,5 +14,10 @@ class RelationshipInline(admin.TabularInline): class UserRelationshipAdmin(UserAdmin): inlines = (RelationshipInline,) + +class RelationshipStatusAdmin(admin.ModelAdmin): + form = RelationshipStatusAdminForm + admin.site.unregister(User) admin.site.register(User, UserRelationshipAdmin) +admin.site.register(RelationshipStatus, RelationshipStatusAdmin) diff --git a/relationships/forms.py b/relationships/forms.py new file mode 100644 index 0000000..537e646 --- /dev/null +++ b/relationships/forms.py @@ -0,0 +1,39 @@ +from django import forms +from django.db.models import Q +from relationships.models import RelationshipStatus + +class RelationshipStatusAdminForm(forms.ModelForm): + class Meta: + model = RelationshipStatus + + def duplicate_slug_check(self, status_slug): + status_qs = RelationshipStatus.objects.filter( + Q(from_slug=status_slug) | + Q(to_slug=status_slug) | + Q(symmetrical_slug=status_slug)) + if self.instance.pk: + status_qs = status_qs.exclude(pk=self.instance.pk) + if status_qs.count() > 0: + raise forms.ValidationError('"%s" slug already in use on %s' % \ + (status_slug, unicode(status_qs[0]))) + + def clean_from_slug(self): + self.duplicate_slug_check(self.cleaned_data['from_slug']) + return self.cleaned_data['from_slug'] + + def clean_to_slug(self): + self.duplicate_slug_check(self.cleaned_data['to_slug']) + return self.cleaned_data['to_slug'] + + def clean_symmetrical_slug(self): + self.duplicate_slug_check(self.cleaned_data['symmetrical_slug']) + return self.cleaned_data['symmetrical_slug'] + + def clean(self): + if self.errors: + return self.cleaned_data + if self.cleaned_data['from_slug'] == self.cleaned_data['to_slug'] or \ + self.cleaned_data['to_slug'] == self.cleaned_data['symmetrical_slug'] or \ + self.cleaned_data['symmetrical_slug'] == self.cleaned_data['from_slug']: + raise forms.ValidationError('from, to, and symmetrical slugs cannot be the same') + return self.cleaned_data diff --git a/relationships/migrations/0003_slugs_to_charfields.py b/relationships/migrations/0003_slugs_to_charfields.py new file mode 100644 index 0000000..3dcbe76 --- /dev/null +++ b/relationships/migrations/0003_slugs_to_charfields.py @@ -0,0 +1,121 @@ + +from south.db import db +from django.db import models +from relationships.models import * + +class Migration: + + def forwards(self, orm): + + # Deleting unique_together for [symmetrical_slug] on relationshipstatus. + db.delete_unique('relationships_relationshipstatus', ['symmetrical_slug']) + + # Deleting unique_together for [from_slug] on relationshipstatus. + db.delete_unique('relationships_relationshipstatus', ['from_slug']) + + # Deleting unique_together for [to_slug] on relationshipstatus. + db.delete_unique('relationships_relationshipstatus', ['to_slug']) + + # Changing field 'RelationshipStatus.to_slug' + # (to signature: django.db.models.fields.CharField(max_length=100)) + db.alter_column('relationships_relationshipstatus', 'to_slug', orm['relationships.relationshipstatus:to_slug']) + + # Changing field 'RelationshipStatus.symmetrical_slug' + # (to signature: django.db.models.fields.CharField(max_length=100)) + db.alter_column('relationships_relationshipstatus', 'symmetrical_slug', orm['relationships.relationshipstatus:symmetrical_slug']) + + # Changing field 'RelationshipStatus.from_slug' + # (to signature: django.db.models.fields.CharField(max_length=100)) + db.alter_column('relationships_relationshipstatus', 'from_slug', orm['relationships.relationshipstatus:from_slug']) + + + + def backwards(self, orm): + + # Changing field 'RelationshipStatus.to_slug' + # (to signature: django.db.models.fields.SlugField(max_length=50, unique=True, db_index=True)) + db.alter_column('relationships_relationshipstatus', 'to_slug', orm['relationships.relationshipstatus:to_slug']) + + # Changing field 'RelationshipStatus.symmetrical_slug' + # (to signature: django.db.models.fields.SlugField(max_length=50, unique=True, db_index=True)) + db.alter_column('relationships_relationshipstatus', 'symmetrical_slug', orm['relationships.relationshipstatus:symmetrical_slug']) + + # Changing field 'RelationshipStatus.from_slug' + # (to signature: django.db.models.fields.SlugField(max_length=50, unique=True, db_index=True)) + db.alter_column('relationships_relationshipstatus', 'from_slug', orm['relationships.relationshipstatus:from_slug']) + + # Creating unique_together for [to_slug] on relationshipstatus. + db.create_unique('relationships_relationshipstatus', ['to_slug']) + + # Creating unique_together for [from_slug] on relationshipstatus. + db.create_unique('relationships_relationshipstatus', ['from_slug']) + + # Creating unique_together for [symmetrical_slug] on relationshipstatus. + db.create_unique('relationships_relationshipstatus', ['symmetrical_slug']) + + + + models = { + 'auth.group': { + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'unique_together': "(('content_type', 'codename'),)"}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True', 'blank': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False', 'blank': 'True'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False', 'blank': 'True'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'relationships': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'symmetrical': 'False'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'unique_together': "(('app_label', 'model'),)", 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'relationships.relationship': { + 'Meta': {'unique_together': "(('from_user', 'to_user', 'status'),)"}, + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'from_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'from_users'", 'to': "orm['auth.User']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'site': ('django.db.models.fields.related.ForeignKey', [], {'default': '1', 'to': "orm['sites.Site']"}), + 'status': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['relationships.RelationshipStatus']"}), + 'to_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'to_users'", 'to': "orm['auth.User']"}) + }, + 'relationships.relationshipstatus': { + 'from_slug': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'login_required': ('django.db.models.fields.BooleanField', [], {'default': 'False', 'blank': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'private': ('django.db.models.fields.BooleanField', [], {'default': 'False', 'blank': 'True'}), + 'symmetrical_slug': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'to_slug': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'verb': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'sites.site': { + 'Meta': {'db_table': "'django_site'"}, + 'domain': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + } + } + + complete_apps = ['relationships'] diff --git a/relationships/models.py b/relationships/models.py index 7d5f36a..d8d43a5 100644 --- a/relationships/models.py +++ b/relationships/models.py @@ -18,11 +18,11 @@ def blocking(self): class RelationshipStatus(models.Model): name = models.CharField(max_length=100) verb = models.CharField(max_length=100) - from_slug = models.SlugField(unique=True, + from_slug = models.CharField(max_length=100, help_text="Denote the relationship from the user, i.e. 'following'") - to_slug = models.SlugField(unique=True, + to_slug = models.CharField(max_length=100, help_text="Denote the relationship to the user, i.e. 'followers'") - symmetrical_slug = models.SlugField(unique=True, + symmetrical_slug = models.CharField(max_length=100, help_text="When a mutual relationship exists, i.e. 'friends'") login_required = models.BooleanField(default=False, help_text="Users must be logged in to see these relationships") @@ -33,6 +33,7 @@ class RelationshipStatus(models.Model): class Meta: ordering = ('name',) + verbose_name_plural = 'Relationship statuses' def __unicode__(self): return self.name diff --git a/relationships/tests.py b/relationships/tests.py index 80dcbb7..dfbab93 100644 --- a/relationships/tests.py +++ b/relationships/tests.py @@ -1,6 +1,8 @@ +from django import forms from django.test import TestCase from django.contrib.auth.models import User from django.template import Template, Context +from relationships.forms import RelationshipStatusAdminForm from relationships.models import Relationship, RelationshipStatus class RelationshipsTestCase(TestCase): @@ -294,3 +296,66 @@ def test_if_relationship_tag(self): c = Context({'john': self.john, 'yoko': self.yoko}) rendered = t.render(c) self.assertEqual(rendered, 'y') + +class RelationshipStatusAdminFormTestCase(TestCase): + fixtures = ['relationships.json'] + + def setUp(self): + self.walrus = User.objects.get(username='The_Walrus') + self.john = User.objects.get(username='John') + self.paul = User.objects.get(username='Paul') + self.yoko = User.objects.get(username='Yoko') + self.following = RelationshipStatus.objects.get(from_slug='following') + self.blocking = RelationshipStatus.objects.get(from_slug='blocking') + + def test_no_dupes(self): + payload = { + 'name': 'Testing', + 'verb': 'testing', + 'from_slug': 'testing', + 'to_slug': 'testers', + 'symmetrical_slug': 'tests' + } + form = RelationshipStatusAdminForm(payload) + self.assertTrue(form.is_valid()) + test_status = form.save() + + # saving again should work + form = RelationshipStatusAdminForm(payload, instance=test_status) + self.assertTrue(form.is_valid()) + + payload['from_slug'] = 'testers' + payload['to_slug'] = 'testing' + + # saving will work since it will not test against the current instance + form = RelationshipStatusAdminForm(payload, instance=test_status) + self.assertTrue(form.is_valid()) + + # setting the from_slug to the to_slug will raise an error + payload['from_slug'] = 'testers' + payload['to_slug'] = 'testers' + form = RelationshipStatusAdminForm(payload, instance=test_status) + self.assertFalse(form.is_valid()) + + # setting the from_slug to the symmetrical_slug will raise an error + payload['from_slug'] = 'tests' + form = RelationshipStatusAdminForm(payload, instance=test_status) + self.assertFalse(form.is_valid()) + + # setting to a pre-existing from_slug will fail + payload['from_slug'] = 'following' + form = RelationshipStatusAdminForm(payload) + self.assertFalse(form.is_valid()) + self.assertTrue('from_slug' in form.errors) + + # setting the from_slug to a pre-existing to_slug will also fail + payload['from_slug'] = 'followers' + form = RelationshipStatusAdminForm(payload) + self.assertFalse(form.is_valid()) + self.assertTrue('from_slug' in form.errors) + + # setting the from_slug to a pre-existing symetrical_slug will also fail + payload['from_slug'] = 'friends' + form = RelationshipStatusAdminForm(payload) + self.assertFalse(form.is_valid()) + self.assertTrue('from_slug' in form.errors)