Allow editing of group non-chartered group descriptions through UI. Fixes ietf-tools#3388. Commit ready for merge.

jennifer-richards · jennifer-richards · commit d4dbb1bc1ff4 · 2022-01-12T21:04:13.000Z
- Legacy-Id: 19838
diff --git a/ietf/group/forms.py b/ietf/group/forms.py
@@ -66,6 +66,7 @@ class GroupForm(forms.Form):
     list_email = forms.CharField(max_length=64, required=False)
     list_subscribe = forms.CharField(max_length=255, required=False)
     list_archive = forms.CharField(max_length=255, required=False)
+    description = forms.CharField(widget=forms.Textarea, required=False, help_text='Text that appears on the "about" page.')
     urls = forms.CharField(widget=forms.Textarea, label="Additional URLs", help_text="Format: https://site/path (Optional description). Separate multiple entries with newline. Prefer HTTPS URLs where possible.", required=False)
     resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", help_text="Format: tag value (Optional description). Separate multiple entries with newline. Prefer HTTPS URLs where possible.", required=False)
     closing_note = forms.CharField(widget=forms.Textarea, label="Closing note", required=False)
@@ -103,6 +104,9 @@ def __init__(self, *args, **kwargs):
 
         super(self.__class__, self).__init__(*args, **kwargs)
 
+        if not group_features or group_features.has_chartering_process:
+            self.fields.pop('description')  # do not show the description field for chartered groups
+
         for role_slug in self.used_roles:
             role_name = RoleName.objects.get(slug=role_slug)
             fieldname = '%s_roles'%role_slug
diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py
@@ -8,6 +8,7 @@
 import io
 import bleach
 
+from unittest.mock import patch
 from pathlib import Path
 from pyquery import PyQuery
 from tempfile import NamedTemporaryFile
@@ -515,12 +516,16 @@ def test_create(self):
         self.assertTrue(len(q('form .has-error')) > 0)
 
         # Ok creation
-        r = self.client.post(url, dict(acronym="testwg", name="Testing WG", state=bof_state.pk, parent=area.pk))
+        r = self.client.post(
+            url,
+            dict(acronym="testwg", name="Testing WG", state=bof_state.pk, parent=area.pk, description="ignored"),
+        )
         self.assertEqual(r.status_code, 302)
         self.assertEqual(len(Group.objects.filter(type="wg")), num_wgs + 1)
         group = Group.objects.get(acronym="testwg")
         self.assertEqual(group.name, "Testing WG")
         self.assertEqual(charter_name_for_group(group), "charter-ietf-testwg")
+        self.assertEqual(group.description, '', 'Description should be ignored for a WG')
 
     def test_create_rg(self):
 
@@ -579,6 +584,28 @@ def test_create_based_on_existing_bof(self):
 #         self.assertEqual(Group.objects.get(acronym=group.acronym).state_id, "proposed")
 #         self.assertEqual(Group.objects.get(acronym=group.acronym).name, "Test")
 
+
+    def test_create_non_chartered_includes_description(self):
+        parent = GroupFactory(type_id='area')
+        group_type = GroupTypeName.objects.filter(used=True, features__has_chartering_process=False).first()
+        self.assertIsNotNone(group_type)
+        url = urlreverse('ietf.group.views.edit', kwargs=dict(group_type=group_type.slug, action="create"))
+        login_testing_unauthorized(self, "secretary", url)
+        r = self.client.post(
+            url,
+            {
+                'acronym': "testgrp",
+                'name': "Testing",
+                'state': GroupStateName.objects.get(slug='active').pk,
+                'parent': parent.pk,
+                'description': "not ignored",
+            },
+        )
+        self.assertEqual(r.status_code, 302)
+        group = Group.objects.get(acronym="testgrp")
+        self.assertEqual(group.name, "Testing")
+        self.assertEqual(group.description, 'not ignored', 'Description should not be ignored')
+
     def test_edit_info(self):
         group = GroupFactory(acronym='mars',parent=GroupFactory(type_id='area'))
         CharterFactory(group=group)
@@ -640,6 +667,7 @@ def test_edit_info(self):
                                   list_email="mars@mail",
                                   list_subscribe="subscribe.mars",
                                   list_archive="archive.mars",
+                                  description='ignored'
                                   ))
         self.assertEqual(r.status_code, 302)
 
@@ -658,6 +686,7 @@ def test_edit_info(self):
         self.assertEqual(group.list_email, "mars@mail")
         self.assertEqual(group.list_subscribe, "subscribe.mars")
         self.assertEqual(group.list_archive, "archive.mars")
+        self.assertEqual(group.description, '')
 
         self.assertTrue((Path(settings.CHARTER_PATH) / ("%s-%s.txt" % (group.charter.canonical_name(), group.charter.rev))).exists())
         self.assertEqual(len(outbox), 2)
@@ -843,6 +872,60 @@ def test_edit_reviewers(self):
         self.assertEqual(review_assignment.state_id, 'accepted')
         self.assertEqual(other_review_assignment.state_id, 'assigned')
 
