Implement completing a review with tests. One can currently

enter/upload content or retrieve it from an IETF mailarch archive
through integrated searching support. Support for partial completion.
 - Legacy-Id: 11360

Author: OleLaursen

ietf/doc/tests_review.py

@@ -1,14 +1,19 @@
 # -*- coding: utf-8 -*-
 
-import datetime
+import datetime, os, shutil, json
+import tarfile, tempfile, mailbox
+import email.mime.multipart, email.mime.text, email.utils
+from StringIO import StringIO
 
 from django.core.urlresolvers import reverse as urlreverse
+from django.conf import settings
 
 from pyquery import PyQuery
 
 import debug                            # pyflakes:ignore
 
 from ietf.review.models import ReviewRequest, Reviewer
+import ietf.review.mailarch
 from ietf.person.models import Person
 from ietf.group.models import Group, Role
 from ietf.name.models import ReviewResultName, ReviewRequestStateName
@@ -17,7 +22,6 @@
 from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
 from ietf.utils.mail import outbox, empty_outbox
 
-
 def make_review_data(doc):
     team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team")
     team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]))
@@ -39,9 +43,28 @@ def make_review_data(doc):
     p = Person.objects.get(user__username="marschairman")
     role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
 
+    p = Person.objects.get(user__username="secretary")
+    role = Role.objects.create(name_id="secretary", person=p, email=p.email_set.first(), group=team)
+    
     return review_req
 
 class ReviewTests(TestCase):
+    def setUp(self):
+        self.review_dir = os.path.abspath("tmp-review-dir")
+        if not os.path.exists(self.review_dir):
+            os.mkdir(self.review_dir)
+
+        self.old_document_path_pattern = settings.DOCUMENT_PATH_PATTERN
+        settings.DOCUMENT_PATH_PATTERN = self.review_dir + "/{doc.type_id}/"
+
+        self.review_subdir = os.path.join(self.review_dir, "review")
+        if not os.path.exists(self.review_subdir):
+            os.mkdir(self.review_subdir)
+        
+    def tearDown(self):
+        shutil.rmtree(self.review_dir)
+        settings.DOCUMENT_PATH_PATTERN = self.old_document_path_pattern
+
     def test_request_review(self):
         doc = make_test_data()
         review_req = make_review_data(doc)
@@ -229,3 +252,266 @@ def test_reject_reviewer_assignment(self):
         self.assertEqual(len(outbox), 1)
         self.assertTrue("Test message" in unicode(outbox[0]))
 
