Remove usage of log_state_changed (and duplicate in

doc/utils_charter.py) in favour of add_state_change_event which has
slightly better API but otherwise does basically the same except it
sets an event type we can later search for. Also expand tests slightly
to exercise three more templates.
 - Legacy-Id: 7129

Author: OleLaursen

ietf/doc/expire.py

@@ -10,7 +10,7 @@
 from ietf.doc.models import Document, DocEvent, State, save_document_in_history, IESG_SUBSTATE_TAGS
 from ietf.person.models import Person, Email
 from ietf.meeting.models import Meeting
-from ietf.doc.utils import log_state_changed
+from ietf.doc.utils import add_state_change_event
 
 def expirable_draft(draft):
     """Return whether draft is in an expirable state or not. This is
@@ -122,20 +122,18 @@ def expire_draft(doc):
     # clean up files
     move_draft_files_to_archive(doc, doc.rev)
 
-    # change the state
     system = Person.objects.get(name="(System)")
 
+    # change the state
     save_document_in_history(doc)
     if doc.latest_event(type='started_iesg_process'):
-        dead_state = State.objects.get(used=True, type="draft-iesg", slug="dead")
-        prev_state = doc.friendly_state()
-        prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-        prev_tag = prev_tag[0] if prev_tag else None
-        if doc.get_state("draft-iesg") != dead_state:
-            doc.set_state(dead_state)
-            if prev_tag:
-                doc.tags.remove(prev_tag)
-            log_state_changed(None, doc, system, doc.friendly_state(), prev_state)
+        new_state = State.objects.get(used=True, type="draft-iesg", slug="dead")
+        prev_state = doc.get_state(new_state.type_id)
+        prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
+        if new_state != prev_state:
+            doc.set_state(new_state)
+            doc.tags.remove(*prev_tags)
+            e = add_state_change_event(doc, system, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
 
         e = DocEvent(doc=doc, by=system)
         e.type = "expired_document"

ietf/doc/lastcall.py

@@ -6,7 +6,7 @@
 
 from ietf.doc.models import *
 from ietf.person.models import Person
-from ietf.doc.utils import log_state_changed
+from ietf.doc.utils import add_state_change_event
 from ietf.doc.mails import *
 
 def request_last_call(request, doc):
@@ -36,30 +36,27 @@ def get_expired_last_calls():
 
 def expire_last_call(doc):
     if doc.type_id == 'draft':
-        state = State.objects.get(used=True, type="draft-iesg", slug="writeupw")
+        new_state = State.objects.get(used=True, type="draft-iesg", slug="writeupw")
         e = doc.latest_event(WriteupDocEvent, type="changed_ballot_writeup_text")
         if e and "Relevant content can frequently be found in the abstract" not in e.text:
             # if boiler-plate text has been removed, we assume the
             # write-up has been written
-            state = State.objects.get(used=True, type="draft-iesg", slug="goaheadw")
-        prev = doc.get_state("draft-iesg")
+            new_state = State.objects.get(used=True, type="draft-iesg", slug="goaheadw")
     elif doc.type_id == 'statchg':
-        state = State.objects.get(used=True, type="statchg", slug="goahead")
-        prev = doc.get_state("statchg")
+        new_state = State.objects.get(used=True, type="statchg", slug="goahead")
     else:
         raise ValueError("Unexpected document type to expire_last_call(): %s" % doc.type)
 
-    prev_state = doc.friendly_state()
-
     save_document_in_history(doc)
-    doc.set_state(state)
 
-    prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-    prev_tag = prev_tag[0] if prev_tag else None
-    if prev_tag:
-        doc.tags.remove(prev_tag)
+    prev_state = doc.get_state(new_state.type_id)
+    doc.set_state(new_state)
+
+    prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
+    doc.tags.remove(*prev_tags)
 
-    e = log_state_changed(None, doc, Person.objects.get(name="(System)"), doc.friendly_state(), prev_state)
+    system = Person.objects.get(name="(System)")
+    e = add_state_change_event(doc, system, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
 
     doc.time = e.time
     doc.save()

ietf/doc/models.py

@@ -392,7 +392,7 @@ def friendly_state(self):
                 # Expired/Withdrawn by Submitter/IETF
                 return state.name
         else:
-           return state.name
+            return state.name
 
     def ipr(self):
         """Returns the IPR disclosures against this document (as a queryset over IprDocAlias)."""

ietf/doc/tests_charter.py

@@ -51,7 +51,7 @@ def test_startstop_process(self):
             if option == "abandon":
                 self.assertTrue("abandoned" in charter.latest_event(type="changed_document").desc.lower())
             else:
-                self.assertTrue("state changed" in charter.latest_event(type="changed_document").desc.lower())
+                self.assertTrue("state changed" in charter.latest_event(type="changed_state").desc.lower())
 
     def test_change_state(self):
         make_test_data()
@@ -94,7 +94,7 @@ def test_change_state(self):
             def find_event(t):
                 return [e for e in charter.docevent_set.all()[:events_now - events_before] if e.type == t]
 
-            self.assertTrue("state changed" in find_event("changed_document")[0].desc.lower())
+            self.assertTrue("state changed" in find_event("changed_state")[0].desc.lower())
 
             if slug in ("intrev", "iesgrev"):
                 self.assertTrue(find_event("created_ballot"))
@@ -111,6 +111,10 @@ def test_edit_telechat_date(self):
         url = urlreverse('charter_telechat_date', kwargs=dict(name=charter.name))
         login_testing_unauthorized(self, "secretary", url)
 
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
         # add to telechat
         self.assertTrue(not charter.latest_event(TelechatDocEvent, "scheduled_for_telechat"))
         telechat_date = TelechatDate.objects.active()[0].date
@@ -145,6 +149,10 @@ def test_edit_notify(self):
         url = urlreverse('charter_edit_notify', kwargs=dict(name=charter.name))
         login_testing_unauthorized(self, "secretary", url)
 
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
         # post
         self.assertTrue(not charter.notify)
         r = self.client.post(url, dict(notify="someone@example.com, someoneelse@example.com"))

ietf/doc/tests_draft.py

@@ -67,7 +67,7 @@ def test_change_state(self):
         self.assertTrue(draft.tags.filter(slug="point"))
         self.assertEqual(draft.docevent_set.count(), events_before + 2)
         self.assertTrue("Test comment" in draft.docevent_set.all()[0].desc)
-        self.assertTrue("State changed" in draft.docevent_set.all()[1].desc)
+        self.assertTrue("IESG state changed" in draft.docevent_set.all()[1].desc)
         self.assertEqual(len(outbox), mailbox_before + 2)
         self.assertTrue("State Update Notice" in outbox[-2]['Subject'])
         self.assertTrue(draft.name in outbox[-1]['Subject'])
@@ -232,6 +232,10 @@ def test_edit_telechat_date(self):
                     note="",
                     )
 
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
         # add to telechat
         self.assertTrue(not draft.latest_event(TelechatDocEvent, type="scheduled_for_telechat"))
         data["telechat_date"] = TelechatDate.objects.active()[0].date.isoformat()
@@ -347,6 +351,11 @@ def test_edit_consensus(self):
         url = urlreverse('doc_edit_consensus', kwargs=dict(name=draft.name))
         login_testing_unauthorized(self, "secretary", url)
 
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
+        # post
         self.assertTrue(not draft.latest_event(ConsensusDocEvent, type="changed_consensus"))
         r = self.client.post(url, dict(consensus="Yes"))
         self.assertEqual(r.status_code, 302)

ietf/doc/utils.py

@@ -220,25 +220,24 @@ def get_document_content(key, filename, split=True, markup=True):
     else:
         return raw_content
 
-def log_state_changed(request, doc, by, new_description, old_description):
-    e = DocEvent(doc=doc, by=by)
-    e.type = "changed_document"
-    e.desc = u"State changed to <b>%s</b> from %s" % (new_description, old_description)
-    e.save()
-    return e
-
-def add_state_change_event(doc, by, prev_state, new_state, timestamp=None):
+def add_state_change_event(doc, by, prev_state, new_state, prev_tags=[], new_tags=[], timestamp=None):
     """Add doc event to explain that state change just happened."""
-    if prev_state == new_state:
+    if prev_state and new_state:
+        assert prev_state.type_id == new_state.type_id
+
+    if prev_state == new_state and set(prev_tags) == set(new_tags):
         return None
 
+    def tags_suffix(tags):
+        return (u"::" + u"::".join(t.name for t in tags)) if tags else u""
+
     e = StateDocEvent(doc=doc, by=by)
     e.type = "changed_state"
     e.state_type = (prev_state or new_state).type
     e.state = new_state
-    e.desc = "%s changed to <b>%s</b>" % (e.state_type.label, new_state.name)
+    e.desc = "%s changed to <b>%s</b>" % (e.state_type.label, new_state.name + tags_suffix(new_tags))
     if prev_state:
-        e.desc += " from %s" % prev_state.name
+        e.desc += " from %s" % (prev_state.name + tags_suffix(prev_tags))
     if timestamp:
         e.time = timestamp
     e.save()

ietf/doc/utils_charter.py

@@ -13,15 +13,6 @@
 from ietf.utils.history import find_history_active_at
 
 
-def log_state_changed(request, doc, by, prev_state):
-    e = DocEvent(doc=doc, by=by)
-    e.type = "changed_document"
-    e.desc = u"State changed to <b>%s</b> from %s" % (
-        doc.get_state().name,
-        prev_state.name if prev_state else "None")
-    e.save()
-    return e
-
 def next_revision(rev):
     if rev == "":
         return "00-00"

ietf/doc/views_ballot.py

@@ -49,17 +49,21 @@ def do_undefer_ballot(request, doc):
     telechat_date = TelechatDate.objects.active().order_by("date")[0].date
     save_document_in_history(doc)
 
-    prev_state = doc.friendly_state()
+    new_state = doc.get_state()
+    prev_tags = new_tags = []
+
     if doc.type_id == 'draft':
-        doc.set_state(State.objects.get(used=True, type="draft-iesg", slug='iesg-eva'))
-        prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-        prev_tag = prev_tag[0] if prev_tag else None
-        if prev_tag:
-            doc.tags.remove(prev_tag)
+        new_state = State.objects.get(used=True, type="draft-iesg", slug='iesg-eva')
+        prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
     elif doc.type_id == 'conflrev':
-        doc.set_state(State.objects.get(used=True, type='conflrev',slug='iesgeval'))
+        new_state = State.objects.get(used=True, type='conflrev',slug='iesgeval')
+
+    prev_state = doc.get_state(new_state.type_id if new_state else None)
 
-    e = log_state_changed(request, doc, login, doc.friendly_state(), prev_state)
+    doc.set_state(new_state)
+    doc.tags.remove(*prev_tags)
+
+    e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags)
 
     doc.time = e.time
     doc.save()
@@ -353,17 +357,21 @@ def defer_ballot(request, name):
     if request.method == 'POST':
         save_document_in_history(doc)
 
-        prev_state = doc.friendly_state()
+        new_state = doc.get_state()
+        prev_tags = new_tags = []
+
         if doc.type_id == 'draft':
-            doc.set_state(State.objects.get(used=True, type="draft-iesg", slug='defer'))
-            prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-            prev_tag = prev_tag[0] if prev_tag else None
-            if prev_tag:
-                doc.tags.remove(prev_tag)
+            new_state = State.objects.get(used=True, type="draft-iesg", slug='defer')
+            prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
         elif doc.type_id == 'conflrev':
-            doc.set_state(State.objects.get(used=True, type='conflrev', slug='defer'))
+            new_state = State.objects.get(used=True, type='conflrev', slug='defer')
+
+        prev_state = doc.get_state(new_state.type_id if new_state else None)
+
+        doc.set_state(new_state)
+        doc.tags.remove(*prev_tags)
 
-        e = log_state_changed(request, doc, login, doc.friendly_state(), prev_state)
+        e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags)
 
         doc.time = e.time
         doc.save()
@@ -445,16 +453,16 @@ def lastcalltext(request, name):
                 if "send_last_call_request" in request.POST:
                     save_document_in_history(doc)
 
-                    prev_state = doc.friendly_state()
-                    doc.set_state(State.objects.get(used=True, type="draft-iesg", slug='lc-req'))
+                    prev_state = doc.get_state("draft-iesg")
+                    new_state = State.objects.get(used=True, type="draft-iesg", slug='lc-req')
 
-                    prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-                    prev_tag = prev_tag[0] if prev_tag else None
-                    if prev_tag:
-                        doc.tags.remove(prev_tag)
+                    prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
+
+                    doc.set_state(new_state)
+                    doc.tags.remove(*prev_tags)
+
+                    e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
 
-                    e = log_state_changed(request, doc, login, doc.friendly_state(), prev_state)
-                    
                     doc.time = e.time
                     doc.save()
 
@@ -664,8 +672,9 @@ def approve_ballot(request, name):
         else:
             new_state = State.objects.get(used=True, type="draft-iesg", slug="ann")
 
-        prev_friendly_state = doc.friendly_state()
         prev_state = doc.get_state("draft-iesg")
+        prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
+
         if new_state.slug == "ann" and new_state.slug != prev_state.slug and not request.REQUEST.get("skiprfceditorpost"):
             # start by notifying the RFC Editor
             import ietf.sync.rfceditor
@@ -677,18 +686,14 @@ def approve_ballot(request, name):
                                        error=error),
                                   context_instance=RequestContext(request))
 
-        # fixup document
-        close_open_ballots(doc, login)
-
         save_document_in_history(doc)
 
         doc.set_state(new_state)
-
-        prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-        prev_tag = prev_tag[0] if prev_tag else None
-        if prev_tag:
-            doc.tags.remove(prev_tag)
+        doc.tags.remove(*prev_tags)
 
+        # fixup document
+        close_open_ballots(doc, login)
+
         e = DocEvent(doc=doc, by=login)
         if action == "do_not_publish":
             e.type = "iesg_disapproved"
@@ -701,8 +706,8 @@ def approve_ballot(request, name):
 
         change_description = e.desc + " and state has been changed to %s" % doc.get_state("draft-iesg").name
 
-        e = log_state_changed(request, doc, login, doc.friendly_state(), prev_friendly_state)
-                    
+        e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
+
         doc.time = e.time
         doc.save()
 
@@ -766,26 +771,27 @@ def make_last_call(request, name):
 
             save_document_in_history(doc)
 
-            prev_state = doc.get_state("draft-iesg")
+            new_state = doc.get_state()
+            prev_tags = new_tags = []
+
             if doc.type.slug == 'draft':
-                doc.set_state(State.objects.get(used=True, type="draft-iesg", slug='lc'))
+                new_state = State.objects.get(used=True, type="draft-iesg", slug='lc')
+                prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
+            elif doc.type.slug == 'statchg':
+                new_state = State.objects.get(used=True, type="statchg", slug='in-lc')
 
-                prev_tag = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
-                prev_tag = prev_tag[0] if prev_tag else None
-                if prev_tag:
-                    doc.tags.remove(prev_tag)
+            prev_state = doc.get_state(new_state.type_id)
 
-                e = log_state_changed(request, doc, login, doc.friendly_state(), prev_state)
-                change_description = "Last call has been made for %s and state has been changed to %s" % (doc.name, doc.get_state("draft-iesg").name)
+            doc.set_state(new_state)
+            doc.tags.remove(*prev_tags)
+
+            e = add_state_change_event(doc, login, prev_state, new_state, prev_tags=prev_tags, new_tags=new_tags)
 
-            elif doc.type.slug == 'statchg':
-                doc.set_state(State.objects.get(used=True, type="statchg", slug='in-lc'))
-                e = log_state_changed(request, doc, login, doc.friendly_state(), prev_state)
-                change_description = "Last call has been made for %s and state has been changed to %s" % (doc.name, doc.friendly_state())
-                    
             doc.time = e.time
             doc.save()
 
+            change_description = "Last call has been made for %s and state has been changed to %s" % (doc.name, new_state.name)
+
             email_state_changed(request, doc, change_description)
             email_ad(request, doc, doc.ad, login, change_description)