From 65804be6838a4f4fba62a7975e27820ecf70a816 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Fri, 9 Jan 2015 18:52:06 +0000 Subject: [PATCH] This is the second step towards ADs out of GroupInfo into Role. The use of group.ad has been scrubbed from the code and templates. - Those places that set group.ad have been directly manipulate Role objects instead - Most places that read group.ad now use a new group.ad_role() that returns a Role object, simplifing some views. Related to #1555 and #1557. Commit ready for merge. - Legacy-Id: 8854 --- ietf/doc/utils_charter.py | 2 -- ietf/doc/views_charter.py | 2 +- ietf/doc/views_draft.py | 2 +- ietf/group/edit.py | 8 +++++--- ietf/group/info.py | 6 +++--- ietf/group/mails.py | 8 ++++---- ietf/group/models.py | 14 ++++++++++---- ietf/group/tests_info.py | 10 +++++----- ietf/group/utils.py | 2 +- ietf/iesg/views.py | 6 +++--- ietf/secr/areas/views.py | 4 +--- ietf/secr/sreq/views.py | 4 +--- ietf/secr/templates/groups/view.html | 4 ++-- ietf/secr/templates/proceedings/proceedings.html | 6 +++--- ietf/templates/doc/charter/group_info.txt | 4 ++-- ietf/templates/group/active_wgs.html | 2 +- ietf/templates/meeting/requests.html | 2 +- ietf/utils/test_data.py | 2 +- 18 files changed, 45 insertions(+), 43 deletions(-) diff --git a/ietf/doc/utils_charter.py b/ietf/doc/utils_charter.py index 4d5e17ee90..b3812b11c0 100644 --- a/ietf/doc/utils_charter.py +++ b/ietf/doc/utils_charter.py @@ -125,7 +125,6 @@ def default_action_text(group, charter, by): secr=group.role_set.filter(name="secr"), techadv=group.role_set.filter(name="techadv"), milestones=group.groupmilestone_set.filter(state="charter"), - ad_email=group.ad.role_email("ad") if group.ad else None, action_type=action, )) @@ -145,7 +144,6 @@ def default_review_text(group, charter, by): secr=group.role_set.filter(name="secr"), techadv=group.role_set.filter(name="techadv"), milestones=group.groupmilestone_set.filter(state="charter"), - ad_email=group.ad.role_email("ad") if group.ad else None, review_date=(datetime.date.today() + datetime.timedelta(weeks=1)).isoformat(), review_type="new" if group.state_id == "proposed" else "recharter", ) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index e277750ff1..e0ccc8dd58 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -387,7 +387,7 @@ def submit(request, name=None, option=None): form.save(group, charter.rev) if option in ['initcharter','recharter'] and charter.ad == None: - charter.ad = group.ad + charter.ad = getattr(group.ad_role(),'person',None) charter.time = datetime.datetime.now() charter.save() diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index 5b147a83e8..b0c7be5720 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -534,7 +534,7 @@ def to_iesg(request,name): notify = doc.notify if not notify: notify = get_initial_notify(doc) - ad = doc.ad or doc.group.ad + ad = doc.ad or getattr(doc.group.ad_role(),'person',None) if request.method == 'POST': diff --git a/ietf/group/edit.py b/ietf/group/edit.py index 8a815e9874..8ad0dc69be 100644 --- a/ietf/group/edit.py +++ b/ietf/group/edit.py @@ -238,7 +238,6 @@ def diff(attr, name): diff('name', "Name") diff('acronym', "Acronym") diff('state', "State") - diff('ad', "Shepherding AD") diff('parent', "IETF Area") diff('list_email', "Mailing list email") diff('list_subscribe', "Mailing list subscribe address") @@ -246,8 +245,10 @@ def diff(attr, name): personnel_change_text="" # update roles - for attr, slug, title in [('chairs', 'chair', "Chairs"), ('secretaries', 'secr', "Secretaries"), ('techadv', 'techadv', "Tech Advisors"), ('delegates', 'delegate', "Delegates")]: + for attr, slug, title in [('ad','ad','Shepherding AD'), ('chairs', 'chair', "Chairs"), ('secretaries', 'secr', "Secretaries"), ('techadv', 'techadv', "Tech Advisors"), ('delegates', 'delegate', "Delegates")]: new = clean[attr] + if attr == 'ad': + 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(desc(title, @@ -298,6 +299,7 @@ def diff(attr, name): return HttpResponseRedirect(group.about_url()) else: # form.is_valid() if not new_group: + ad_role = group.ad_role() init = dict(name=group.name, acronym=group.acronym, state=group.state, @@ -305,7 +307,7 @@ def diff(attr, name): 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=group.ad_id if group.ad else None, + 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, list_subscribe=group.list_subscribe if group.list_subscribe else None, diff --git a/ietf/group/info.py b/ietf/group/info.py index 6b91e2186d..090c3c388a 100644 --- a/ietf/group/info.py +++ b/ietf/group/info.py @@ -61,7 +61,7 @@ def roles(group, role_name): return Role.objects.filter(group=group, name=role_name).select_related("email", "person") def fill_in_charter_info(group, include_drafts=False): - group.areadirector = group.ad.role_email("ad", group.parent) if group.ad else None + group.areadirector = getattr(group.ad_role(),'email',None) personnel = {} for r in Role.objects.filter(group=group).select_related("email", "person", "name"): @@ -69,8 +69,8 @@ def fill_in_charter_info(group, include_drafts=False): personnel[r.name_id] = [] personnel[r.name_id].append(r) - if group.parent and group.parent.type_id == "area" and group.ad and "ad" not in personnel: - ad_roles = list(Role.objects.filter(group=group.parent, name="ad", person=group.ad)) + if group.parent and group.parent.type_id == "area" and group.ad_role() and "ad" not in personnel: + ad_roles = list(Role.objects.filter(group=group.parent, name="ad", person=group.ad_role().person)) if ad_roles: personnel["ad"] = ad_roles diff --git a/ietf/group/mails.py b/ietf/group/mails.py index c56a179f27..6f3c7239f3 100644 --- a/ietf/group/mails.py +++ b/ietf/group/mails.py @@ -44,8 +44,8 @@ def wrap_up_email(to, text): # first send to management and chairs to = [] - if group.ad: - to.append(group.ad.role_email("ad").formatted_email()) + if group.ad_role(): + to.append(group.ad_role().email.formatted_email()) elif group.type_id == "rg": to.append("IRTF Chair ") @@ -66,8 +66,8 @@ def email_milestone_review_reminder(group, grace_period=7): """Email reminders about milestones needing review to management.""" to = [] - if group.ad: - to.append(group.ad.role_email("ad").formatted_email()) + if group.ad_role(): + to.append(group.ad_role().email.formatted_email()) elif group.type_id == "rg": to.append("IRTF Chair ") diff --git a/ietf/group/models.py b/ietf/group/models.py index c9f1967c14..bd237fe1dc 100644 --- a/ietf/group/models.py +++ b/ietf/group/models.py @@ -36,19 +36,25 @@ def name_with_acronym(self): res += " %s (%s)" % (self.type, self.acronym) return res + def ad_role(self): + return self.role_set.filter(name='ad').first() + @property def ad(self): - ad_role = self.role_set.filter(name__slug='ad').first() + #assert(False) # These methods are deprecated - expect them to go away when the _ad field is removed + ad_role = self.ad_role() return ad_role and ad_role.person @ad.setter def ad(self,value): - self.role_set.filter(name__slug='ad').delete() + #assert(False) + self.role_set.filter(name='ad').delete() if value: self.role_set.create(name=RoleName.objects.get(slug='ad'), person=value, email=value.role_email('ad')) @property def ad_id(self): + #assert(False) return self.ad.id @property @@ -127,8 +133,8 @@ def json_dict(self, host_scheme): group1['parent_href'] = urljoin(host_scheme, self.parent.json_url()) # uncomment when people URL handle is created try: - if self.ad is not None: - group1['ad_href'] = urljoin(host_scheme, self.ad.json_url()) + if self.ad_role() is not None: + group1['ad_href'] = urljoin(host_scheme, self.ad_role().person.json_url()) except Person.DoesNotExist: pass group1['list_email'] = self.list_email diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 802eb64427..e24297f12a 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -42,7 +42,7 @@ def test_active_groups(self): self.assertTrue(group.parent.name in r.content) self.assertTrue(group.acronym in r.content) self.assertTrue(group.name in r.content) - self.assertTrue(group.ad.plain_name() in r.content) + self.assertTrue(group.ad_role().person.plain_name() in r.content) url = urlreverse('ietf.group.info.active_groups', kwargs=dict(group_type="rg")) r = self.client.get(url) @@ -77,7 +77,7 @@ def test_wg_summaries(self): self.assertEqual(r.status_code, 200) self.assertTrue(group.acronym in r.content) self.assertTrue(group.name in r.content) - self.assertTrue(group.ad.plain_name() in r.content) + self.assertTrue(group.ad_role().person.plain_name() in r.content) self.assertTrue(chair.address in r.content) self.assertTrue("This is a charter." in r.content) @@ -86,7 +86,7 @@ def test_wg_summaries(self): self.assertEqual(r.status_code, 200) self.assertTrue(group.acronym in r.content) self.assertTrue(group.name in r.content) - self.assertTrue(group.ad.plain_name() in r.content) + self.assertTrue(group.ad_role().person.plain_name() in r.content) self.assertTrue(chair.address in r.content) self.assertTrue("This is a charter." in r.content) @@ -424,7 +424,7 @@ def test_edit_info(self): group = Group.objects.get(acronym="mars") self.assertEqual(group.name, "Mars Not Special Interest Group") self.assertEqual(group.parent, area) - self.assertEqual(group.ad, ad) + self.assertEqual(group.ad_role().person, ad) for k in ("chair", "secr", "techadv"): self.assertTrue(group.role_set.filter(name=k, email__address="aread@ietf.org")) self.assertTrue(group.role_set.filter(name="delegate", email__address="ad2@ietf.org")) @@ -707,7 +707,7 @@ def test_edit_milestone(self): self.assertTrue("Changed milestone" in m.milestonegroupevent_set.all()[0].desc) self.assertEqual(len(outbox), mailbox_before + 2) self.assertTrue("Milestones changed" in outbox[-2]["Subject"]) - self.assertTrue(group.ad.role_email("ad").address in str(outbox[-2])) + self.assertTrue(group.ad_role().email.address in str(outbox[-2])) self.assertTrue("Milestones changed" in outbox[-1]["Subject"]) self.assertTrue(group.list_email in str(outbox[-1])) diff --git a/ietf/group/utils.py b/ietf/group/utils.py index b9edb5d99c..b62ecc75fb 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -61,7 +61,7 @@ def get_group_ads_emails(wg): return get_area_ads_emails(wg.parent) # As fallback, just return the single ad within the wg - return [wg.ad and wg.ad.email_address()] + return [wg.ad_role() and wg.ad_role().email.address] def get_group_chairs_emails(wg): " Get list of area chairs' emails for a given WG " diff --git a/ietf/iesg/views.py b/ietf/iesg/views.py index 25ec44aa93..7d5d5bacd8 100644 --- a/ietf/iesg/views.py +++ b/ietf/iesg/views.py @@ -114,7 +114,7 @@ def agenda_json(request, date=None): 'rev': doc.rev, 'wgname': doc.group.name, 'acronym': doc.group.acronym, - 'ad': doc.group.ad.name if doc.group.ad else None, + 'ad': doc.group.ad_role().person.name if doc.group.ad_role() else None, } # consider moving the charters to "docs" like the other documents @@ -446,8 +446,8 @@ def milestones_needing_review(request): # collect milestones, grouped on AD and group ads = {} for m in GroupMilestone.objects.filter(state="review").exclude(group__state="concluded").distinct().select_related("group"): - if m.group.ad: - groups = ads.setdefault(m.group.ad, {}) + if m.group.ad_role(): + groups = ads.setdefault(m.group.ad_role().person, {}) milestones = groups.setdefault(m.group, []) milestones.append(m) diff --git a/ietf/secr/areas/views.py b/ietf/secr/areas/views.py index b201c1ffa2..9d4d90afb0 100644 --- a/ietf/secr/areas/views.py +++ b/ietf/secr/areas/views.py @@ -274,9 +274,7 @@ def modify(request, name): role.delete() # update groups that have this AD as primary AD - for group in Group.objects.filter(ad=person,type='wg',state__in=('active','bof')): - group.ad = None - group.save() + Role.objects.filter(name__in=('ad','pre-ad'),person=person,group__type='wg',group__state__in=('active','bof')).delete() messages.success(request, 'The Area Director has been retired successfully!') diff --git a/ietf/secr/sreq/views.py b/ietf/secr/sreq/views.py index 8c25f4f115..3ddbb03cfc 100644 --- a/ietf/secr/sreq/views.py +++ b/ietf/secr/sreq/views.py @@ -314,9 +314,7 @@ def add_essential_people(group,initial): people = set() if 'bethere' in initial: people.update(initial['bethere']) - people.update(Person.objects.filter(role__group=group, role__name='chair')) - if group.ad: - people.add(group.ad) + people.update(Person.objects.filter(role__group=group, role__name__in=['chair','ad'])) initial['bethere'] = list(people) diff --git a/ietf/secr/templates/groups/view.html b/ietf/secr/templates/groups/view.html index 7c581400c6..6cb3735be6 100644 --- a/ietf/secr/templates/groups/view.html +++ b/ietf/secr/templates/groups/view.html @@ -37,8 +37,8 @@

