From 4a2f807bcda0e4233d2501994a76270e56cbbf0f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 13:06:48 -0300 Subject: [PATCH 01/21] fix: Enforce naming of charter docs in submit() --- ietf/doc/views_charter.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index d3173291d3d..ca86db2313d 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -8,6 +8,8 @@ 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 +34,7 @@ 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 +44,7 @@ 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 @@ -365,23 +368,24 @@ 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: 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 @@ -408,7 +412,7 @@ 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")) @@ -419,14 +423,14 @@ def submit(request, name, option=None): 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.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: + charter_filename = charter_filename.with_name(f"{name}-{charter.rev}") # update rev + with charter_filename.open('w', encoding='utf-8') as destination: if form.cleaned_data['txt']: destination.write(form.cleaned_data['txt']) else: @@ -449,14 +453,11 @@ def submit(request, name, option=None): last_approved = charter.rev.split("-")[0] 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) From 2748f719b93214a67726a3d46496c2c8b5ddc895 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 13:23:29 -0300 Subject: [PATCH 02/21] style: Reformat submit() with Black --- ietf/doc/views_charter.py | 76 ++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index ca86db2313d..a7daf20f2be 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -365,7 +365,7 @@ def clean_txt(self): @login_required def submit(request, name, option=None): - if not name.startswith('charter-'): + if not name.startswith("charter-"): raise Http404 # Charters are named "charter--" @@ -381,7 +381,10 @@ def submit(request, name, option=None): 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.") charter_filename = Path(settings.CHARTER_PATH) / f"{name}-{charter_rev}.txt" @@ -392,12 +395,14 @@ def submit(request, name, option=None): 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 @@ -414,7 +419,9 @@ def submit(request, name, option=None): ) 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() @@ -422,39 +429,56 @@ 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.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 - charter_filename = charter_filename.with_name(f"{name}-{charter.rev}") # update rev - with charter_filename.open('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}" + ) # 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: assertion("h.name == charter_name_for_group(group)") - charter_filename = charter_filename.with_name(f"{name}-{h.rev}.txt") # update rev + charter_filename = charter_filename.with_name( + f"{name}-{h.rev}.txt" + ) # update rev try: init["content"] = charter_filename.read_text() @@ -463,12 +487,16 @@ def submit(request, name, option=None): 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) From ff913892529c8b57e193376442438a41add2bb83 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 13:25:13 -0300 Subject: [PATCH 03/21] refactor: Remove redundant check of charter name --- ietf/doc/views_charter.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index a7daf20f2be..285c272feb2 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -365,9 +365,6 @@ def clean_txt(self): @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: From c0736eff231137e42343d6cf469a6a1d1d47e7cc Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 13:27:52 -0300 Subject: [PATCH 04/21] style: Reformat charter_with_milestones_txt with Black --- ietf/doc/views_charter.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 285c272feb2..2686b5d8bdb 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -835,30 +835,36 @@ def approve(request, name): 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) + 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) + filename = "%s-%s.txt" % (c.canonical_name(), rev) charter_text = "" try: - with io.open(os.path.join(settings.CHARTER_PATH, filename), 'r') as f: - charter_text = force_str(f.read(), errors='ignore') + with io.open(os.path.join(settings.CHARTER_PATH, filename), "r") as f: + charter_text = force_str(f.read(), errors="ignore") except IOError: charter_text = "Error reading charter text %s" % filename 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, + ) From c971c6b6db96ab9618fa203045747ac77c561c5a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 13:33:19 -0300 Subject: [PATCH 05/21] refactor: Drop canonical_name, use Path in charter_with_milestones_txt --- ietf/doc/views_charter.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 2686b5d8bdb..378f1ee0c1d 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -3,9 +3,7 @@ import datetime -import io import json -import os import textwrap from pathlib import Path @@ -832,6 +830,7 @@ 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) @@ -843,15 +842,12 @@ def charter_with_milestones_txt(request, name, 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: + 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) From b2ea37983eea6f3874d0e2b6ed2300dab8dd9b41 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 15:37:19 -0300 Subject: [PATCH 06/21] style: Reformat review_announcement_text() with Black --- ietf/doc/views_charter.py | 98 ++++++++++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 27 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 378f1ee0c1d..7ec059f1b60 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -508,7 +508,7 @@ def clean_announcement_text(self): 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) @@ -517,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) @@ -530,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 @@ -556,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 @@ -573,31 +579,69 @@ 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.canonical_name() + ) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_doc.document_writeup", name=charter.canonical_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))) - - 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) + form = ReviewAnnouncementTextForm( + initial=dict( + announcement_text=escape(existing.text), + new_work_text=escape(existing_new_work.text), + ) + ) - 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, - )) + 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, + ), + ) @role_required('Area Director','Secretariat') def action_announcement_text(request, name): From 7b9909ee2e2a227abafd542eac8c8c72859774f2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 15:42:20 -0300 Subject: [PATCH 07/21] style: Reformat action_announcement_text() with Black --- ietf/doc/views_charter.py | 55 +++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 7ec059f1b60..2aebf2ed2d6 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -643,7 +643,7 @@ def review_announcement_text(request, name): ), ) -@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) @@ -658,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() @@ -675,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.canonical_name() + ) - return redirect('ietf.doc.views_doc.document_writeup', name=charter.canonical_name()) + return redirect( + "ietf.doc.views_doc.document_writeup", name=charter.canonical_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) From a35e5ed1f228601dfac6111d70f48d9333e63bc9 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 15:43:27 -0300 Subject: [PATCH 08/21] refactor: Change uses of charter.canonical_name() to charter.name --- ietf/doc/views_charter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 2aebf2ed2d6..51b237c062b 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -580,11 +580,11 @@ def review_announcement_text(request, name): if request.GET.get("next", "") == "approve": return redirect( - "ietf.doc.views_charter.approve", name=charter.canonical_name() + "ietf.doc.views_charter.approve", name=charter.name ) return redirect( - "ietf.doc.views_doc.document_writeup", name=charter.canonical_name() + "ietf.doc.views_doc.document_writeup", name=charter.name ) if "regenerate_text" in request.POST: @@ -678,11 +678,11 @@ def action_announcement_text(request, name): if request.GET.get("next", "") == "approve": return redirect( - "ietf.doc.views_charter.approve", name=charter.canonical_name() + "ietf.doc.views_charter.approve", name=charter.name ) return redirect( - "ietf.doc.views_doc.document_writeup", name=charter.canonical_name() + "ietf.doc.views_doc.document_writeup", name=charter.name ) if "regenerate_text" in request.POST: From 1812c684ad6cfe649023bb15df83cbfd9a422d36 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:03:44 -0300 Subject: [PATCH 09/21] refactor: Skip docialias when retrieving charter --- ietf/doc/views_charter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 51b237c062b..b908188537b 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -899,7 +899,7 @@ def approve(request, name): def charter_with_milestones_txt(request, name, rev): - charter = get_object_or_404(Document, type="charter", docalias__name=name) + charter = get_object_or_404(Document, type="charter", name=name) revision_event = charter.latest_event( NewRevisionDocEvent, type="new_revision", rev=rev From 07388d2af25ff7d3dfcee28461221a4ec3f238ea Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:07:19 -0300 Subject: [PATCH 10/21] refactor: Change canonical_name() to name in utils_charter.py --- ietf/doc/utils_charter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ietf/doc/utils_charter.py b/ietf/doc/utils_charter.py index 2e85b3cc10a..ae613fe55e7 100644 --- a/ietf/doc/utils_charter.py +++ b/ietf/doc/utils_charter.py @@ -62,7 +62,7 @@ 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 = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (doc.name, doc.rev)) try: with io.open(filename, 'r') as f: return f.read() @@ -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) From 90b0fd8c193517aad4c8550f4c260231af91502f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:12:05 -0300 Subject: [PATCH 11/21] refactor: Use Path in read_charter_text() --- ietf/doc/utils_charter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ietf/doc/utils_charter.py b/ietf/doc/utils_charter.py index ae613fe55e7..7d2001e4d7c 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.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" From dc42f830209262510bf724570d0abaf1bd60c1ef Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:19:30 -0300 Subject: [PATCH 12/21] refactor: Drop canonical_name, minor refactor of tests_charter.py --- ietf/doc/tests_charter.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ietf/doc/tests_charter.py b/ietf/doc/tests_charter.py index f65cf14e088..301d6d2b9f6 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 @@ def test_submit_charter(self): 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') From c6d5f0d5e6003a305061e3faa5fd1dd88d89b0c4 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:23:32 -0300 Subject: [PATCH 13/21] refactor: charter.name instead of canonical_name in milestones.py --- ietf/group/milestones.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/group/milestones.py b/ietf/group/milestones.py index 64ebb389e21..039fdb44ce0 100644 --- a/ietf/group/milestones.py +++ b/ietf/group/milestones.py @@ -369,7 +369,7 @@ def save_milestone_form(f): 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: From 04c65201310b980cd63735817d9ef1336c91d416 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:27:25 -0300 Subject: [PATCH 14/21] refactor: charter.name instead of canonical_name in tests_info.py --- ietf/group/tests_info.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 672d18c8ff7..72b632bcc6e 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -117,8 +117,9 @@ def test_wg_summaries(self): 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 @@ def test_group_charter(self): 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 @@ def test_edit_info(self): 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 @@ def test_edit_info(self): 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']: From d646bb28ca522431e45bb0fa9c189bb0a6d158e2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:31:28 -0300 Subject: [PATCH 15/21] refactor: Remove unused functions in ietf/secr/utils/groups.py --- ietf/secr/utils/group.py | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/ietf/secr/utils/group.py b/ietf/secr/utils/group.py index a4c1c0f98a9..61d165d37bc 100644 --- a/ietf/secr/utils/group.py +++ b/ietf/secr/utils/group.py @@ -3,8 +3,7 @@ # Python imports -import io -import os +from pathlib import Path # Django imports from django.conf import settings @@ -15,27 +14,6 @@ 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) From 4ce4dc150a4032ba9203dd80cb34a5d2a74267ad Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 13 Jun 2023 16:34:23 -0300 Subject: [PATCH 16/21] refactor: charter.canonical_name -> charter.name in templates --- ietf/templates/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 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/ietf/templates/doc/charter/action_announcement_text.html b/ietf/templates/doc/charter/action_announcement_text.html index 5722b342a15..87af2510a37 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 f109da6872b..2a8654482eb 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 36a4a714ca5..1ae1c57a56f 100644 --- a/ietf/templates/group/edit_milestones.html +++ b/ietf/templates/group/edit_milestones.html @@ -14,8 +14,8 @@

{{ title }}

{{ 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 @@

{{ title }}

+ href="{% if milestone_set == "charter" %}{% url "ietf.doc.views_doc.document_main" name=group.charter.name %}{% else %}{{ group.about_url }}{% endif %}"> Cancel