From c8dcda0fae7b7142fbc1abf4cad472c2e4870b80 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 14 Jun 2023 10:32:21 -0300 Subject: [PATCH] refactor: Do not use canonical_name() for charters (#5818) * fix: Enforce naming of charter docs in submit() * style: Reformat submit() with Black * refactor: Remove redundant check of charter name * style: Reformat charter_with_milestones_txt with Black * refactor: Drop canonical_name, use Path in charter_with_milestones_txt * style: Reformat review_announcement_text() with Black * style: Reformat action_announcement_text() with Black * refactor: Change uses of charter.canonical_name() to charter.name * refactor: Skip docialias when retrieving charter * refactor: Change canonical_name() to name in utils_charter.py * refactor: Use Path in read_charter_text() * refactor: Drop canonical_name, minor refactor of tests_charter.py * refactor: charter.name instead of canonical_name in milestones.py * refactor: charter.name instead of canonical_name in tests_info.py * refactor: Remove unused functions in ietf/secr/utils/groups.py * refactor: charter.canonical_name -> charter.name in templates * refactor: Remove charter handling from canonical_name Temporarily raise an exception for testing * refactor: Refactor get_charter_text() without canonical_name * refactor: Remove raise when canonical_name called on a charter * fix: Add back missing ".txt" extension * test: Test rejection of invalid charter names --- ietf/doc/models.py | 6 - ietf/doc/tests_charter.py | 32 +- ietf/doc/utils_charter.py | 14 +- ietf/doc/views_charter.py | 289 ++++++++++++------ ietf/group/milestones.py | 2 +- ietf/group/tests_info.py | 19 +- ietf/group/utils.py | 18 +- ietf/secr/utils/group.py | 24 -- .../doc/charter/action_announcement_text.html | 2 +- ietf/templates/doc/charter/approve.html | 6 +- ietf/templates/doc/charter/change_ad.html | 6 +- ietf/templates/group/edit_milestones.html | 6 +- 12 files changed, 256 insertions(+), 168 deletions(-) diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 86d814d73..547e40f4f 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -856,12 +856,6 @@ class Document(DocumentInfo): a = self.docalias.filter(name__startswith="rfc").order_by('-name').first() if a: name = a.name - elif self.type_id == "charter": - from ietf.doc.utils_charter import charter_name_for_group # Imported locally to avoid circular imports - try: - name = charter_name_for_group(self.chartered_group) - except Group.DoesNotExist: - pass self._canonical_name = name return self._canonical_name diff --git a/ietf/doc/tests_charter.py b/ietf/doc/tests_charter.py index f65cf14e0..c5c5ce5f9 100644 --- a/ietf/doc/tests_charter.py +++ b/ietf/doc/tests_charter.py @@ -88,10 +88,7 @@ class EditCharterTests(TestCase): settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['CHARTER_PATH'] def write_charter_file(self, charter): - with (Path(settings.CHARTER_PATH) / - ("%s-%s.txt" % (charter.canonical_name(), charter.rev)) - ).open("w") as f: - f.write("This is a charter.") + (Path(settings.CHARTER_PATH) / f"{charter.name}-{charter.rev}.txt").write_text("This is a charter.") def test_startstop_process(self): CharterFactory(group__acronym='mars') @@ -509,8 +506,13 @@ class EditCharterTests(TestCase): self.assertEqual(charter.rev, next_revision(prev_rev)) self.assertTrue("new_revision" in charter.latest_event().type) - with (Path(settings.CHARTER_PATH) / (charter.canonical_name() + "-" + charter.rev + ".txt")).open(encoding='utf-8') as f: - self.assertEqual(f.read(), "Windows line\nMac line\nUnix line\n" + utf_8_snippet.decode('utf-8')) + file_contents = ( + Path(settings.CHARTER_PATH) / (charter.name + "-" + charter.rev + ".txt") + ).read_text("utf-8") + self.assertEqual( + file_contents, + "Windows line\nMac line\nUnix line\n" + utf_8_snippet.decode("utf-8"), + ) def test_submit_initial_charter(self): group = GroupFactory(type_id='wg',acronym='mars',list_email='mars-wg@ietf.org') @@ -538,6 +540,24 @@ class EditCharterTests(TestCase): group = Group.objects.get(pk=group.pk) self.assertEqual(group.charter, charter) + def test_submit_charter_with_invalid_name(self): + self.client.login(username="secretary", password="secretary+password") + ietf_group = GroupFactory(type_id="wg") + for bad_name in ("charter-irtf-{}", "charter-randomjunk-{}", "charter-ietf-thisisnotagroup"): + url = urlreverse("ietf.doc.views_charter.submit", kwargs={"name": bad_name.format(ietf_group.acronym)}) + r = self.client.get(url) + self.assertEqual(r.status_code, 404, f"GET of charter named {bad_name} should 404") + r = self.client.post(url, {}) + self.assertEqual(r.status_code, 404, f"POST of charter named {bad_name} should 404") + + irtf_group = GroupFactory(type_id="rg") + for bad_name in ("charter-ietf-{}", "charter-whatisthis-{}", "charter-irtf-thisisnotagroup"): + url = urlreverse("ietf.doc.views_charter.submit", kwargs={"name": bad_name.format(irtf_group.acronym)}) + r = self.client.get(url) + self.assertEqual(r.status_code, 404, f"GET of charter named {bad_name} should 404") + r = self.client.post(url, {}) + self.assertEqual(r.status_code, 404, f"POST of charter named {bad_name} should 404") + def test_edit_review_announcement_text(self): area = GroupFactory(type_id='area') RoleFactory(name_id='ad',group=area,person=Person.objects.get(user__username='ad')) diff --git a/ietf/doc/utils_charter.py b/ietf/doc/utils_charter.py index 2e85b3cc1..7d2001e4d 100644 --- a/ietf/doc/utils_charter.py +++ b/ietf/doc/utils_charter.py @@ -3,11 +3,12 @@ import datetime -import io import os import re import shutil +from pathlib import Path + from django.conf import settings from django.urls import reverse as urlreverse from django.template.loader import render_to_string @@ -62,10 +63,9 @@ def next_approved_revision(rev): return "%#02d" % (int(m.group('major')) + 1) def read_charter_text(doc): - filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (doc.canonical_name(), doc.rev)) + filename = Path(settings.CHARTER_PATH) / f"{doc.name}-{doc.rev}.txt" try: - with io.open(filename, 'r') as f: - return f.read() + return filename.read_text() except IOError: return "Error: couldn't read charter text" @@ -92,8 +92,8 @@ def change_group_state_after_charter_approval(group, by): def fix_charter_revision_after_approval(charter, by): # according to spec, 00-02 becomes 01, so copy file and record new revision try: - old = os.path.join(charter.get_file_path(), '%s-%s.txt' % (charter.canonical_name(), charter.rev)) - new = os.path.join(charter.get_file_path(), '%s-%s.txt' % (charter.canonical_name(), next_approved_revision(charter.rev))) + old = os.path.join(charter.get_file_path(), '%s-%s.txt' % (charter.name, charter.rev)) + new = os.path.join(charter.get_file_path(), '%s-%s.txt' % (charter.name, next_approved_revision(charter.rev))) shutil.copy(old, new) except IOError: log("There was an error copying %s to %s" % (old, new)) @@ -101,7 +101,7 @@ def fix_charter_revision_after_approval(charter, by): events = [] e = NewRevisionDocEvent(doc=charter, by=by, type="new_revision") e.rev = next_approved_revision(charter.rev) - e.desc = "New version available: %s-%s.txt" % (charter.canonical_name(), e.rev) + e.desc = "New version available: %s-%s.txt" % (charter.name, e.rev) e.save() events.append(e) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index d3173291d..b3d9d06c3 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -3,11 +3,11 @@ import datetime -import io import json -import os import textwrap +from pathlib import Path + from django.http import HttpResponseRedirect, HttpResponseNotFound, Http404 from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse as urlreverse @@ -32,7 +32,7 @@ from ietf.doc.utils_charter import ( historic_milestones_for_charter, generate_ballot_writeup, generate_issue_ballot_mail, next_revision, derive_new_work_text, change_group_state_after_charter_approval, fix_charter_revision_after_approval, - split_charter_name) + split_charter_name, charter_name_for_group) from ietf.doc.mails import email_state_changed, email_charter_internal_review from ietf.group.mails import email_admin_re_charter from ietf.group.models import Group, ChangeStateGroupEvent, MilestoneGroupEvent @@ -42,6 +42,7 @@ from ietf.ietfauth.utils import has_role, role_required from ietf.name.models import GroupStateName from ietf.person.models import Person from ietf.utils.history import find_history_active_at +from ietf.utils.log import assertion from ietf.utils.mail import send_mail_preformatted from ietf.utils.textupload import get_cleaned_text_file_content from ietf.utils.response import permission_denied @@ -362,38 +363,41 @@ class UploadForm(forms.Form): @login_required def submit(request, name, option=None): - if not name.startswith('charter-'): - raise Http404 - + # Charters are named "charter--" charter = Document.objects.filter(type="charter", name=name).first() if charter: group = charter.group - charter_canonical_name = charter.canonical_name() + assertion("charter.name == charter_name_for_group(group)") 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 + if name != charter_name_for_group(group): + raise Http404 # do not allow creation of misnamed charters charter_rev = "00-00" - if not can_manage_all_groups_of_type(request.user, group.type_id) or not group.features.has_chartering_process: + if ( + not can_manage_all_groups_of_type(request.user, group.type_id) + or not group.features.has_chartering_process + ): permission_denied(request, "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) + charter_filename = Path(settings.CHARTER_PATH) / f"{name}-{charter_rev}.txt" + not_uploaded_yet = charter_rev.endswith("-00") and not charter_filename.exists() 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 else: # search history for possible collisions with abandoned efforts - prev_revs = list(charter.history_set.order_by('-time').values_list('rev', flat=True)) + prev_revs = list( + charter.history_set.order_by("-time").values_list("rev", flat=True) + ) next_rev = next_revision(charter.rev) while next_rev in prev_revs: next_rev = next_revision(next_rev) - if request.method == 'POST': + if request.method == "POST": form = UploadForm(request.POST, request.FILES) if form.is_valid(): # Also save group history so we can search for it @@ -408,9 +412,11 @@ def submit(request, name, option=None): abstract=group.name, rev=next_rev, ) - DocAlias.objects.create(name=charter.name).docs.add(charter) + DocAlias.objects.create(name=name).docs.add(charter) - charter.set_state(State.objects.get(used=True, type="charter", slug="notrev")) + charter.set_state( + State.objects.get(used=True, type="charter", slug="notrev") + ) group.charter = charter group.save() @@ -418,56 +424,74 @@ def submit(request, name, option=None): charter.rev = next_rev events = [] - e = NewRevisionDocEvent(doc=charter, by=request.user.person, type="new_revision") - e.desc = "New version available: %s-%s.txt" % (charter.canonical_name(), charter.rev) + e = NewRevisionDocEvent( + doc=charter, by=request.user.person, type="new_revision" + ) + e.desc = "New version available: %s-%s.txt" % ( + charter.name, + charter.rev, + ) e.rev = charter.rev e.save() events.append(e) # Save file on disk - filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter.canonical_name(), charter.rev)) - with io.open(filename, 'w', encoding='utf-8') as destination: - if form.cleaned_data['txt']: - destination.write(form.cleaned_data['txt']) + charter_filename = charter_filename.with_name( + f"{name}-{charter.rev}.txt" + ) # update rev + with charter_filename.open("w", encoding="utf-8") as destination: + if form.cleaned_data["txt"]: + destination.write(form.cleaned_data["txt"]) else: - destination.write(form.cleaned_data['content']) + destination.write(form.cleaned_data["content"]) - if option in ['initcharter','recharter'] and charter.ad == None: - charter.ad = getattr(group.ad_role(),'person',None) + if option in ["initcharter", "recharter"] and charter.ad == None: + charter.ad = getattr(group.ad_role(), "person", None) charter.save_with_history(events) if option: - return redirect('ietf.doc.views_charter.change_state', name=charter.name, option=option) + return redirect( + "ietf.doc.views_charter.change_state", + name=charter.name, + option=option, + ) else: return redirect("ietf.doc.views_doc.document_main", name=charter.name) else: - init = { "content": "" } + init = {"content": ""} 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").first() + h = ( + charter.history_set.filter(rev=last_approved) + .order_by("-time", "-id") + .first() + ) if h: - charter_canonical_name = h.canonical_name() - charter_rev = h.rev - - filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter_canonical_name, charter_rev)) + assertion("h.name == charter_name_for_group(group)") + charter_filename = charter_filename.with_name( + f"{name}-{h.rev}.txt" + ) # update rev try: - with io.open(filename, 'r') as f: - init["content"] = f.read() + init["content"] = charter_filename.read_text() except IOError: pass form = UploadForm(initial=init) fill_in_charter_info(group) - return render(request, 'doc/charter/submit.html', { - 'form': form, - 'next_rev': next_rev, - 'group': group, - 'name': name, - }) + return render( + request, + "doc/charter/submit.html", + { + "form": form, + "next_rev": next_rev, + "group": group, + "name": name, + }, + ) class ActionAnnouncementTextForm(forms.Form): announcement_text = forms.CharField(widget=forms.Textarea, required=True, strip=False) @@ -484,7 +508,7 @@ class ReviewAnnouncementTextForm(forms.Form): return self.cleaned_data["announcement_text"].replace("\r", "") -@role_required('Area Director','Secretariat') +@role_required("Area Director", "Secretariat") def review_announcement_text(request, name): """Editing of review announcement text""" charter = get_object_or_404(Document, type="charter", name=name) @@ -493,7 +517,9 @@ def review_announcement_text(request, name): by = request.user.person existing = charter.latest_event(WriteupDocEvent, type="changed_review_announcement") - existing_new_work = charter.latest_event(WriteupDocEvent, type="changed_new_work_text") + existing_new_work = charter.latest_event( + WriteupDocEvent, type="changed_new_work_text" + ) if not existing: (existing, existing_new_work) = default_review_text(group, charter, by) @@ -506,19 +532,23 @@ def review_announcement_text(request, name): existing_new_work.by = by existing_new_work.type = "changed_new_work_text" existing_new_work.desc = "%s review text was changed" % group.type.name - existing_new_work.text = derive_new_work_text(existing.text,group) + existing_new_work.text = derive_new_work_text(existing.text, group) existing_new_work.time = timezone.now() - form = ReviewAnnouncementTextForm(initial=dict(announcement_text=escape(existing.text),new_work_text=escape(existing_new_work.text))) + form = ReviewAnnouncementTextForm( + initial=dict( + announcement_text=escape(existing.text), + new_work_text=escape(existing_new_work.text), + ) + ) - if request.method == 'POST': + if request.method == "POST": form = ReviewAnnouncementTextForm(request.POST) if "save_text" in request.POST and form.is_valid(): - now = timezone.now() events = [] - t = form.cleaned_data['announcement_text'] + t = form.cleaned_data["announcement_text"] if t != existing.text: e = WriteupDocEvent(doc=charter, rev=charter.rev) e.by = by @@ -532,11 +562,11 @@ def review_announcement_text(request, name): existing.save() events.append(existing) - t = form.cleaned_data['new_work_text'] + t = form.cleaned_data["new_work_text"] if t != existing_new_work.text: e = WriteupDocEvent(doc=charter, rev=charter.rev) e.by = by - e.type = "changed_new_work_text" + e.type = "changed_new_work_text" e.desc = "%s new work message text was changed" % (group.type.name) e.text = t e.time = now @@ -549,33 +579,71 @@ def review_announcement_text(request, name): charter.save_with_history(events) if request.GET.get("next", "") == "approve": - return redirect('ietf.doc.views_charter.approve', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_charter.approve", name=charter.name + ) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_doc.document_writeup", name=charter.name + ) if "regenerate_text" in request.POST: (existing, existing_new_work) = default_review_text(group, charter, by) existing.save() existing_new_work.save() - form = ReviewAnnouncementTextForm(initial=dict(announcement_text=escape(existing.text), - new_work_text=escape(existing_new_work.text))) + form = ReviewAnnouncementTextForm( + initial=dict( + announcement_text=escape(existing.text), + new_work_text=escape(existing_new_work.text), + ) + ) - if any(x in request.POST for x in ['send_annc_only','send_nw_only','send_both']) and form.is_valid(): - if any(x in request.POST for x in ['send_annc_only','send_both']): - parsed_msg = send_mail_preformatted(request, form.cleaned_data['announcement_text']) - messages.success(request, "The email To: '%s' with Subject: '%s' has been sent." % (parsed_msg["To"],parsed_msg["Subject"],)) - if any(x in request.POST for x in ['send_nw_only','send_both']): - parsed_msg = send_mail_preformatted(request, form.cleaned_data['new_work_text']) - messages.success(request, "The email To: '%s' with Subject: '%s' has been sent." % (parsed_msg["To"],parsed_msg["Subject"],)) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.name) + if ( + any( + x in request.POST + for x in ["send_annc_only", "send_nw_only", "send_both"] + ) + and form.is_valid() + ): + if any(x in request.POST for x in ["send_annc_only", "send_both"]): + parsed_msg = send_mail_preformatted( + request, form.cleaned_data["announcement_text"] + ) + messages.success( + request, + "The email To: '%s' with Subject: '%s' has been sent." + % ( + parsed_msg["To"], + parsed_msg["Subject"], + ), + ) + if any(x in request.POST for x in ["send_nw_only", "send_both"]): + parsed_msg = send_mail_preformatted( + request, form.cleaned_data["new_work_text"] + ) + messages.success( + request, + "The email To: '%s' with Subject: '%s' has been sent." + % ( + parsed_msg["To"], + parsed_msg["Subject"], + ), + ) + return redirect("ietf.doc.views_doc.document_writeup", name=charter.name) - return render(request, 'doc/charter/review_announcement_text.html', - dict(charter=charter, - back_url=urlreverse('ietf.doc.views_doc.document_writeup', kwargs=dict(name=charter.name)), - announcement_text_form=form, - )) + return render( + request, + "doc/charter/review_announcement_text.html", + dict( + charter=charter, + back_url=urlreverse( + "ietf.doc.views_doc.document_writeup", kwargs=dict(name=charter.name) + ), + announcement_text_form=form, + ), + ) -@role_required('Area Director','Secretariat') +@role_required("Area Director", "Secretariat") def action_announcement_text(request, name): """Editing of action announcement text""" charter = get_object_or_404(Document, type="charter", name=name) @@ -590,16 +658,18 @@ def action_announcement_text(request, name): if not existing: raise Http404 - form = ActionAnnouncementTextForm(initial=dict(announcement_text=escape(existing.text))) + form = ActionAnnouncementTextForm( + initial=dict(announcement_text=escape(existing.text)) + ) - if request.method == 'POST': + if request.method == "POST": form = ActionAnnouncementTextForm(request.POST) if "save_text" in request.POST and form.is_valid(): - t = form.cleaned_data['announcement_text'] + t = form.cleaned_data["announcement_text"] if t != existing.text: e = WriteupDocEvent(doc=charter, rev=charter.rev) e.by = by - e.type = "changed_action_announcement" + e.type = "changed_action_announcement" e.desc = "%s action text was changed" % group.type.name e.text = t e.save() @@ -607,25 +677,46 @@ def action_announcement_text(request, name): existing.save() if request.GET.get("next", "") == "approve": - return redirect('ietf.doc.views_charter.approve', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_charter.approve", name=charter.name + ) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_doc.document_writeup", name=charter.name + ) if "regenerate_text" in request.POST: e = default_action_text(group, charter, by) e.save() - form = ActionAnnouncementTextForm(initial=dict(announcement_text=escape(e.text))) + form = ActionAnnouncementTextForm( + initial=dict(announcement_text=escape(e.text)) + ) if "send_text" in request.POST and form.is_valid(): - parsed_msg = send_mail_preformatted(request, form.cleaned_data['announcement_text']) - messages.success(request, "The email To: '%s' with Subject: '%s' has been sent." % (parsed_msg["To"],parsed_msg["Subject"],)) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.name) + parsed_msg = send_mail_preformatted( + request, form.cleaned_data["announcement_text"] + ) + messages.success( + request, + "The email To: '%s' with Subject: '%s' has been sent." + % ( + parsed_msg["To"], + parsed_msg["Subject"], + ), + ) + return redirect("ietf.doc.views_doc.document_writeup", name=charter.name) - return render(request, 'doc/charter/action_announcement_text.html', - dict(charter=charter, - back_url=urlreverse('ietf.doc.views_doc.document_writeup', kwargs=dict(name=charter.name)), - announcement_text_form=form, - )) + return render( + request, + "doc/charter/action_announcement_text.html", + dict( + charter=charter, + back_url=urlreverse( + "ietf.doc.views_doc.document_writeup", kwargs=dict(name=charter.name) + ), + announcement_text_form=form, + ), + ) class BallotWriteupForm(forms.Form): ballot_writeup = forms.CharField(widget=forms.Textarea, required=True, strip=False) @@ -806,33 +897,37 @@ def approve(request, name): dict(charter=charter, announcement=escape(announcement))) -def charter_with_milestones_txt(request, name, rev): - charter = get_object_or_404(Document, type="charter", docalias__name=name) - revision_event = charter.latest_event(NewRevisionDocEvent, type="new_revision", rev=rev) +def charter_with_milestones_txt(request, name, rev): + charter = get_object_or_404(Document, type="charter", name=name) + + revision_event = charter.latest_event( + NewRevisionDocEvent, type="new_revision", rev=rev + ) if not revision_event: return HttpResponseNotFound("Revision %s not found in database" % rev) # read charter text c = find_history_active_at(charter, revision_event.time) or charter - filename = '%s-%s.txt' % (c.canonical_name(), rev) - - charter_text = "" - + filename = Path(settings.CHARTER_PATH) / f"{c.name}-{rev}.txt" try: - with io.open(os.path.join(settings.CHARTER_PATH, filename), 'r') as f: - charter_text = force_str(f.read(), errors='ignore') + with filename.open() as f: + charter_text = force_str(f.read(), errors="ignore") except IOError: - charter_text = "Error reading charter text %s" % filename + charter_text = f"Error reading charter text {filename.name}" milestones = historic_milestones_for_charter(charter, rev) # wrap the output nicely - wrapper = textwrap.TextWrapper(initial_indent="", subsequent_indent=" " * 11, width=80, break_long_words=False) + wrapper = textwrap.TextWrapper( + initial_indent="", subsequent_indent=" " * 11, width=80, break_long_words=False + ) for m in milestones: m.desc_filled = wrapper.fill(m.desc) - return render(request, 'doc/charter/charter_with_milestones.txt', - dict(charter_text=charter_text, - milestones=milestones), - content_type="text/plain; charset=%s"%settings.DEFAULT_CHARSET) + return render( + request, + "doc/charter/charter_with_milestones.txt", + dict(charter_text=charter_text, milestones=milestones), + content_type="text/plain; charset=%s" % settings.DEFAULT_CHARSET, + ) diff --git a/ietf/group/milestones.py b/ietf/group/milestones.py index 64ebb389e..039fdb44c 100644 --- a/ietf/group/milestones.py +++ b/ietf/group/milestones.py @@ -369,7 +369,7 @@ def edit_milestones(request, acronym, group_type=None, milestone_set="current"): email_milestones_changed(request, group, changes, states) if milestone_set == "charter": - return redirect('ietf.doc.views_doc.document_main', name=group.charter.canonical_name()) + return redirect('ietf.doc.views_doc.document_main', name=group.charter.name) else: return HttpResponseRedirect(group.about_url()) else: diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 672d18c8f..72b632bcc 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -117,8 +117,9 @@ class GroupPagesTests(TestCase): chair = Email.objects.filter(role__group=group, role__name="chair")[0] - with (Path(settings.CHARTER_PATH) / ("%s-%s.txt" % (group.charter.canonical_name(), group.charter.rev))).open("w") as f: - f.write("This is a charter.") + ( + Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt" + ).write_text("This is a charter.") url = urlreverse('ietf.group.views.wg_summary_area', kwargs=dict(group_type="wg")) r = self.client.get(url) @@ -264,8 +265,9 @@ class GroupPagesTests(TestCase): group = CharterFactory().group draft = WgDraftFactory(group=group) - with (Path(settings.CHARTER_PATH) / ("%s-%s.txt" % (group.charter.canonical_name(), group.charter.rev))).open("w") as f: - f.write("This is a charter.") + ( + Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt" + ).write_text("This is a charter.") milestone = GroupMilestone.objects.create( group=group, @@ -674,8 +676,9 @@ class GroupEditTests(TestCase): self.assertTrue(len(q('form .is-invalid')) > 0) # edit info - with (Path(settings.CHARTER_PATH) / ("%s-%s.txt" % (group.charter.canonical_name(), group.charter.rev))).open("w") as f: - f.write("This is a charter.") + ( + Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt" + ).write_text("This is a charter.") area = group.parent ad = Person.objects.get(name="Areað Irector") state = GroupStateName.objects.get(slug="bof") @@ -717,7 +720,9 @@ class GroupEditTests(TestCase): 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.assertTrue( + (Path(settings.CHARTER_PATH) / f"{group.charter.name}-{group.charter.rev}.txt").exists() + ) self.assertEqual(len(outbox), 2) self.assertTrue('Personnel change' in outbox[0]['Subject']) for prefix in ['ad1','ad2','aread','marschairman','marsdelegate']: diff --git a/ietf/group/utils.py b/ietf/group/utils.py index b701d6a7c..9cf093044 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -2,8 +2,7 @@ # -*- coding: utf-8 -*- -import io -import os +from pathlib import Path from django.db.models import Q from django.shortcuts import get_object_or_404 @@ -55,15 +54,14 @@ def get_charter_text(group): if (h.rev > c.rev and not (c_appr and not h_appr)) or (h_appr and not c_appr): c = h - filename = os.path.join(c.get_file_path(), "%s-%s.txt" % (c.canonical_name(), c.rev)) + filename = Path(c.get_file_path()) / f"{c.name}-{c.rev}.txt" try: - with io.open(filename, 'rb') as f: - text = f.read() - try: - text = text.decode('utf8') - except UnicodeDecodeError: - text = text.decode('latin1') - return text + text = filename.read_bytes() + try: + text = text.decode('utf8') + except UnicodeDecodeError: + text = text.decode('latin1') + return text except IOError: return 'Error Loading Group Charter' diff --git a/ietf/secr/utils/group.py b/ietf/secr/utils/group.py index a4c1c0f98..40a9065ac 100644 --- a/ietf/secr/utils/group.py +++ b/ietf/secr/utils/group.py @@ -3,11 +3,8 @@ # Python imports -import io -import os # Django imports -from django.conf import settings from django.core.exceptions import ObjectDoesNotExist # Datatracker imports @@ -15,27 +12,6 @@ from ietf.group.models import Group from ietf.ietfauth.utils import has_role - - -def current_nomcom(): - qs = Group.objects.filter(acronym__startswith='nomcom',state__slug="active").order_by('-time') - if qs.count(): - return qs[0] - else: - return None - -def get_charter_text(group): - ''' - Takes a group object and returns the text or the group's charter as a string - ''' - charter = group.charter - path = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter.canonical_name(), charter.rev)) - f = io.open(path,'r') - text = f.read() - f.close() - - return text - def get_my_groups(user,conclude=False): ''' Takes a Django user object (from request) diff --git a/ietf/templates/doc/charter/action_announcement_text.html b/ietf/templates/doc/charter/action_announcement_text.html index 5722b342a..87af2510a 100644 --- a/ietf/templates/doc/charter/action_announcement_text.html +++ b/ietf/templates/doc/charter/action_announcement_text.html @@ -21,7 +21,7 @@ {% if user|has_role:"Secretariat" %} + href="{% url 'ietf.doc.views_charter.approve' name=charter.name %}"> Charter approval page {% endif %} diff --git a/ietf/templates/doc/charter/approve.html b/ietf/templates/doc/charter/approve.html index f109da687..2a8654482 100644 --- a/ietf/templates/doc/charter/approve.html +++ b/ietf/templates/doc/charter/approve.html @@ -2,16 +2,16 @@ {# Copyright The IETF Trust 2015, All Rights Reserved #} {% load origin %} {% load django_bootstrap5 %} -{% block title %}Approve {{ charter.canonical_name }}{% endblock %} +{% block title %}Approve {{ charter.name }}{% endblock %} {% block content %} {% origin %} -

Approve {{ charter.canonical_name }}-{{ charter.rev }}

+

Approve {{ charter.name }}-{{ charter.rev }}

{% csrf_token %} 
{{ announcement }}
+ href="{% url "ietf.doc.views_charter.action_announcement_text" name=charter.name %}?next=approve"> Edit/regenerate announcement Change responsible AD
- {{ charter.canonical_name }}-{{ charter.rev }} + {{ charter.name }}-{{ charter.rev }} {% csrf_token %} {% bootstrap_form form %} + href="{% url "ietf.doc.views_doc.document_main" name=charter.name %}"> Back
diff --git a/ietf/templates/group/edit_milestones.html b/ietf/templates/group/edit_milestones.html index 36a4a714c..1ae1c57a5 100644 --- a/ietf/templates/group/edit_milestones.html +++ b/ietf/templates/group/edit_milestones.html @@ -14,8 +14,8 @@ {{ group.acronym }} {{ group.type.name }} {% if group.charter %} - {{ group.charter.canonical_name }} + href="{% url "ietf.doc.views_doc.document_main" name=group.charter.name %}"> + {{ group.charter.name }} {% endif %} {% if can_change_uses_milestone_dates %} @@ -106,7 +106,7 @@ + href="{% if milestone_set == "charter" %}{% url "ietf.doc.views_doc.document_main" name=group.charter.name %}{% else %}{{ group.about_url }}{% endif %}"> Cancel