Summary: Get rid of submit_initial_charter hack, only create a charter

when a first revision has actually been submitted.
 - Legacy-Id: 10136

Author: OleLaursen

ietf/doc/tests_charter.py

@@ -9,7 +9,8 @@
 
 from ietf.doc.models import ( Document, State, BallotDocEvent, BallotType, NewRevisionDocEvent,
     TelechatDocEvent, WriteupDocEvent )
-from ietf.doc.utils_charter import next_revision, default_review_text, default_action_text
+from ietf.doc.utils_charter import ( next_revision, default_review_text, default_action_text,
+    charter_name_for_group )
 from ietf.group.models import Group, GroupMilestone
 from ietf.iesg.models import TelechatDate
 from ietf.person.models import Person
@@ -270,6 +271,40 @@ def test_submit_charter(self):
             self.assertEqual(f.read(),
                               "Windows line\nMac line\nUnix line\n" + utf_8_snippet)
 
+    def test_submit_initial_charter(self):
+        make_test_data()
+
+        group = Group.objects.get(acronym="mars")
+        # get rid of existing charter
+        charter = group.charter
+        group.charter = None
+        group.save()
+        charter.delete()
+        charter = None
+
+        url = urlreverse('charter_submit', kwargs=dict(name=charter_name_for_group(group)))
+        login_testing_unauthorized(self, "secretary", url)
+
+        # normal get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        q = PyQuery(r.content)
+        self.assertEqual(len(q('form input[name=txt]')), 1)
+
+        # create charter
+        test_file = StringIO("Simple test")
+        test_file.name = "unnamed"
+
+        r = self.client.post(url, dict(txt=test_file))
+        self.assertEqual(r.status_code, 302)
+
+        charter = Document.objects.get(name="charter-ietf-%s" % group.acronym)
+        self.assertEqual(charter.rev, "00-00")
+        self.assertTrue("new_revision" in charter.latest_event().type)
+
+        group = Group.objects.get(pk=group.pk)
+        self.assertEqual(group.charter, charter)
+
     def test_edit_announcement_text(self):
         draft = make_test_data()
         charter = draft.group.charter

ietf/doc/utils_charter.py

@@ -21,6 +21,10 @@ def charter_name_for_group(group):
 
     return "charter-%s-%s" % (top_org, group.acronym)
 
+def split_charter_name(charter_name):
+    top_org, group_acronym = charter_name.split("-")[1:]
+    return top_org, group_acronym
+
 def next_revision(rev):
     if rev == "":
         return "00-00"

ietf/doc/views_charter.py

@@ -1,7 +1,7 @@
 import os, datetime, textwrap, json
 
 from django.http import HttpResponseRedirect, HttpResponseNotFound, HttpResponseForbidden, Http404
-from django.shortcuts import render_to_response, get_object_or_404, redirect
+from django.shortcuts import render_to_response, get_object_or_404, redirect, render
 from django.core.urlresolvers import reverse as urlreverse
 from django.template import RequestContext
 from django import forms
@@ -12,16 +12,17 @@
 
 import debug                            # pyflakes:ignore
 
-from ietf.doc.models import ( Document, DocHistory, State, DocEvent, BallotDocEvent,
-    BallotPositionDocEvent, InitialReviewDocEvent, NewRevisionDocEvent,
+from ietf.doc.models import ( Document, DocAlias, DocHistory, State, DocEvent,
+    BallotDocEvent, BallotPositionDocEvent, InitialReviewDocEvent, NewRevisionDocEvent,
     WriteupDocEvent )
 from ietf.doc.utils import ( add_state_change_event, close_open_ballots,
     create_ballot_if_not_open, get_chartering_type )
 from ietf.doc.utils_charter import ( historic_milestones_for_charter,
     approved_revision, default_review_text, default_action_text, email_state_changed,
     generate_ballot_writeup, generate_issue_ballot_mail, next_revision,
-    change_group_state_after_charter_approval, fix_charter_revision_after_approval)
-from ietf.group.models import ChangeStateGroupEvent, MilestoneGroupEvent
+    change_group_state_after_charter_approval, fix_charter_revision_after_approval,
+    split_charter_name)
+from ietf.group.models import Group, ChangeStateGroupEvent, MilestoneGroupEvent
 from ietf.group.utils import save_group_in_history, save_milestone_in_history, can_manage_group_type
 from ietf.ietfauth.utils import has_role, role_required
 from ietf.name.models import GroupStateName