Groups - View

Primary Area Director: - {% if group.ad %} - {{ group.ad }} + {% if group.ad_role %} + {{ group.ad_role.person }} {% endif %} Meeting Scheduled:{{ group.meeting_scheduled}} diff --git a/ietf/secr/templates/proceedings/proceedings.html b/ietf/secr/templates/proceedings/proceedings.html index 4f81613f0f..8463a97d67 100644 --- a/ietf/secr/templates/proceedings/proceedings.html +++ b/ietf/secr/templates/proceedings/proceedings.html @@ -68,10 +68,10 @@

{{ group.parent.name }} Area Director(s):

  • {{ ad.person.name }}
    • {% endfor %} - {% if group.ad %} -

    {{ group.parent.name }} Advisor

    + {% if group.ad_role %} +

    Assigned Area Director

    {% endif %} {% if tas %} diff --git a/ietf/templates/doc/charter/group_info.txt b/ietf/templates/doc/charter/group_info.txt index 4d21461ecb..3f37dbbe67 100644 --- a/ietf/templates/doc/charter/group_info.txt +++ b/ietf/templates/doc/charter/group_info.txt @@ -11,8 +11,8 @@ Current Status: {{ group.state.name }} {{ group.type.name }} {% endif %}{% if techadv %}Technical advisors: {% for r in techadv %} {{ r.person.plain_name }} <{{r.email.address}}> {% endfor %} -{% endif %}{% if group.ad %}Assigned Area Director: - {{ group.ad.plain_name }} <{{ ad_email }}> +{% endif %}{% if group.ad_role %}Assigned Area Director: + {{ group.ad_role.person.plain_name }} <{{ group.ad_role.email.address }}> {% endif %}{% if group.list_email %}Mailing list Address: {{ group.list_email }} diff --git a/ietf/templates/group/active_wgs.html b/ietf/templates/group/active_wgs.html index d4dbeac854..fa7bd79310 100644 --- a/ietf/templates/group/active_wgs.html +++ b/ietf/templates/group/active_wgs.html @@ -81,7 +81,7 @@

    {{ area.name }}

    {% for group in area.groups %} {{ group.acronym }} - {% for ad in area.ads %}{% if ad.person_id == group.ad_id %}{% endif %}{% endfor %} + {% for ad in area.ads %}{% if ad.person_id == group.ad_role.person.id %}{% endif %}{% endfor %} {{ group.name }} {{ group.list_email }} subscribe diff --git a/ietf/templates/meeting/requests.html b/ietf/templates/meeting/requests.html index 204072c5c2..b628df0283 100644 --- a/ietf/templates/meeting/requests.html +++ b/ietf/templates/meeting/requests.html @@ -53,7 +53,7 @@

    {{session.group.parent.acronym|up {{session.requested_by}} - {% if session.group.ad %}{{session.group.ad}} {%endif%} + {% if session.group.ad_role %}{{session.group.ad_role.person}} {%endif%} {%if session.requested_duration%}{% for constraint in session.constraints %}{%ifchanged%}{%endifchanged%} {% if constraint %} {% if not forloop.first %}, {%endif%}{{constraint.brief_display}}{% if constraint.target.parent.id != constraint.source.parent.id and not constraint.person %} ({{constraint.target.parent.acronym}}){%endif%} {% endif %} {% endfor %}{%endif%} {% if session.comments %}{{session.comments|linebreaksbr}}{% endif %} diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index ea887c7aad..0c33d966af 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -178,7 +178,7 @@ def make_test_data(): create_person(mars_wg, "chair", name="WG Chair Man", username="marschairman") create_person(mars_wg, "delegate", name="WG Delegate", username="marsdelegate") - mars_wg.ad = ad + mars_wg.role_set.get_or_create(name_id='ad',person=ad,email=ad.role_email('ad')) mars_wg.save()