Refactor role handling in group editing slightly and add support for
editing reviewer roles in review teams. Also fix a couple of review related bugs. - Legacy-Id: 11921
This commit is contained in:
Ole Laursen
parent
119f7256f1
commit
1f7d4870a8
|
|
@ -11,6 +11,7 @@ class GroupFeatures(object):
|
|||
about_page = "group_about"
|
||||
default_tab = about_page
|
||||
material_types = ["slides"]
|
||||
admin_roles = ["chair"]
|
||||
|
||||
def __init__(self, group):
|
||||
if group.type_id in ("wg", "rg"):
|
||||
|
|
@ -31,3 +32,6 @@ class GroupFeatures(object):
|
|||
self.has_reviews = True
|
||||
import ietf.group.views
|
||||
self.default_tab = ietf.group.views.review_requests
|
||||
|
||||
if group.type_id == "dir":
|
||||
self.admin_roles = ["chair", "secr"]
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ def edit_milestones(request, acronym, group_type=None, milestone_set="current"):
|
|||
|
||||
needs_review = False
|
||||
if not can_manage_group(request.user, group):
|
||||
if group.has_role(request.user, "chair"):
|
||||
if group.has_role(request.user, group.features.admin_roles):
|
||||
if milestone_set == "current":
|
||||
needs_review = True
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ from ietf.person.models import Person, Email
|
|||
from ietf.utils.test_utils import TestCase, unicontent
|
||||
from ietf.utils.mail import outbox, empty_outbox
|
||||
from ietf.utils.test_data import make_test_data, create_person, make_review_data
|
||||
from ietf.utils.test_utils import login_testing_unauthorized
|
||||
from ietf.utils.test_utils import login_testing_unauthorized, reload_db_objects
|
||||
from ietf.group.factories import GroupFactory, RoleFactory, GroupEventFactory
|
||||
from ietf.meeting.factories import SessionFactory
|
||||
import ietf.group.views
|
||||
|
|
@ -569,10 +569,10 @@ class GroupEditTests(TestCase):
|
|||
parent=area.pk,
|
||||
ad=ad.pk,
|
||||
state=state.pk,
|
||||
chairs="aread@ietf.org, ad1@ietf.org",
|
||||
secretaries="aread@ietf.org, ad1@ietf.org, ad2@ietf.org",
|
||||
techadv="aread@ietf.org",
|
||||
delegates="ad2@ietf.org",
|
||||
chair_roles="aread@ietf.org, ad1@ietf.org",
|
||||
secr_roles="aread@ietf.org, ad1@ietf.org, ad2@ietf.org",
|
||||
techadv_roles="aread@ietf.org",
|
||||
delegate_roles="ad2@ietf.org",
|
||||
list_email="mars@mail",
|
||||
list_subscribe="subscribe.mars",
|
||||
list_archive="archive.mars",
|
||||
|
|
@ -598,6 +598,40 @@ class GroupEditTests(TestCase):
|
|||
for prefix in ['ad1','ad2','aread','marschairman','marsdelegate']:
|
||||
self.assertTrue(prefix+'@' in outbox[0]['To'])
|
||||
|
||||
def test_edit_reviewers(self):
|
||||
doc = make_test_data()
|
||||
review_req = make_review_data(doc)
|
||||
group = review_req.team
|
||||
|
||||
url = urlreverse('group_edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym))
|
||||
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=reviewer_roles]')), 1)
|
||||
|
||||
# set reviewers
|
||||
empty_outbox()
|
||||
r = self.client.post(url,
|
||||
dict(name=group.name,
|
||||
acronym=group.acronym,
|
||||
parent=group.parent_id,
|
||||
ad=Person.objects.get(name="Areað Irector").pk,
|
||||
state=group.state_id,
|
||||
reviewer_roles="ad2@ietf.org",
|
||||
list_email=group.list_email,
|
||||
list_subscribe=group.list_subscribe,
|
||||
list_archive=group.list_archive,
|
||||
urls=""
|
||||
))
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
||||
group = reload_db_objects(group)
|
||||
self.assertEqual(list(group.role_set.filter(name="reviewer").values_list("email", flat=True)), ["ad2@ietf.org"])
|
||||
self.assertTrue('Personnel change' in outbox[0]['Subject'])
|
||||
|
||||
def test_initial_charter(self):
|
||||
make_test_data()
|
||||
group = Group.objects.get(acronym="mars")
|
||||
|
|
|
|||
|
|
@ -38,7 +38,6 @@ import re
|
|||
from tempfile import mkstemp
|
||||
import datetime
|
||||
from collections import OrderedDict
|
||||
import math
|
||||
|
||||
import debug # pyflakes:ignore
|
||||
|
||||
|
|
@ -364,11 +363,11 @@ def construct_group_menu_context(request, group, selected, group_type, others):
|
|||
# actions
|
||||
actions = []
|
||||
|
||||
is_chair = group.has_role(request.user, "chair")
|
||||
is_admin = group.has_role(request.user, group.features.admin_roles)
|
||||
can_manage = can_manage_group(request.user, group)
|
||||
|
||||
if group.features.has_milestones:
|
||||
if group.state_id != "proposed" and (is_chair or can_manage):
|
||||
if group.state_id != "proposed" and (is_admin or can_manage):
|
||||
actions.append((u"Edit milestones", urlreverse("group_edit_milestones", kwargs=kwargs)))
|
||||
|
||||
if group.features.has_documents:
|
||||
|
|
@ -384,10 +383,10 @@ def construct_group_menu_context(request, group, selected, group_type, others):
|
|||
import ietf.group.views_review
|
||||
actions.append((u"Manage review requests", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=kwargs)))
|
||||
|
||||
if group.state_id != "conclude" and (is_chair or can_manage):
|
||||
if group.state_id != "conclude" and (is_admin or can_manage):
|
||||
actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs)))
|
||||
|
||||
if group.features.customize_workflow and (is_chair or can_manage):
|
||||
if group.features.customize_workflow and (is_admin or can_manage):
|
||||
actions.append((u"Customize workflow", urlreverse("ietf.group.views_edit.customize_workflow", kwargs=kwargs)))
|
||||
|
||||
if group.state_id in ("active", "dormant") and not group.type_id in ["sdo", "rfcedtyp", "isoc", ] and can_manage:
|
||||
|
|
|
|||
|
|
@ -23,19 +23,31 @@ from ietf.person.fields import SearchableEmailsField
|
|||
from ietf.person.models import Person, Email
|
||||
from ietf.group.mails import ( email_admin_re_charter, email_personnel_change)
|
||||
from ietf.utils.ordereddict import insert_after_in_ordered_dict
|
||||
from ietf.utils.text import skip_suffix
|
||||
|
||||
|
||||
MAX_GROUP_DELEGATES = 3
|
||||
|
||||
def roles_for_group_type(group_type):
|
||||
roles = ["chair", "secr", "techadv", "delegate"]
|
||||
if group_type == "dir":
|
||||
roles.append("reviewer")
|
||||
return roles
|
||||
|
||||
class GroupForm(forms.Form):
|
||||
name = forms.CharField(max_length=255, label="Name", required=True)
|
||||
acronym = forms.CharField(max_length=10, label="Acronym", required=True)
|
||||
state = forms.ModelChoiceField(GroupStateName.objects.all(), label="State", required=True)
|
||||
chairs = SearchableEmailsField(label="Chairs", required=False, only_users=True)
|
||||
secretaries = SearchableEmailsField(label="Secretaries", required=False, only_users=True)
|
||||
techadv = SearchableEmailsField(label="Technical Advisors", required=False, only_users=True)
|
||||
delegates = SearchableEmailsField(label="Delegates", required=False, only_users=True, max_entries=MAX_GROUP_DELEGATES,
|
||||
|
||||
# roles
|
||||
chair_roles = SearchableEmailsField(label="Chairs", required=False, only_users=True)
|
||||
secr_roles = SearchableEmailsField(label="Secretaries", required=False, only_users=True)
|
||||
techadv_roles = SearchableEmailsField(label="Technical Advisors", required=False, only_users=True)
|
||||
delegate_roles = SearchableEmailsField(label="Delegates", required=False, only_users=True, max_entries=MAX_GROUP_DELEGATES,
|
||||
help_text=mark_safe("Chairs can delegate the authority to update the state of group documents - at most %s persons at a given time." % MAX_GROUP_DELEGATES))
|
||||
reviewer_roles = SearchableEmailsField(label="Reviewers", required=False, only_users=True)
|
||||
ad = forms.ModelChoiceField(Person.objects.filter(role__name="ad", role__group__state="active", role__group__type='area').order_by('name'), label="Shepherding AD", empty_label="(None)", required=False)
|
||||
|
||||
parent = forms.ModelChoiceField(Group.objects.filter(state="active").order_by('name'), empty_label="(None)", required=False)
|
||||
list_email = forms.CharField(max_length=64, required=False)
|
||||
list_subscribe = forms.CharField(max_length=255, required=False)
|
||||
|
|
@ -69,6 +81,11 @@ class GroupForm(forms.Form):
|
|||
self.fields['parent'].queryset = self.fields['parent'].queryset.filter(type="area")
|
||||
self.fields['parent'].label = "IETF Area"
|
||||
|
||||
role_fields_to_remove = (set(roles_for_group_type(self.group_type))
|
||||
- set(skip_suffix(attr, "_roles") for attr in self.fields if attr.endswith("_roles")))
|
||||
for r in role_fields_to_remove:
|
||||
del self.fields[r + "_roles"]
|
||||
|
||||
def clean_acronym(self):
|
||||
# Changing the acronym of an already existing group will cause 404s all
|
||||
# over the place, loose history, and generally muck up a lot of
|
||||
|
|
@ -211,7 +228,8 @@ def edit(request, group_type=None, acronym=None, action="edit"):
|
|||
group = get_group_or_404(acronym, group_type)
|
||||
if not group_type and group:
|
||||
group_type = group.type_id
|
||||
if not (can_manage_group(request.user, group) or group.has_role(request.user, "chair")):
|
||||
if not (can_manage_group(request.user, group)
|
||||
or group.has_role(request.user, group.features.admin_roles)):
|
||||
return HttpResponseForbidden("You don't have permission to access this view")
|
||||
|
||||
if request.method == 'POST':
|
||||
|
|
@ -274,10 +292,18 @@ def edit(request, group_type=None, acronym=None, action="edit"):
|
|||
personnel_change_text=""
|
||||
changed_personnel = set()
|
||||
# update roles
|
||||
for attr, slug, title in [('ad','ad','Shepherding AD'), ('chairs', 'chair', "Chairs"), ('secretaries', 'secr', "Secretaries"), ('techadv', 'techadv', "Tech Advisors"), ('delegates', 'delegate', "Delegates")]:
|
||||
for attr, f in form.fields.iteritems():
|
||||
if not (attr.endswith("_roles") or attr == "ad"):
|
||||
continue
|
||||
|
||||
slug = attr
|
||||
slug = skip_suffix(slug, "_roles")
|
||||
|
||||
title = f.label
|
||||
|
||||
new = clean[attr]
|
||||
if attr == 'ad':
|
||||
new = [ new.role_email('ad'),] if new else []
|
||||
new = [ new.role_email('ad') ] if new else []
|
||||
old = Email.objects.filter(role__group=group, role__name=slug).select_related("person")
|
||||
if set(new) != set(old):
|
||||
changes.append((attr, new, desc(title,
|
||||
|
|
@ -336,10 +362,6 @@ def edit(request, group_type=None, acronym=None, action="edit"):
|
|||
init = dict(name=group.name,
|
||||
acronym=group.acronym,
|
||||
state=group.state,
|
||||
chairs=Email.objects.filter(role__group=group, role__name="chair"),
|
||||
secretaries=Email.objects.filter(role__group=group, role__name="secr"),
|
||||
techadv=Email.objects.filter(role__group=group, role__name="techadv"),
|
||||
delegates=Email.objects.filter(role__group=group, role__name="delegate"),
|
||||
ad=ad_role and ad_role.person and ad_role.person.id,
|
||||
parent=group.parent.id if group.parent else None,
|
||||
list_email=group.list_email if group.list_email else None,
|
||||
|
|
@ -347,6 +369,9 @@ def edit(request, group_type=None, acronym=None, action="edit"):
|
|||
list_archive=group.list_archive if group.list_archive else None,
|
||||
urls=format_urls(group.groupurl_set.all()),
|
||||
)
|
||||
|
||||
for slug in roles_for_group_type(group_type):
|
||||
init[slug + "_roles"] = Email.objects.filter(role__group=group, role__name=slug)
|
||||
else:
|
||||
init = dict(ad=request.user.person.id if group_type == "wg" and has_role(request.user, "Area Director") else None,
|
||||
)
|
||||
|
|
@ -400,8 +425,8 @@ def customize_workflow(request, group_type=None, acronym=None):
|
|||
if not group.features.customize_workflow:
|
||||
raise Http404
|
||||
|
||||
if (not has_role(request.user, "Secretariat") and
|
||||
not group.role_set.filter(name="chair", person__user=request.user)):
|
||||
if not (can_manage_group(request.user, group)
|
||||
or group.has_role(request.user, group.features.admin_roles)):
|
||||
return HttpResponseForbidden("You don't have permission to access this view")
|
||||
|
||||
if group_type == "rg":
|
||||
|
|
|
|||
|
|
@ -1827,7 +1827,7 @@
|
|||
"fields": {
|
||||
"order": 7,
|
||||
"used": true,
|
||||
"name": "No Review of Version",
|
||||
"name": "Team Will not Review Version",
|
||||
"desc": ""
|
||||
},
|
||||
"model": "name.reviewrequeststatename",
|
||||
|
|
@ -1837,7 +1837,7 @@
|
|||
"fields": {
|
||||
"order": 8,
|
||||
"used": true,
|
||||
"name": "No Review of Document",
|
||||
"name": "Team Will not Review Document",
|
||||
"desc": ""
|
||||
},
|
||||
"model": "name.reviewrequeststatename",
|
||||
|
|
|
|||
|
|
@ -360,7 +360,7 @@ def make_test_data():
|
|||
return draft
|
||||
|
||||
def make_review_data(doc):
|
||||
team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team", list_email="reviewteam@ietf.org")
|
||||
team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="dir", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
|
||||
for r in ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]):
|
||||
ReviewTeamResult.objects.create(team=team, result=r)
|
||||
|
||||
|
|
@ -383,7 +383,7 @@ def make_review_data(doc):
|
|||
Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
|
||||
|
||||
p = Person.objects.get(user__username="secretary")
|
||||
Role.objects.create(name_id="secretary", person=p, email=p.email_set.first(), group=team)
|
||||
Role.objects.create(name_id="secr", person=p, email=p.email_set.first(), group=team)
|
||||
|
||||
return review_req
|
||||
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ def skip_prefix(text, prefix):
|
|||
else:
|
||||
return text
|
||||
|
||||
def skip_suffix(text, prefix):
|
||||
if text.endswith(prefix):
|
||||
return text[:-len(prefix)]
|
||||
def skip_suffix(text, suffix):
|
||||
if text.endswith(suffix):
|
||||
return text[:-len(suffix)]
|
||||
else:
|
||||
return text
|
||||
|
|
|
|||
Loading…
Reference in a new issue