@@ -346,22 +347,31 @@ def save(self, group, rev):
                 destination.write(self.cleaned_data['content'].encode("utf-8"))
 
 @login_required
-def submit(request, name=None, option=None):
+def submit(request, name, option=None):
     if not name.startswith('charter-'):
         raise Http404
 
-    charter = get_object_or_404(Document, type="charter", name=name)
-    group = charter.group
+    charter = Document.objects.filter(type="charter", name=name).first()
+    if charter:
+        group = charter.group
+        charter_canonical_name = charter.canonical_name()
+        charter_rev = charter.rev
+    else:
+        top_org, group_acronym = split_charter_name(name)
+        group = get_object_or_404(Group, acronym=group_acronym)
+        charter_canonical_name = name
+        charter_rev = "00-00"
 
-    if not can_manage_group_type(request.user, group.type_id):
+    if not can_manage_group_type(request.user, group.type_id) or not group.features.has_chartering_process:
         return HttpResponseForbidden("You don't have permission to access this view")
 
-    path = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter.canonical_name(), charter.rev))
-    not_uploaded_yet = charter.rev.endswith("-00") and not os.path.exists(path)
 
-    if not_uploaded_yet:
+    path = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter_canonical_name, charter_rev))
+    not_uploaded_yet = charter_rev.endswith("-00") and not os.path.exists(path)
+
+    if not_uploaded_yet or not charter:
         # this case is special - we recently chartered or rechartered and have no file yet
-        next_rev = charter.rev
+        next_rev = charter_rev
     else:
         # search history for possible collisions with abandoned efforts
         prev_revs = list(charter.history_set.order_by('-time').values_list('rev', flat=True))
@@ -375,7 +385,23 @@ def submit(request, name=None, option=None):
             # Also save group history so we can search for it
             save_group_in_history(group)
 
-            charter.rev = next_rev
+            if not charter:
+                charter = Document.objects.create(
+                    name=name,
+                    type_id="charter",
+                    title=group.name,
+                    group=group,
+                    abstract=group.name,
+                    rev=next_rev,
+                )
+                DocAlias.objects.create(name=charter.name, document=charter)
+
+                charter.set_state(State.objects.get(used=True, type="charter", slug="notrev"))
+
+                group.charter = charter
+                group.save()
+            else:
+                charter.rev = next_rev
 
             events = []
             e = NewRevisionDocEvent(doc=charter, by=request.user.person, type="new_revision")
@@ -397,30 +423,30 @@ def submit(request, name=None, option=None):
             else:
                 return redirect("doc_view", name=charter.name)
     else:
-        init = { "content": ""}
-        c = charter
+        init = { "content": "" }
 
-        if not_uploaded_yet:
+        if not_uploaded_yet and charter:
             # use text from last approved revision
             last_approved = charter.rev.split("-")[0]
-            h = charter.history_set.filter(rev=last_approved).order_by("-time", "-id")
+            h = charter.history_set.filter(rev=last_approved).order_by("-time", "-id").first()
             if h:
-                c = h[0]
+                charter_canonical_name = h.canonical_name()
+                charter_rev = h.rev
 
-        filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (c.canonical_name(), c.rev))
+        filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter_canonical_name, charter_rev))
 
         try:
             with open(filename, 'r') as f:
                 init["content"] = f.read()
         except IOError:
             pass
         form = UploadForm(initial=init)
-    return render_to_response('doc/charter/submit.html',
-                              {'form': form,
-                               'next_rev': next_rev,
-                               'group': group,
-                               'name': name },
-                              context_instance=RequestContext(request))
+    return render(request, 'doc/charter/submit.html', {
+        'form': form,
+        'next_rev': next_rev,
+        'group': group,
+        'name': name,
+    })
 
 class AnnouncementTextForm(forms.Form):
     announcement_text = forms.CharField(widget=forms.Textarea, required=True)

ietf/group/edit.py

@@ -11,7 +11,7 @@
 
 import debug                            # pyflakes:ignore
 
