Rewrote Document.href() to not do database queries when possible, as that has a big performance impact. Fixed a number of tests which relied on href() not doing the right thing for simplified test data. Added caching of canonical_name(), which can be quite heavy. Additional refactoring in a number of places, to use better test data and avoid test failures for good code :-)

 - Legacy-Id: 12226

Author: levkowetz

ietf/doc/models.py

@@ -92,7 +92,7 @@ def get_file_path(self):
         else:
             return settings.DOCUMENT_PATH_PATTERN.format(doc=self)
 
-    def href(self):
+    def href(self, meeting=None):
         """
         Returns an url to the document text.  This differs from .get_absolute_url(),
         which returns an url to the datatracker page for the document.   
@@ -105,31 +105,44 @@ def href(self):
         # better named, or regularize the filename based on self.name
         if not hasattr(self, '_cached_href'):
             validator = URLValidator()
-            try:
-                validator(self.external_url)
-                return self.external_url
-            except ValidationError:
-                pass
-
-            meeting_related = self.meeting_related()
+            if self.external_url and self.external_url.split(':')[0] in validator.schemes:
+                try:
+                    validator(self.external_url)
+                    return self.external_url
+                except ValidationError:
+                    pass
 
-            settings_var = settings.DOC_HREFS
-            if meeting_related:
-                settings_var = settings.MEETING_DOC_HREFS
 
-            try:
-                format = settings_var[self.type_id]
-            except KeyError:
+            if self.type_id in settings.DOC_HREFS and self.type_id in settings.MEETING_DOC_HREFS:
+                if self.meeting_related():
+                    self.is_meeting_related = True
+                    format = settings.MEETING_DOC_HREFS[self.type_id]
+                else:
+                    self.is_meeting_related = False
+                    format = settings.DOC_HREFS[self.type_id]
+            elif self.type_id in settings.DOC_HREFS:
+                self.is_meeting_related = False
+                format = settings.DOC_HREFS[self.type_id]
+            elif self.type_id in settings.MEETING_DOC_HREFS:
+                self.is_meeting_related = True
+                format = settings.MEETING_DOC_HREFS[self.type_id]
+            else:
                 if len(self.external_url):
                     return self.external_url
-                return None
-
-            meeting = None
-            if meeting_related:
-                doc = self.doc if isinstance(self, DocHistory) else self
-                meeting = doc.session_set.first().meeting
+                else:
+                    return None
+
+            if self.is_meeting_related:
+                if not meeting:
+                    # we need to do this because DocHistory items don't have
+                    # any session_set entry:
+                    doc = self.doc if isinstance(self, DocHistory) else self
+                    meeting = doc.session_set.first().meeting
+                info = dict(doc=self, meeting=meeting)
+            else:
+                info = dict(doc=self)
 
-            self._cached_href = format.format(doc=self,meeting=meeting)
+            self._cached_href = format.format(**info)
         return self._cached_href
 
     def set_state(self, state):
@@ -424,20 +437,21 @@ def latest_event(self, *args, **filter_args):
         return e[0] if e else None
 
     def canonical_name(self):
-        from ietf.doc.utils_charter import charter_name_for_group # Imported locally to avoid circular imports
-        if hasattr(self, '_canonical_name'):
-            return self._canonical_name
-        name = self.name
-        if self.type_id == "draft" and self.get_state_slug() == "rfc":
-            a = self.docalias_set.filter(name__startswith="rfc")
-            if a:
-                name = a[0].name
-        elif self.type_id == "charter":
-            try:
-                name = charter_name_for_group(self.chartered_group)
-            except Group.DoesNotExist:
-                pass
-        return name
+        if not hasattr(self, '_canonical_name'):
+            name = self.name
+            if self.type_id == "draft" and self.get_state_slug() == "rfc":
+                a = self.docalias_set.filter(name__startswith="rfc")
+                if a:
+                    name = a[0].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
+
 
     def canonical_docalias(self):
         return self.docalias_set.get(name=self.name)

ietf/doc/tests.py

@@ -27,6 +27,7 @@
 from ietf.group.factories import GroupFactory
 from ietf.meeting.models import Meeting, Session, SessionPresentation
 from ietf.meeting.factories import SessionFactory
+from ietf.meeting.test_data import make_meeting_test_data
 from ietf.name.models import SessionStatusName
 from ietf.person.models import Person
 from ietf.person.factories import PersonFactory
@@ -117,6 +118,7 @@ def test_search(self):
 
     def test_search_for_name(self):
         draft = make_test_data()
+        make_meeting_test_data()
         draft.save_with_history([DocEvent.objects.create(doc=draft, type="changed_document", by=Person.objects.get(user__username="secretary"), desc="Test")])
 
         prev_rev = draft.rev
@@ -559,6 +561,7 @@ def test_document_draft(self):
 
     def test_document_primary_and_history_views(self):
         make_test_data()
+        make_meeting_test_data()
 
         # Ensure primary views of both current and historic versions of documents works
         for docname in ["draft-imaginary-independent-submission",
@@ -567,7 +570,7 @@ def test_document_primary_and_history_views(self):
                         "charter-ietf-mars",
                         "agenda-42-mars",
                         "minutes-42-mars",
-                        "slides-42-mars-1",
+                        "slides-42-mars-1-active",
                         # TODO: add
                         #"bluesheets-42-mars-1",
                         #"recording-42-mars-1-00",

ietf/liaisons/views.py

@@ -9,6 +9,8 @@
 from django.http import HttpResponse, HttpResponseForbidden
 from django.shortcuts import render, get_object_or_404, redirect
 
+import debug                            # pyflakes:ignore
+
 from ietf.doc.models import Document
 from ietf.ietfauth.utils import role_required, has_role
 from ietf.group.models import Group, Role

ietf/meeting/models.py

@@ -1020,9 +1020,12 @@ def agenda(self):
         return self._agenda_cache
 
     def minutes(self):
-        return self.get_material("minutes", only_one=True)
+        if not hasattr(self, '_cached_minutes'):
+            self._cached_minutes = self.get_material("minutes", only_one=True)
+        return self._cached_minutes
 
     def recordings(self):
+        
         return list(self.get_material("recording", only_one=False))
 
     def bluesheets(self):

ietf/meeting/test_data.py

@@ -1,6 +1,6 @@
 import datetime
 
-from ietf.doc.models import Document, State
+from ietf.doc.factories import DocumentFactory
 from ietf.group.models import Group
 from ietf.meeting.models import (Meeting, Room, TimeSlot, Session, Schedule, SchedTimeSessAssignment,
     ResourceAssociation, SessionPresentation, UrlResource)
@@ -96,22 +96,22 @@ def make_meeting_test_data(meeting=None):
     meeting.unofficial_schedule = unofficial_schedule
 
 
-    doc = Document.objects.create(name='agenda-mars-ietf-42', type_id='agenda', title="Agenda", external_url="agenda-mars.txt",group=mars,rev='00')
-    doc.set_state(State.objects.get(type=doc.type_id, slug="active"))
-    mars_session.sessionpresentation_set.add(SessionPresentation(session=mars_session,document=doc,rev=doc.rev))
+    doc = DocumentFactory.create(name='agenda-42-mars', type_id='agenda', title="Agenda",
+        external_url="agenda-42-mars.txt", group=mars, rev='00', states=[('draft','active')])
+    mars_session.sessionpresentation_set.add(SessionPresentation(session=mars_session,document=doc,rev=doc.rev)) # 
 
-    doc = Document.objects.create(name='minutes-mars-ietf-42', type_id='minutes', title="Minutes", external_url="minutes-mars.txt",group=mars,rev='00')
-    doc.set_state(State.objects.get(type=doc.type_id, slug="active"))
+    doc = DocumentFactory.create(name='minutes-42-mars', type_id='minutes', title="Minutes",
+        external_url="minutes-42-mars.txt", group=mars, rev='00', states=[('minutes','active')])
     mars_session.sessionpresentation_set.add(SessionPresentation(session=mars_session,document=doc,rev=doc.rev))
 
-    doc = Document.objects.create(name='slides-mars-ietf-42', type_id='slides', title="Slideshow", external_url="slides-mars.txt",group=mars,rev='00')
-    doc.set_state(State.objects.get(type=doc.type_id, slug="active"))
-    doc.set_state(State.objects.get(type='reuse_policy',slug='single'))
+    doc = DocumentFactory.create(name='slides-42-mars-1-active', type_id='slides', title="Slideshow",
+        external_url="slides-42-mars.txt", group=mars, rev='00',
+        states=[('slides','active'), ('reuse_policy', 'single')])
     mars_session.sessionpresentation_set.add(SessionPresentation(session=mars_session,document=doc,rev=doc.rev))
 
-    doc = Document.objects.create(name='slides-mars-ietf-42-deleted', type_id='slides', title="Bad Slideshow", external_url="slides-mars-deleted.txt",group=mars,rev='00')
-    doc.set_state(State.objects.get(type=doc.type_id, slug="deleted"))
-    doc.set_state(State.objects.get(type='reuse_policy',slug='single'))
+    doc = DocumentFactory.create(name='slides-42-mars-2-deleted', type_id='slides',
+        title="Bad Slideshow", external_url="slides-42-mars-2-deleted.txt", group=mars, rev='00',
+        states=[('slides','deleted'), ('reuse_policy', 'single')])
     mars_session.sessionpresentation_set.add(SessionPresentation(session=mars_session,document=doc,rev=doc.rev))
 
     # Future Interim Meetings

ietf/meeting/views.py

@@ -109,8 +109,10 @@ def materials(request, num=None):
 
     #sessions  = Session.objects.filter(meeting__number=meeting.number, timeslot__isnull=False)
     schedule = get_schedule(meeting, None)
-    sessions  = Session.objects.filter(meeting__number=meeting.number,
-        timeslotassignments__schedule=schedule).select_related('meeting__agenda','status','group__state','group__parent')
+    sessions  = ( Session.objects
+        .filter(meeting__number=meeting.number, timeslotassignments__schedule=schedule)
+        .select_related('meeting__agenda','status','group__state','group__parent', )
+    )
     for session in sessions:
         session.past_cutoff_date = past_cutoff_date
     plenaries = sessions.filter(name__icontains='plenary')

ietf/settings.py

@@ -636,6 +636,7 @@ def skip_unreadable_post(record):
 # Set debug apps in DEV_APPS settings_local
 DEV_APPS = ()
 DEV_MIDDLEWARE_CLASSES = ()
+DEV_TEMPLATE_CONTEXT_PROCESSORS = ()
 
 # Domain which hosts draft and wg alias lists
 DRAFT_ALIAS_DOMAIN = IETF_DOMAIN
@@ -742,6 +743,7 @@ def skip_unreadable_post(record):
 # Add DEV_APPS to INSTALLED_APPS
 INSTALLED_APPS += DEV_APPS
 MIDDLEWARE_CLASSES += DEV_MIDDLEWARE_CLASSES
+TEMPLATE_CONTEXT_PROCESSORS += DEV_TEMPLATE_CONTEXT_PROCESSORS
 
 
 # We provide a secret key only for test and development modes.  It's

ietf/templates/meeting/edit_materials_button.html

@@ -1,5 +1,5 @@
 {% load ietf_filters session_filters %}
-{% if user|has_role:"Secretariat" or session|can_manage_materials:user and not session.past_cutoff_date %}
+{% if session|can_manage_materials:user and not session.past_cutoff_date %}
 {% with gt=session.group.type_id %}
         <a class="button btn-default btn-sm" href="{% url 'ietf.meeting.views.session_details' num=session.meeting.number acronym=session.group.acronym %}{% if gt == 'wg' or gt == 'rg' or gt == 'ag' %}{% else %}#session_{{session.pk}}{% endif %}">Edit</a>
 {% endwith %}

ietf/templates/meeting/group_materials.html

@@ -22,7 +22,7 @@
   {% else %}
     <td>
       {% if session.agenda %}
-        <a href="{{ session.agenda.href }}">Agenda</a>
+        <a href="https://www.ietf.org/proceedings/{{ meeting_num }}/agenda/{{ session.agenda.external_url }}">Agenda</a>
       {% else %}
         {% if show_agenda == "True" %}
           <span class="label label-warning">No agenda</span>
@@ -31,7 +31,7 @@
     </td>
     <td>
       {% if session.minutes %}
-        <a href="https://www.ietf.org/proceedings/{{ meeting_num }}/minutes/{{ session.minutes }}">Minutes</a>
+        <a href="https://www.ietf.org/proceedings/{{ meeting_num }}/minutes/{{ session.minutes.external_url }}">Minutes</a>
       {% else %}
         {% if show_agenda == "True" %}
           <span class="label label-warning">No minutes</span>
@@ -41,7 +41,7 @@
     <td>
       {% with session.slides as slides %}
         {% for slide in slides %}
-          <a href="https://www.ietf.org/proceedings/{{meeting_num}}/slides/{{ slide.external_url }}">{{ slide.title|clean_whitespace }}</a>
+          <a href="https://www.ietf.org/proceedings/{{ meeting_num }}/slides/{{ slide.external_url }}">{{ slide.title|clean_whitespace }}</a>
           <br>
         {% empty %}
           <span class="label label-warning">No slides</span>
@@ -51,7 +51,7 @@
     <td>
      {% with session.all_meeting_drafts as drafts %}
        {% for draft in drafts %}
-         <a href="{% url "doc_view" name=draft.canonical_name %}">{{ draft.canonical_name }}</a><br>
+         <a href="{{ draft.href }}">{{ draft.name }}</a><br>
        {% empty %} 
          <span class="label label-warning">No drafts</span>
        {% endfor %}

ietf/utils/test_data.py

@@ -375,17 +375,7 @@ def rfc_for_status_change_test_factory(name,rfc_num,std_level_id):
 
     # Instances of the remaining document types 
     # (Except liaison, liai-att, and recording  which the code in ietf.doc does not use...)
-    def other_doc_factory(type_id,name):
-        doc = Document.objects.create(type_id=type_id,name=name,rev='00',group=mars_wg)
-        DocAlias.objects.create(name=name,document=doc)
-        doc.set_state(State.objects.get(type__slug=doc.type.slug,slug='active'))
-        if type_id=='slides':
-            doc.set_state(State.objects.get(type='reuse_policy',slug='single'))
-    other_doc_factory('agenda','agenda-42-mars')
-    other_doc_factory('minutes','minutes-42-mars')
-    other_doc_factory('slides','slides-42-mars-1')
-    # TODO: add
-    #other_doc_factory('bluesheets','bluesheets-42-mars-1')
-    #other_doc_factory('recording','recording-42-mars-1-00')
+    # Meeting-related documents are created in make_meeting_test_data, and
+    # associated with a session
 
     return draft