Re: [PATCH] models: Add State.slug field
On Sun, 2019-12-08 at 16:00 +, Stephen Finucane wrote: > This reduces a lot of the tech debt we had built up around this. This > field is populated from a slugified representation of State.name and has > a uniqueness constraint. As a result, we also need to a uniqueness > constraint to the State.name field. This shouldn't be an issue and > probably should have been used from day one. > > Note that there is no REST API impact for this since we've been using > state slugs all along. > > Signed-off-by: Stephen Finucane Applied. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] models: Add State.slug field
On 11/12/19 2:32 am, Stephen Finucane wrote: I think the migration can still fail without printing a helpful exception message if two states have names that are different but still slugify to the same thing? Good point. I can fix this if we think it's worth it. Personally, I think the uniqueness check itself is overkill and could probably be removed. Agreed, though if we're gonna do it I want it done thoroughly ;) -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] models: Add State.slug field
On Tue, 2019-12-10 at 15:43 +1100, Andrew Donnellan wrote:
> On 9/12/19 3:00 am, Stephen Finucane wrote:
> > --- /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')
> > +
> > +unique_count = len(State.objects.order_by().values_list(
> > +'name', flat=True).distinct())
> > +total_count = State.objects.count()
> > +
> > +if unique_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 '
> > +'patchw...@ozlabs.org')
>
> I think the migration can still fail without printing a helpful
> exception message if two states have names that are different but still
> slugify to the same thing?
Good point. I can fix this if we think it's worth it. Personally, I
think the uniqueness check itself is overkill and could probably be
removed.
Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] models: Add State.slug field
On 9/12/19 3:00 am, Stephen Finucane wrote:
--- /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')
+
+unique_count = len(State.objects.order_by().values_list(
+'name', flat=True).distinct())
+total_count = State.objects.count()
+
+if unique_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 '
+'patchw...@ozlabs.org')
I think the migration can still fail without printing a helpful
exception message if two states have names that are different but still
slugify to the same thing?
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
