From 63d96e0354cc975fb1df606cd1221d048b9e6606 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 6 Oct 2015 15:14:31 +0000 Subject: [PATCH] Summary: Get rid of submit_initial_charter hack, only create a charter when a first revision has actually been submitted. - Legacy-Id: 10136 --- ietf/doc/tests_charter.py | 37 +++++++++++- ietf/doc/utils_charter.py | 4 ++ ietf/doc/views_charter.py | 78 +++++++++++++++++--------- ietf/group/edit.py | 58 +------------------ ietf/group/info.py | 5 ++ ietf/group/tests_info.py | 21 +------ ietf/group/urls_info_details.py | 1 - ietf/templates/doc/charter/submit.html | 6 +- ietf/templates/group/group_about.html | 2 +- 9 files changed, 108 insertions(+), 104 deletions(-) diff --git a/ietf/doc/tests_charter.py b/ietf/doc/tests_charter.py index 912d788db..ca8e12ccf 100644 --- a/ietf/doc/tests_charter.py +++ b/ietf/doc/tests_charter.py @@ -9,7 +9,8 @@ from django.core.urlresolvers import reverse as urlreverse 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 @@ class EditCharterTests(TestCase): 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 diff --git a/ietf/doc/utils_charter.py b/ietf/doc/utils_charter.py index 02cea0872..a93a00984 100644 --- a/ietf/doc/utils_charter.py +++ b/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" diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 01767577d..09bf473bd 100644 --- a/ietf/doc/views_charter.py +++ b/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 @@ from django.contrib.auth.decorators import login_required 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 @@ class UploadForm(forms.Form): 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,17 +423,17 @@ 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: @@ -415,12 +441,12 @@ def submit(request, name=None, option=None): 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) diff --git a/ietf/group/edit.py b/ietf/group/edit.py index 5133c3d4d..bdb9d39c2 100644 --- a/ietf/group/edit.py +++ b/ietf/group/edit.py @@ -11,7 +11,7 @@ from django.contrib.auth.decorators import login_required 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 edit(request, group_type=None, acronym=None, action="edit"): 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() diff --git a/ietf/group/info.py b/ietf/group/info.py index 22fb13000..aae7726d2 100644 --- a/ietf/group/info.py +++ b/ietf/group/info.py @@ -50,6 +50,7 @@ from django.utils.safestring import mark_safe 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,6 +472,9 @@ 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, { @@ -478,6 +482,7 @@ def group_about(request, acronym, group_type=None): "milestone_reviewer": milestone_reviewer_for_group_type(group_type), "requested_close": requested_close, "can_manage": can_manage, + "charter_submit_url": charter_submit_url })) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 9cb180e1e..c658df1f2 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -13,6 +13,7 @@ from django.core.urlresolvers import reverse as urlreverse 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 @@ class GroupEditTests(TestCase): 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 @@ class GroupEditTests(TestCase): 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 @@ class GroupEditTests(TestCase): 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() diff --git a/ietf/group/urls_info_details.py b/ietf/group/urls_info_details.py index 430c09b39..4d1ef4212 100644 --- a/ietf/group/urls_info_details.py +++ b/ietf/group/urls_info_details.py @@ -9,7 +9,6 @@ urlpatterns = patterns('', (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"), diff --git a/ietf/templates/doc/charter/submit.html b/ietf/templates/doc/charter/submit.html index 767f00472..10dfa293c 100644 --- a/ietf/templates/doc/charter/submit.html +++ b/ietf/templates/doc/charter/submit.html @@ -22,7 +22,11 @@ {% buttons %} - Back + {% if group.charter %} + Back + {% else %} + Back + {% endif %} {% endbuttons %} diff --git a/ietf/templates/group/group_about.html b/ietf/templates/group/group_about.html index 567543b88..0910b6df9 100644 --- a/ietf/templates/group/group_about.html +++ b/ietf/templates/group/group_about.html @@ -57,7 +57,7 @@ {% else %} (None) {% if user|has_role:"Area Director,Secretariat" %} - Submit charter + Submit charter {% endif %} {% endif %}