manager: make slug conflict resolution simpler (#57138)

src/authentic2/manager/forms.py

@@ -74,10 +74,12 @@ class SlugMixin(forms.ModelForm):
             qs = instance.__class__.objects.all()
             if instance.pk:
                 qs = qs.exclude(pk=instance.pk)
+            new_slug = instance.slug
             i = 1
-            while qs.filter(slug=instance.slug).exists():
-                instance.slug += str(i)
+            while qs.filter(slug=new_slug).exists():
+                new_slug = '%s-%d' % (instance.slug, i)
                 i += 1
+            instance.slug = new_slug
         if len(instance.slug) > 256:
             instance.slug = instance.slug[:252] + hashlib.md5(instance.slug).hexdigest()[:4]
         return super().save(commit=commit)

tests/test_manager.py

@@ -71,7 +71,7 @@ def test_manager_create_ou(superuser_or_admin, app):
     assert 'Old OU' in response
     assert 'New OU' in response
     assert OU.objects.get(name='Old OU').slug == 'new-ou'
-    assert OU.objects.get(name='New OU').slug == 'new-ou1'
+    assert OU.objects.get(name='New OU').slug == 'new-ou-1'
     assert OU.objects.count() == 3
 
 
@@ -100,9 +100,17 @@ def test_manager_create_role(superuser_or_admin, app):
     assert 'New role' in role_list
     assert 'Old role' in role_list
     assert non_admin_roles.count() == 2
-    assert non_admin_roles.get(name='New role').slug == 'new-role1'
+    assert non_admin_roles.get(name='New role').slug == 'new-role-1'
     assert non_admin_roles.get(name='Old role').slug == 'new-role'
 
+    assert non_admin_roles.filter(name='New role').update(name='New role 0')
+    role3_add = app.get(reverse('a2-manager-role-add'))
+    form = role3_add.form
+    form.set('name', 'New role')
+    form.submit().follow()
+    assert non_admin_roles.count() == 3
+    assert non_admin_roles.get(name='New role').slug == 'new-role-2'
+
     # Test multi-ou form
     OU.objects.create(name='New OU', slug='new-ou')
     ou_add = app.get(reverse('a2-manager-role-add'))