-from ietf.doc.models import Document, DocAlias, DocTagName, State
+from ietf.doc.models import DocTagName, State
 from ietf.doc.utils import get_tags_for_stream_id
 from ietf.doc.utils_charter import charter_name_for_group
 from ietf.group.models import ( Group, Role, GroupEvent, GroupHistory, GroupStateName,
@@ -144,57 +144,6 @@ def format_urls(urls, fs="\n"):
             res.append(u.url)
     return fs.join(res)
 
-def get_or_create_initial_charter(group, group_type):
-    charter_name = charter_name_for_group(group)
-
-    try:
-        charter = Document.objects.get(docalias__name=charter_name)
-    except Document.DoesNotExist:
-        charter = Document.objects.create(
-            name=charter_name,
-            type_id="charter",
-            title=group.name,
-            group=group,
-            abstract=group.name,
-            rev="00-00",
-        )
-        charter.set_state(State.objects.get(used=True, type="charter", slug="notrev"))
-
-        # Create an alias as well
-        DocAlias.objects.create(name=charter.name, document=charter)
-
-    return charter
-
-@login_required
-def submit_initial_charter(request, group_type=None, acronym=None):
-
-    # This needs refactoring.
-    # The signature assumed you could have groups with the same name, but with different types, which we do not allow.
-    # Consequently, this can be called with an existing group acronym and a type 
-    # that doesn't match the existing group type. The code below essentially ignores the group_type argument.
-    #
-    # If possible, the use of get_or_create_initial_charter should be moved
-    # directly into charter_submit, and this function should go away.
-
-    if acronym==None:
-        raise Http404
-
-    group = get_object_or_404(Group, acronym=acronym)
-    if not group.features.has_chartering_process:
-        raise Http404
-
-    # This is where we start ignoring the passed in group_type
-    group_type = group.type_id
-
-    if not can_manage_group_type(request.user, group_type):
-        return HttpResponseForbidden("You don't have permission to access this view")
-
-    if not group.charter:
-        group.charter = get_or_create_initial_charter(group, group_type)
-        group.save()
-
-    return redirect('charter_submit', name=group.charter.name, option="initcharter")
-
 @login_required
 def edit(request, group_type=None, acronym=None, action="edit"):
     """Edit or create a group, notifying parties as
@@ -241,9 +190,6 @@ def edit(request, group_type=None, acronym=None, action="edit"):
                 save_group_in_history(group)
 
 
-            if action == "charter" and not group.charter:  # make sure we have a charter
-                group.charter = get_or_create_initial_charter(group, group_type)
-
             changes = []
 
             def desc(attr, new, old):
@@ -323,7 +269,7 @@ def diff(attr, name):
             group.save()
 
             if action=="charter":
-                return redirect('charter_submit', name=group.charter.name, option="initcharter")
+                return redirect('charter_submit', name=charter_name_for_group(group), option="initcharter")
 
             return HttpResponseRedirect(group.about_url())
     else: # form.is_valid()

ietf/group/info.py

@@ -50,6 +50,7 @@
 from ietf.doc.views_search import SearchForm, retrieve_search_results, get_doc_is_tracked
 from ietf.doc.models import Document, State, DocAlias, RelatedDocument
 from ietf.doc.utils import get_chartering_type
+from ietf.doc.utils_charter import charter_name_for_group
 from ietf.doc.templatetags.ietf_filters import clean_whitespace
 from ietf.group.models import Group, Role, ChangeStateGroupEvent
 from ietf.name.models import GroupTypeName
@@ -471,13 +472,17 @@ def group_about(request, acronym, group_type=None):
     requested_close = group.state_id != "conclude" and e and e.type == "requested_close"
 
     can_manage = can_manage_group_type(request.user, group.type_id)
+    charter_submit_url = ""
+    if group.features.has_chartering_process:
+        charter_submit_url = urlreverse("charter_submit", kwargs={ "name": charter_name_for_group(group) })
 
     return render(request, 'group/group_about.html',
                   construct_group_menu_context(request, group, "charter" if group.features.has_chartering_process else "about", group_type, {
                       "milestones_in_review": group.groupmilestone_set.filter(state="review"),
                       "milestone_reviewer": milestone_reviewer_for_group_type(group_type),
                       "requested_close": requested_close,
                       "can_manage": can_manage,
+                      "charter_submit_url": charter_submit_url
                   }))
 
 

ietf/group/tests_info.py

@@ -13,6 +13,7 @@
 from django.core.urlresolvers import NoReverseMatch
 
 from ietf.doc.models import Document, DocAlias, DocEvent, State
+from ietf.doc.utils_charter import charter_name_for_group
 from ietf.group.models import Group, GroupEvent, GroupMilestone, GroupStateTransitions, MilestoneGroupEvent
 from ietf.group.utils import save_group_in_history
 from ietf.name.models import DocTagName, GroupStateName, GroupTypeName
@@ -377,8 +378,7 @@ def test_create(self):
         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(group.charter.name, "charter-ietf-testwg")
-        self.assertEqual(group.charter.rev, "00-00")
+        self.assertEqual(charter_name_for_group(group), "charter-ietf-testwg")
 
     def test_create_rg(self):
 
@@ -404,9 +404,7 @@ def test_create_rg(self):
         self.assertEqual(len(Group.objects.filter(type="rg")), num_rgs + 1)
         group = Group.objects.get(acronym="testrg")
         self.assertEqual(group.name, "Testing RG")
-        self.assertEqual(group.charter.name, "charter-irtf-testrg")
-        self.assertEqual(group.charter.rev, "00-00")
-        self.assertEqual(group.parent.acronym,'irtf')
+        self.assertEqual(charter_name_for_group(group), "charter-irtf-testrg")
 
     def test_create_based_on_existing_bof(self):
         make_test_data()
@@ -513,19 +511,6 @@ def test_edit_info(self):
         self.assertEqual(group.groupurl_set.all()[0].name, "MARS site")
         self.assertTrue(os.path.exists(os.path.join(self.charter_dir, "%s-%s.txt" % (group.charter.canonical_name(), group.charter.rev))))
 
-    def test_initial_charter(self):
-        make_test_data()
-        group = Group.objects.get(acronym="mars")
-        for url in [ urlreverse('ietf.group.edit.submit_initial_charter', kwargs={'acronym':group.acronym}),
-                     urlreverse('ietf.group.edit.submit_initial_charter', kwargs={'acronym':group.acronym,'group_type':group.type_id}),
-                   ]:
-            login_testing_unauthorized(self, "secretary", url)
-            r = self.client.get(url,follow=True)
-            self.assertEqual(r.status_code,200) 
-            self.assertTrue(r.redirect_chain[0][0].endswith(urlreverse('charter_submit',kwargs={'name':group.charter.name,'option':'initcharter'})))
-            self.client.logout()
-                    
-
     def test_conclude(self):
         make_test_data()
 

ietf/group/urls_info_details.py

@@ -9,7 +9,6 @@
     (r'^history/$','ietf.group.info.history'),
     (r'^deps/dot/$', 'ietf.group.info.dependencies_dot'),
     (r'^deps/pdf/$', 'ietf.group.info.dependencies_pdf'),
-    (r'^init-charter/', 'ietf.group.edit.submit_initial_charter'),
     (r'^edit/$', 'ietf.group.edit.edit', {'action': "edit"}, "group_edit"),
     (r'^conclude/$', 'ietf.group.edit.conclude'),
     (r'^milestones/$', 'ietf.group.milestones.edit_milestones', {'milestone_set': "current"}, "group_edit_milestones"),

ietf/templates/doc/charter/submit.html

@@ -22,7 +22,11 @@ <h1>Charter submission<br><small>{{ group.acronym }} {{ group.type.name }}</smal
 
     {% buttons %}
       <button type="submit" class="btn btn-primary">Submit</button>
-      <a class="btn btn-default pull-right" href="{% url "doc_view" name=group.charter.name %}">Back</a>
+      {% if group.charter %}
+        <a class="btn btn-default pull-right" href="{% url "doc_view" name=name %}">Back</a>
+      {% else %}
+        <a class="btn btn-default pull-right" href="{% url "group_charter" group_type=group.type_id acronym=group.acronym %}">Back</a>
+      {% endif %}
     {% endbuttons %}
   </form>
 

ietf/templates/group/group_about.html

@@ -57,7 +57,7 @@
           {% else %}
             (None)
             {% if user|has_role:"Area Director,Secretariat" %}
-              <a class="btn btn-warning btn-xs" href="{% url "ietf.group.edit.submit_initial_charter" group_type=group.type_id acronym=group.acronym %}">Submit charter</a>
+              <a class="btn btn-warning btn-xs" href="{{ charter_submit_url }}">Submit charter</a>
             {% endif %}
           {% endif %}
         </td>