+    def test_edit_info_non_chartered_includes_description(self):
+        group_type = GroupTypeName.objects.filter(used=True, features__has_chartering_process=False).first()
+        self.assertIsNotNone(group_type)
+        group = GroupFactory(type_id=group_type.pk, description='Original description')
+        url = urlreverse('ietf.group.views.edit', kwargs={'acronym': group.acronym, 'action': 'edit'})
+        PersonFactory(user__username='plain')
+        self.client.login(username='plain', password='plain+password')
+
+        # mock the auth check so we don't have to delve into details of GroupFeatures for testing
+        with patch('ietf.group.views.can_manage_group', return_value=True):
+            r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        q = PyQuery(r.content)
+        self.assertTrue(q('textarea[name="description"]'))
+
+        with patch('ietf.group.views.can_manage_group', return_value=True):
+            r = self.client.post(url, {
+                'name': group.name,
+                'acronym': group.acronym,
+                'state': group.state.pk,
+                'description': 'Updated description',
+            })
+        self.assertEqual(r.status_code, 302)
+        group = Group.objects.get(pk=group.pk)  # refresh
+        self.assertEqual(group.description, 'Updated description')
+
+    def test_edit_description_field(self):
+        group_type = GroupTypeName.objects.filter(used=True, features__has_chartering_process=False).first()
+        self.assertIsNotNone(group_type)
+        group = GroupFactory(type_id=group_type.pk, description='Original description')
+        url = urlreverse('ietf.group.views.edit',
+                         kwargs={'acronym': group.acronym, 'action': 'edit', 'field': 'description'})
+        PersonFactory(user__username='plain')
+        self.client.login(username='plain', password='plain+password')
+
+        # mock the auth check so we don't have to delve into details of GroupFeatures for testing
+        with patch('ietf.group.views.can_manage_group', return_value=True):
+            r = self.client.post(url, {
+                'description': 'Updated description',
+            })
+        self.assertEqual(r.status_code, 302)
+        group = Group.objects.get(pk=group.pk)  # refresh
+        self.assertEqual(group.description, 'Updated description')
+
+        # Convert the group to a chartered type and repeat - should no longer be able to edit the desc
+        group.type = GroupTypeName.objects.filter(used=True, features__has_chartering_process=True).first()
+        group.save()
+        with patch('ietf.group.views.can_manage_group', return_value=True):
+            r = self.client.post(url, {
+                'description': 'Ignored description',
+            })
+        self.assertEqual(r.status_code, 302)
+        group = Group.objects.get(pk=group.pk)  # refresh
+        self.assertEqual(group.description, 'Updated description')
 
     def test_conclude(self):
         group = GroupFactory(acronym="mars")
@@ -1037,6 +1120,32 @@ def test_need_parent(self):
         self.assertTrue(form.is_valid())
         self._assert_cleaned_data_equal(form.cleaned_data, data)
 
+    def test_no_description_field_for_chartered_groups(self):
+        group = GroupFactory()
+        self.assertTrue(
+            group.features.has_chartering_process,
+            'Group type must have has_chartering_process=True for this test',
+        )
+        self.assertNotIn('description', GroupForm(group=group).fields)
+        self.assertNotIn('description', GroupForm(group_type=group.type).fields)
+        self.assertNotIn('description', GroupForm(group=group, group_type=group.type).fields)
+        self.assertNotIn('description', GroupForm(data={'description': 'blah'}, group=group).fields)
+        self.assertNotIn('description', GroupForm(data={'description': 'blah'}, group_type=group.type).fields)
+        self.assertNotIn('description', GroupForm(data={'description': 'blah'}, group=group, group_type=group.type).fields)
+
+    def test_have_description_field_for_non_chartered_groups(self):
+        group = GroupFactory(type_id='dir')
+        self.assertFalse(
+            group.features.has_chartering_process,
+            'Group type must have has_chartering_process=False for this test',
+        )
+        self.assertIn('description', GroupForm(group=group).fields)
+        self.assertIn('description', GroupForm(group_type=group.type).fields)
+        self.assertIn('description', GroupForm(group=group, group_type=group.type).fields)
+        self.assertIn('description', GroupForm(data={'description': 'blah'}, group=group).fields)
+        self.assertIn('description', GroupForm(data={'description': 'blah'}, group_type=group.type).fields)
+        self.assertIn('description', GroupForm(data={'description': 'blah'}, group=group, group_type=group.type).fields)
+
 
 class MilestoneTests(TestCase):
     def create_test_milestones(self):
diff --git a/ietf/group/views.py b/ietf/group/views.py
@@ -884,7 +884,7 @@ def format_resources(resources, fs="\n"):
         return fs.join(res)
 
     def diff(attr, name):
-        if field and attr != field:
+        if attr not in clean or (field and attr != field):
             return
         v = getattr(group, attr)
         if clean[attr] != v:
@@ -951,6 +951,7 @@ def diff(attr, name):
             diff('name', "Name")
             diff('acronym', "Acronym")
             diff('state', "State")
+            diff('description', "Description")
             diff('parent', "IETF Area" if group.type=="wg" else "Group parent")
             diff('list_email', "Mailing list email")
             diff('list_subscribe', "Mailing list subscribe address")
@@ -1067,6 +1068,7 @@ def diff(attr, name):
             init = dict(name=group.name,
                         acronym=group.acronym,
                         state=group.state,
+                        description = group.description,
                         parent=group.parent.id if group.parent else None,
                         list_email=group.list_email if group.list_email else None,
                         list_subscribe=group.list_subscribe if group.list_subscribe else None,
diff --git a/ietf/templates/group/group_about.html b/ietf/templates/group/group_about.html
@@ -246,7 +246,12 @@ <h2>Charter for {% if group.state_id == "proposed" %}proposed{% endif %} {{ grou
     {# the linebreaks filter adds <p/>, no surrounding <p/> necessary: #}
     {{ group.charter_text|linebreaks }}
   {% else %}
-    <h2>About</h2>
+    <h2>
+      About
+      {% if can_edit_group %}
+        <a class="btn btn-default btn-xs" href="{% url 'ietf.group.views.edit' acronym=group.acronym field='description' %}">Edit</a>&nbsp;
+      {% endif %}
+    </h2>
     {% comment %}{{ group.description|default:"No description yet."|linebreaks }}{% endcomment %}
     {{ group.description|default:"No description yet."| apply_markup:"restructuredtext" }}
   {% endif %}
