This is the second step towards ADs out of GroupInfo into Role.

rjsparks · rjsparks · commit 65804be6838a · 2015-01-09T18:52:06.000Z
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 ietf-tools#1555 and ietf-tools#1557. Commit ready for merge. - Legacy-Id: 8854
diff --git 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
@@ -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
@@ -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
@@ -238,16 +238,17 @@ 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")
             diff('list_archive', "Mailing list archive")
 
             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,14 +299,15 @@ 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,
                         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=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
@@ -61,16 +61,16 @@ 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"):
         if r.name_id not in personnel:
             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
@@ -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 <irtf-chair@irtf.org>")
 
@@ -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 <irtf-chair@irtf.org>")
 
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -37,8 +37,8 @@ <h2>Groups - View</h2>
             </td>
         </tr>
         <tr><td>Primary Area Director:</td>
-            <td>{% if group.ad %}
-                  <a href="{% url "rolodex_view" id=group.ad.id %}">{{ group.ad }}</a>
+            <td>{% if group.ad_role %}
+                  <a href="{% url "rolodex_view" id=group.ad_role.person.id %}">{{ group.ad_role.person }}</a>
                 {% endif %} 
             </td></tr>
         <tr><td>Meeting Scheduled:</td><td>{{ group.meeting_scheduled}}</td></tr>
diff --git a/ietf/secr/templates/proceedings/proceedings.html b/ietf/secr/templates/proceedings/proceedings.html
@@ -68,10 +68,10 @@ <h3>{{ group.parent.name }} Area Director(s):</h3>
         <li><a href="mailto:{{ ad.email.address }}">{{ ad.person.name }}</a></li>
       {% endfor %}
       </ul>
-      {% if group.ad %}
-          <h3>{{ group.parent.name }} Advisor</h3>
+      {% if group.ad_role %}
+          <h3>Assigned Area Director</h3>
           <ul>
-              <li><a href="mailto:{{ group.ad.email_address }}">{{ group.ad }}</a></li>
+              <li><a href="mailto:{{ group.ad_role.email.address }}">{{ group.ad_role.person }}</a></li>
           </ul>
       {% endif %}
       {% if tas %}
diff --git 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
@@ -81,7 +81,7 @@ <h2 class="ietf-divider" id="{{area.name|cut:" "}}">{{ area.name }}</h2>
          {% for group in area.groups %}
 	 <tr>
             <td width="8%;"><a href="{% url "ietf.group.info.group_home" group_type=group.type_id acronym=group.acronym %}">{{ group.acronym }}</a></td>
-	    <td width="1%">{% for ad in area.ads %}{% if ad.person_id == group.ad_id %}<span title="AD for {{ group.acronym }}: {{ ad.person }}" class="square bgcolor{{forloop.counter}}"></span>{% endif %}{% endfor %}</td>
+	    <td width="1%">{% for ad in area.ads %}{% if ad.person_id == group.ad_role.person.id %}<span title="AD for {{ group.acronym }}: {{ ad.person }}" class="square bgcolor{{forloop.counter}}"></span>{% endif %}{% endfor %}</td>
 	    <td width="35%">{{ group.name }}</td>
 	    <td width="15%"><a href="{{ group.list_archive }}"><span title="Click to view list archive">{{ group.list_email }}</span></a></td>
 	    <td width="8%"><a href="{{ group.list_subscribe_url }}">subscribe</a></td>
diff --git a/ietf/templates/meeting/requests.html b/ietf/templates/meeting/requests.html
@@ -53,7 +53,7 @@ <h2><a name="{{session.group.parent.acronym}}">{{session.group.parent.acronym|up
     <a href="mailto:{{session.requested_by.email_address}}">{{session.requested_by}}</a>
   </td>
   <td valign="top">
-    {% if session.group.ad %}<a href="mailto:{{session.group.ad.email_address}}">{{session.group.ad}}</a> {%endif%}
+    {% if session.group.ad_role %}<a href="mailto:{{session.group.ad_role.email.address}}">{{session.group.ad_role.person}}</a> {%endif%}
   </td>
   <td width="30%" valign="top">{%if session.requested_duration%}<span>{% for constraint in session.constraints %}{%ifchanged%}</span><span class="{{constraint.name.slug}}">{%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 %}</span>{%endif%}</td>
   <td width="30%" valign="top">{% if session.comments %}<i><font color="#800000">{{session.comments|linebreaksbr}}</font></i>{% endif %}</td>
diff --git 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()
 
 