+    def make_test_mbox_tarball(self, review_req):
+        mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz")
+        with tarfile.open(mbox_path, "w:gz") as tar:
+            with tempfile.NamedTemporaryFile(dir=self.review_dir, suffix=".mbox") as tmp:
+                mbox = mailbox.mbox(tmp.name)
+
+                # plain text
+                msg = email.mime.text.MIMEText("Hello,\n\nI have reviewed the document and did not find any problems.\n\nJohn Doe")
+                msg["From"] = "johndoe@example.com"
+                msg["To"] = review_req.team.list_email
+                msg["Subject"] = "Review of {}-01".format(review_req.doc.name)
+                msg["Message-ID"] = email.utils.make_msgid()
+                msg["Archived-At"] = "<https://www.example.com/testmessage>"
+                msg["Date"] = email.utils.formatdate()
+
+                mbox.add(msg)
+
+                # plain text + HTML
+                msg = email.mime.multipart.MIMEMultipart('alternative')
+                msg["From"] = "johndoe2@example.com"
+                msg["To"] = review_req.team.list_email
+                msg["Subject"] = "Review of {}".format(review_req.doc.name)
+                msg["Message-ID"] = email.utils.make_msgid()
+                msg["Archived-At"] = "<https://www.example.com/testmessage2>"
+
+                msg.attach(email.mime.text.MIMEText("Hi!,\r\nLooks OK!\r\n-John", "plain"))
+                msg.attach(email.mime.text.MIMEText("<html><body><p>Hi!,</p><p>Looks OK!</p><p>-John</p></body></html>", "html"))
+                mbox.add(msg)
+
+                tmp.flush()
+
+                tar.add(os.path.relpath(tmp.name))
+
+        return mbox_path
+
+    def test_search_mail_archive(self):
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+        review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
+        review_req.save()
+        review_req.team.list_email = "{}@ietf.org".format(review_req.team.acronym)
+        review_req.team.save()
+
+        # test URL construction
+        query_urls = ietf.review.mailarch.construct_query_urls(review_req)
+        self.assertTrue(review_req.doc.name in query_urls["query_data_url"])
+
+        # test parsing
+        mbox_path = self.make_test_mbox_tarball(review_req)
+
+        try:
+            # mock URL generator and point it to local file - for this
+            # to work, the module (and not the function) must be
+            # imported in the view
+            real_fn = ietf.review.mailarch.construct_query_urls
+            ietf.review.mailarch.construct_query_urls = lambda review_req, query=None: { "query_data_url": "file://" + os.path.abspath(mbox_path) }
+
+            url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "request_id": review_req.pk })
+            login_testing_unauthorized(self, "secretary", url)
+
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            messages = json.loads(r.content)["messages"]
+            self.assertEqual(len(messages), 2)
+
+            self.assertEqual(messages[0]["url"], "https://www.example.com/testmessage")
+            self.assertTrue("John Doe" in messages[0]["content"])
+            self.assertEqual(messages[0]["subject"], "Review of {}-01".format(review_req.doc.name))
+
+            self.assertEqual(messages[1]["url"], "https://www.example.com/testmessage2")
+            self.assertTrue("Looks OK" in messages[1]["content"])
+            self.assertTrue("<html>" not in messages[1]["content"])
+            self.assertEqual(messages[1]["subject"], "Review of {}".format(review_req.doc.name))
+        finally:
+            ietf.review.mailarch.construct_query_urls = real_fn
+
+    def setup_complete_review_test(self):
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+        review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
+        review_req.save()
+        review_req.team.list_email = "{}@ietf.org".format(review_req.team.acronym)
+        for r in ReviewResultName.objects.filter(slug__in=("issues", "ready")):
+            review_req.team.reviewresultname_set.add(r)
+        review_req.team.save()
+
+        url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk })
+
+        return review_req, url
+
+    def test_complete_review_upload_content(self):
+        review_req, url = self.setup_complete_review_test()
+
+        login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
+
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+
+        # faulty post
+        r = self.client.post(url, data={
+            "result": "ready",
+            "state": "completed",
+            "reviewed_rev": "abc",
+            "review_submission": "upload",
+            "review_content": "",
+            "review_url": "",
+            "review_file": "",
+        })
+        self.assertEqual(r.status_code, 200)
+        q = PyQuery(r.content)
+        self.assertTrue(q("[name=reviewed_rev]").closest(".form-group").filter(".has-error"))
+        self.assertTrue(q("[name=review_file]").closest(".form-group").filter(".has-error"))
+
+        # complete by uploading file
+        empty_outbox()
+
+        test_file = StringIO("This is a review\nwith two lines")
+        test_file.name = "unnamed"
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "upload",
+            "review_content": "",
+            "review_url": "",
+            "review_file": test_file,
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "completed")
+        self.assertEqual(review_req.result_id, "ready")
+        self.assertEqual(review_req.reviewed_rev, review_req.doc.rev)
+        self.assertTrue(review_req.team.acronym.lower() in review_req.review.name)
+        self.assertTrue(review_req.doc.rev in review_req.review.name)
+
+        with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f:
+            self.assertEqual(f.read(), "This is a review\nwith two lines")
+
+        self.assertEqual(len(outbox), 1)
+        self.assertTrue(review_req.team.list_email in outbox[0]["To"])
+        self.assertTrue("This is a review" in unicode(outbox[0]))
+
+        self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url)
+
+    def test_complete_review_enter_content(self):
+        review_req, url = self.setup_complete_review_test()
+
+        login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
+
+        # complete by uploading file
+        empty_outbox()
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "enter",
+            "review_content": "This is a review\nwith two lines",
+            "review_url": "",
+            "review_file": "",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "completed")
+
+        with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f:
+            self.assertEqual(f.read(), "This is a review\nwith two lines")
+
+        self.assertEqual(len(outbox), 1)
+        self.assertTrue(review_req.team.list_email in outbox[0]["To"])
+        self.assertTrue("This is a review" in unicode(outbox[0]))
+
+        self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url)
+
+    def test_complete_review_link_to_mailing_list(self):
+        review_req, url = self.setup_complete_review_test()
+
+        login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
+
+        # complete by uploading file
+        empty_outbox()
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "link",
+            "review_content": "This is a review\nwith two lines",
+            "review_url": "http://example.com/testreview/",
+            "review_file": "",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "completed")
+
+        with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f:
+            self.assertEqual(f.read(), "This is a review\nwith two lines")
+
+        self.assertEqual(len(outbox), 0)
+        self.assertTrue("http://example.com" in review_req.review.external_url)
+
+    def test_partially_complete_review(self):
+        review_req, url = self.setup_complete_review_test()
+
+        login_testing_unauthorized(self, review_req.reviewer.person.user.username, url)
+
+        # partially complete
+        empty_outbox()
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="part-completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "enter",
+            "review_content": "This is a review\nwith two lines",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "part-completed")
+        self.assertTrue(review_req.doc.rev in review_req.review.name)
+
+        self.assertEqual(len(outbox), 2)
+        self.assertTrue("secretary" in outbox[0]["To"])
+        self.assertTrue("partially" in outbox[0]["Subject"].lower())
+        self.assertTrue("new review request" in unicode(outbox[0]))
+
+        self.assertTrue(review_req.team.list_email in outbox[1]["To"])
+        self.assertTrue("partial review" in outbox[1]["Subject"].lower())
+        self.assertTrue("This is a review" in unicode(outbox[1]))
+
+        first_review = review_req.review
+        first_reviewer = review_req.reviewer
+
+
+        # complete
+        review_req = ReviewRequest.objects.get(state="requested", doc=review_req.doc, team=review_req.team)
+        self.assertEqual(review_req.reviewer, None)
+        review_req.reviewer = first_reviewer # same reviewer, so we can test uniquification
+        review_req.save()
+
+        url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk })
+
+        r = self.client.post(url, data={
+            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "state": ReviewRequestStateName.objects.get(slug="completed").pk,
+            "reviewed_rev": review_req.doc.rev,
+            "review_submission": "enter",
+            "review_content": "This is another review\nwith\nthree lines",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "completed")
+        self.assertTrue(review_req.doc.rev in review_req.review.name)
+        second_review = review_req.review
+        self.assertTrue(first_review.name != second_review.name)
+        self.assertTrue(second_review.name.endswith("-2")) # uniquified

ietf/doc/urls_review.py

@@ -7,5 +7,7 @@
     url(r'^(?P<request_id>[0-9]+)/withdraw/$', views_review.withdraw_request),
     url(r'^(?P<request_id>[0-9]+)/assignreviewer/$', views_review.assign_reviewer),
     url(r'^(?P<request_id>[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment),
+    url(r'^(?P<request_id>[0-9]+)/complete/$', views_review.complete_review),
+    url(r'^(?P<request_id>[0-9]+)/searchmailarchive/$', views_review.search_mail_archive),
 )
 

ietf/doc/utils.py

@@ -16,7 +16,7 @@
 from ietf.doc.models import save_document_in_history
 from ietf.name.models import DocReminderTypeName, DocRelationshipName
 from ietf.group.models import Role
-from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
+from ietf.ietfauth.utils import has_role
 from ietf.utils import draft, markup_txt
 from ietf.utils.mail import send_mail
 from ietf.mailtrigger.utils import gather_address_lists