Don't change slug for existing instances fixes #1129 (#1188)

Author: jaredlockhart

app/experimenter/experiments/forms.py

@@ -275,7 +275,7 @@ def clean(self):
                     "The size of all branches must add up to 100"
                 ]
 
-        unique_slugs = set([form.cleaned_data["slug"] for form in alive_forms])
+        unique_slugs = set([form.cleaned_data["name"] for form in alive_forms])
 
         if not len(unique_slugs) == len(alive_forms):
             for form in alive_forms:

app/experimenter/projects/forms.py

@@ -23,8 +23,11 @@ def clean_name(self):
     def clean(self):
         cleaned_data = super().clean()
 
-        name = cleaned_data.get("name")
-        cleaned_data["slug"] = slugify(name)
+        if self.instance.slug:
+            del cleaned_data["slug"]
+        else:
+            name = cleaned_data.get("name")
+            cleaned_data["slug"] = slugify(name)
 
         return cleaned_data
 

app/experimenter/projects/tests/test_forms.py

@@ -1,9 +1,69 @@
+from django import forms
 from django.test import TestCase
 
-from experimenter.projects.forms import ProjectForm
+from experimenter.projects.models import Project
+from experimenter.projects.forms import (
+    ProjectForm,
+    NameSlugFormMixin,
+    UniqueNameSlugFormMixin,
+)
 from experimenter.projects.tests.factories import ProjectFactory
 
 
+class TestNameSlugFormMixin(TestCase):
+
+    def setUp(self):
+
+        class TestForm(NameSlugFormMixin, forms.ModelForm):
+            name = forms.CharField()
+            slug = forms.CharField(required=False)
+
+            class Meta:
+                model = Project
+                fields = ("name", "slug")
+
+        self.Form = TestForm
+
+    def test_sets_slug_for_new_instance(self):
+        form = self.Form({"name": "New Name"})
+        self.assertTrue(form.is_valid())
+        project = form.save()
+        self.assertEqual(project.slug, "new-name")
+
+    def test_doesnt_set_slug_for_existing_instance(self):
+        project = ProjectFactory.create(name="Old Name", slug="old-slug")
+        form = self.Form({"name": "New Name"}, instance=project)
+        self.assertTrue(form.is_valid())
+        project = form.save()
+        self.assertEqual(project.name, "New Name")
+        self.assertEqual(project.slug, "old-slug")
+
+
+class TestUniqueNameSlugFormMixin(TestCase):
+
+    def setUp(self):
+
+        class TestForm(UniqueNameSlugFormMixin, forms.ModelForm):
+            name = forms.CharField()
+            slug = forms.CharField(required=False)
+
+            class Meta:
+                model = Project
+                fields = ("name", "slug")
+
+        self.Form = TestForm
+
+    def test_valid_for_no_slug_match(self):
+        form = self.Form({"name": "New Name"})
+        self.assertTrue(form.is_valid())
+
+    def test_invalid_for_existing_slug_match(self):
+        ProjectFactory(name="Unique Existing Name", slug="existing-slug")
+        form = self.Form({"name": "Existing Slug"})
+        self.assertFalse(form.is_valid())
+        self.assertIn("name", form.errors)
+
+
 class TestProjectForm(TestCase):
 
     def test_project_form_creates_project(self):

app/experimenter/projects/tests/test_views.py

@@ -54,7 +54,6 @@ def test_project_update_view_updates_project(self):
         updated_project = Project.objects.get(id=project.id)
 
         self.assertEqual(updated_project.name, project_data["name"])
-        self.assertEqual(updated_project.slug, "new-name")
         self.assertRedirects(
             response,
             reverse("projects-detail", kwargs={"slug": updated_project.slug}),