From 404d1159348f0ff86e0c34ccb35f986b7103d2c4 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Mon, 12 Apr 2021 17:08:58 +0000 Subject: [PATCH] Merged in [18936] from rjsparks@nostrum.com: 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 #3236. Commit - Legacy-Id: 18945 Note: SVN reference [18936] has been migrated to Git commit 04f5b4ae4dcbbf8a39281f0b79b1a6271439fd1a --- ietf/group/admin.py | 12 +++++++++-- ietf/group/factories.py | 3 +++ ietf/group/forms.py | 8 ++++++-- ietf/group/tests_info.py | 44 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/ietf/group/admin.py b/ietf/group/admin.py index d3e075145..a4aec0883 100644 --- a/ietf/group/admin.py +++ b/ietf/group/admin.py @@ -5,6 +5,8 @@ import re from functools import update_wrapper +import debug # pyflakes:ignore + from django import forms from django.contrib import admin @@ -40,8 +42,14 @@ class GroupForm(forms.ModelForm): 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): diff --git a/ietf/group/factories.py b/ietf/group/factories.py index 125dd74a5..911628ba6 100644 --- a/ietf/group/factories.py +++ b/ietf/group/factories.py @@ -3,6 +3,8 @@ import datetime 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 GroupFactory(factory.DjangoModelFactory): 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): diff --git a/ietf/group/forms.py b/ietf/group/forms.py index 441631724..05932a79b 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -148,8 +148,12 @@ class GroupForm(forms.Form): 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) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 8de4c7eb6..4144645d0 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -17,6 +17,7 @@ import debug # pyflakes:ignore 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.community.utils import reset_name_contains_index_for_rule 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 @@ class GroupParentLoopTests(TestCase): # 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()) +