Retain strict acronym validation, specifically disallowing hyphens, for new groups of types that create documents, while allowing existing groups and new non-document-creating groups to validate when they contain hyphens. Fixes ietf-tools#3236. Commitready for merge.

 - Legacy-Id: 18936

Author: rjsparks

ietf/group/admin.py

@@ -5,6 +5,8 @@
 
 from functools import update_wrapper
 
+import debug # pyflakes:ignore
+
 from django import forms
 
 from django.contrib import admin
@@ -40,8 +42,14 @@ def clean_acronym(self):
             See ietf.group.forms.GroupForm.clean_acronym()
         '''
         acronym = self.cleaned_data['acronym'].strip().lower()
-        if not re.match(r'^[a-z][a-z0-9]+$', acronym):
-            raise forms.ValidationError("Acronym is invalid, must be at least two characters and only contain lowercase letters and numbers starting with a letter.")
+        if not self.instance.pk:
+            type = self.cleaned_data['type']
+            if GroupFeatures.objects.get(type=type).has_documents:
+                if not re.match(r'^[a-z][a-z0-9]+$', acronym):
+                    raise forms.ValidationError("Acronym is invalid, for groups that create documents, the acronym must be at least two characters and only contain lowercase letters and numbers starting with a letter.")
+            else:
+                if not re.match(r'^[a-z][a-z0-9-]*[a-z0-9]$', acronym):
+                    raise forms.ValidationError("Acronym is invalid, must be at least two characters and only contain lowercase letters and numbers starting with a letter. It may contain hyphens, but that is discouraged.")
         return acronym
 
     def clean_used_roles(self):

ietf/group/factories.py

@@ -3,6 +3,8 @@
 import debug # pyflakes:ignore
 import factory
 
+from typing import List    # pyflakes:ignore
+
 from ietf.group.models import Group, Role, GroupEvent, GroupMilestone
 from ietf.review.factories import ReviewTeamSettingsFactory
 
@@ -17,6 +19,7 @@ class Meta:
     type_id = 'wg'
     list_email = factory.LazyAttribute(lambda a: '%s@ietf.org'% a.acronym)
     uses_milestone_dates = True
+    used_roles = [] # type: List[str]
 
     @factory.lazy_attribute
     def parent(self):

ietf/group/forms.py

@@ -148,8 +148,12 @@ def clean_acronym(self):
 
         acronym = self.cleaned_data['acronym'].strip().lower()
 
-        if not re.match(r'^[a-z][a-z0-9]+$', acronym):
-            raise forms.ValidationError("Acronym is invalid, must be at least two characters and only contain lowercase letters and numbers starting with a letter.")
+        if self.group_type and GroupFeatures.objects.get(type=self.group_type).has_documents:
+            if not re.match(r'^[a-z][a-z0-9]+$', acronym):
+                raise forms.ValidationError("Acronym is invalid, for groups that create documents, the acronym must be at least two characters and only contain lowercase letters and numbers starting with a letter.")
+        else:
+            if not re.match(r'^[a-z][a-z0-9-]*[a-z0-9]$', acronym):
+                raise forms.ValidationError("Acronym is invalid, must be at least two characters and only contain lowercase letters and numbers starting with a letter. It may contain hyphens, but that is discouraged.")
 
         # be careful with acronyms, requiring confirmation to take existing or override historic
         existing = Group.objects.filter(acronym__iexact=acronym)

ietf/group/tests_info.py

@@ -17,6 +17,7 @@
 from django.conf import settings
 from django.urls import reverse as urlreverse
 from django.urls import NoReverseMatch
+from django.utils import timezone
 from django.contrib.auth.models import User
 
 from django.utils.html import escape
@@ -26,8 +27,10 @@
 from ietf.doc.factories import WgDraftFactory, CharterFactory, BallotDocEventFactory
 from ietf.doc.models import Document, DocAlias, DocEvent, State
 from ietf.doc.utils_charter import charter_name_for_group
+from ietf.group.admin import GroupForm as AdminGroupForm
 from ietf.group.factories import (GroupFactory, RoleFactory, GroupEventFactory, 
     DatedGroupMilestoneFactory, DatelessGroupMilestoneFactory)
+from ietf.group.forms import GroupForm
 from ietf.group.models import Group, GroupEvent, GroupMilestone, GroupStateTransitions, Role
 from ietf.group.utils import save_group_in_history, setup_default_community_list_for_group
 from ietf.meeting.factories import SessionFactory
@@ -1638,3 +1641,44 @@ def timeout_handler(signum, frame):
 
         # If we get here, then there is not an infinite loop
         return
+
+class AcronymValidationTests(TestCase):
+
+    def test_admin_acronym_validation(self):
+        now = timezone.now()
+        form = AdminGroupForm({'acronym':'shouldpass','name':'should pass','type':'wg','state':'active','used_roles':'[]','time':now})
+        self.assertTrue(form.is_valid())
+        form = AdminGroupForm({'acronym':'should-fail','name':'should fail','type':'wg','state':'active','used_roles':'[]','time':now})
+        self.assertIn('acronym',form.errors)
+        form = AdminGroupForm({'acronym':'f','name':'should fail','type':'wg','state':'active','used_roles':'[]','time':now})
+        self.assertIn('acronym',form.errors)
+        # For the ITU we have a heirarchy of group names that use hyphens as delimeters
+        form = AdminGroupForm({'acronym':'should-pass','name':'should pass','type':'sdo','state':'active','used_roles':'[]','time':now})
+        self.assertTrue(form.is_valid())
+        form = AdminGroupForm({'acronym':'shouldfail-','name':'should fail','type':'wg','state':'active','used_roles':'[]','time':now})
+        self.assertIn('acronym',form.errors)
+        form = AdminGroupForm({'acronym':'-shouldfail','name':'should fail','type':'wg','state':'active','used_roles':'[]','time':now})
+        self.assertIn('acronym',form.errors)
+
+        wg = GroupFactory(acronym='bad-idea', type_id='wg') # There are some existing wg and programs with hyphens in their acronyms.
+        form = AdminGroupForm({'acronym':wg.acronym,'name':wg.name,'type':wg.type_id,'state':wg.state_id,'used_roles':str(wg.used_roles),'time':now},instance=wg)
+        self.assertTrue(form.is_valid())
+
+    def test_groupform_acronym_validation(self):
+        form = GroupForm({'acronym':'shouldpass','name':'should pass','state':'active'},group_type='wg')
+        self.assertTrue(form.is_valid())
+        form = GroupForm({'acronym':'should-fail','name':'should fail','state':'active'},group_type='wg')
+        self.assertIn('acronym',form.errors)
+        form = GroupForm({'acronym':'f','name':'should fail','state':'active'},group_type='wg')
+        self.assertIn('acronym',form.errors)
+        form = GroupForm({'acronym':'should-pass','name':'should pass','state':'active'},group_type='sdo')
+        self.assertTrue(form.is_valid())
+        form = GroupForm({'acronym':'shouldfail-','name':'should fail','state':'active'},group_type='sdo')
+        self.assertIn('acronym',form.errors)
+        form = GroupForm({'acronym':'-shouldfail','name':'should fail','state':'active'},group_type='sdo')
+        self.assertIn('acronym',form.errors)
+
+        wg = GroupFactory(acronym='bad-idea', type_id='wg') 
+        form = GroupForm({'acronym':wg.acronym,'name':wg.name,'state':wg.state_id},group=wg, group_type=wg.type_id)
+        self.assertTrue(form.is_valid())
+