Fix a missing HttpResponseForbidden in review statistics, make the

review test code use a separate reviewer and reviewsecretary user to
avoid confounding things - also let these use Unicode in their names
to check for Unicode trouble.
 - Legacy-Id: 12175
This commit is contained in:
Ole Laursen 2016-10-19 11:58:49 +00:00
parent 95bbabf384
commit 4b987436c0
6 changed files with 95 additions and 72 deletions

View file

@ -62,7 +62,7 @@ class ReviewTests(TestCase):
"team": review_team.pk,
"deadline": deadline.isoformat(),
"requested_rev": "01",
"requested_by": Person.objects.get(user__username="plain").pk,
"requested_by": Person.objects.get(user__username="reviewsecretary").pk,
})
self.assertEqual(r.status_code, 302)
@ -112,14 +112,14 @@ class ReviewTests(TestCase):
# follow link
req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
self.client.login(username="secretary", password="secretary+password")
self.client.login(username="reviewsecretary", password="reviewsecretary+password")
r = self.client.get(req_url)
self.assertEqual(r.status_code, 200)
self.assertTrue(close_url in unicontent(r))
self.client.logout()
# get close page
login_testing_unauthorized(self, "secretary", close_url)
login_testing_unauthorized(self, "reviewsecretary", close_url)
r = self.client.get(close_url)
self.assertEqual(r.status_code, 200)
@ -134,7 +134,7 @@ class ReviewTests(TestCase):
self.assertEqual(e.type, "closed_review_request")
self.assertTrue("closed" in e.desc.lower())
self.assertEqual(len(outbox), 1)
self.assertTrue("closed" in unicode(outbox[0]).lower())
self.assertTrue("closed" in outbox[0].get_payload(decode=True).decode("utf-8").lower())
def test_possibly_advance_next_reviewer_for_team(self):
doc = make_test_data()
@ -225,21 +225,21 @@ class ReviewTests(TestCase):
def test_assign_reviewer(self):
doc = make_test_data()
# set up some reviewer-suitability factors
plain_email = Email.objects.filter(person__user__username="plain").first()
DocumentAuthor.objects.create(
author=plain_email,
document=doc,
)
doc.rev = "10"
doc.save_with_history([DocEvent.objects.create(doc=doc, type="changed_document", by=Person.objects.get(user__username="secretary"), desc="Test")])
# review to assign to
review_req = make_review_data(doc)
review_req.state = ReviewRequestStateName.objects.get(slug="requested")
review_req.reviewer = None
review_req.save()
# set up some reviewer-suitability factors
reviewer_email = Email.objects.get(person__user__username="reviewer")
DocumentAuthor.objects.create(
author=reviewer_email,
document=doc,
)
doc.rev = "10"
doc.save_with_history([DocEvent.objects.create(doc=doc, type="changed_document", by=Person.objects.get(user__username="secretary"), desc="Test")])
# previous review
ReviewRequest.objects.create(
time=datetime.datetime.now() - datetime.timedelta(days=100),
@ -250,29 +250,29 @@ class ReviewTests(TestCase):
state=ReviewRequestStateName.objects.get(slug="completed"),
reviewed_rev="01",
deadline=datetime.date.today() - datetime.timedelta(days=80),
reviewer=plain_email,
reviewer=reviewer_email,
)
reviewer_settings = ReviewerSettings.objects.get(person__email=plain_email, team=review_req.team)
reviewer_settings = ReviewerSettings.objects.get(person__email=reviewer_email, team=review_req.team)
reviewer_settings.filter_re = doc.name
reviewer_settings.skip_next = 1
reviewer_settings.save()
UnavailablePeriod.objects.create(
team=review_req.team,
person=plain_email.person,
person=reviewer_email.person,
start_date=datetime.date.today() - datetime.timedelta(days=10),
availability="unavailable",
)
ReviewWish.objects.create(person=plain_email.person, team=review_req.team, doc=doc)
ReviewWish.objects.create(person=reviewer_email.person, team=review_req.team, doc=doc)
# pick a non-existing reviewer as next to see that we can
# handle reviewers who have left
NextReviewerInTeam.objects.filter(team=review_req.team).delete()
NextReviewerInTeam.objects.create(
team=review_req.team,
next_reviewer=Person.objects.exclude(pk=plain_email.person_id).first(),
next_reviewer=Person.objects.exclude(pk=reviewer_email.person_id).first(),
)
assign_url = urlreverse('ietf.doc.views_review.assign_reviewer', kwargs={ "name": doc.name, "request_id": review_req.pk })
@ -280,25 +280,25 @@ class ReviewTests(TestCase):
# follow link
req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
self.client.login(username="secretary", password="secretary+password")
self.client.login(username="reviewsecretary", password="reviewsecretary+password")
r = self.client.get(req_url)
self.assertEqual(r.status_code, 200)
self.assertTrue(assign_url in unicontent(r))
self.client.logout()
# get assign page
login_testing_unauthorized(self, "secretary", assign_url)
login_testing_unauthorized(self, "reviewsecretary", assign_url)
r = self.client.get(assign_url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
plain_label = q("option[value=\"{}\"]".format(plain_email.address)).text().lower()
self.assertIn("reviewed document before", plain_label)
self.assertIn("wishes to review", plain_label)
self.assertIn("is author", plain_label)
self.assertIn("regexp matches", plain_label)
self.assertIn("unavailable indefinitely", plain_label)
self.assertIn("skip next 1", plain_label)
self.assertIn("#1", plain_label)
reviewer_label = q("option[value=\"{}\"]".format(reviewer_email.address)).text().lower()
self.assertIn("reviewed document before", reviewer_label)
self.assertIn("wishes to review", reviewer_label)
self.assertIn("is author", reviewer_label)
self.assertIn("regexp matches", reviewer_label)
self.assertIn("unavailable indefinitely", reviewer_label)
self.assertIn("skip next 1", reviewer_label)
self.assertIn("#1", reviewer_label)
# assign
empty_outbox()
@ -311,7 +311,7 @@ class ReviewTests(TestCase):
self.assertEqual(review_req.state_id, "requested")
self.assertEqual(review_req.reviewer, reviewer)
self.assertEqual(len(outbox), 1)
self.assertTrue("assigned" in unicode(outbox[0]))
self.assertTrue("assigned" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertEqual(NextReviewerInTeam.objects.get(team=review_req.team).next_reviewer, rotation_list[1])
# re-assign
@ -326,8 +326,8 @@ class ReviewTests(TestCase):
self.assertEqual(review_req.state_id, "requested") # check that state is reset
self.assertEqual(review_req.reviewer, reviewer)
self.assertEqual(len(outbox), 2)
self.assertTrue("cancelled your assignment" in unicode(outbox[0]))
self.assertTrue("assigned" in unicode(outbox[1]))
self.assertTrue("cancelled your assignment" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertTrue("assigned" in outbox[1].get_payload(decode=True).decode("utf-8"))
def test_accept_reviewer_assignment(self):
doc = make_test_data()
@ -361,14 +361,14 @@ class ReviewTests(TestCase):
# follow link
req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
self.client.login(username="secretary", password="secretary+password")
self.client.login(username="reviewsecretary", password="reviewsecretary+password")
r = self.client.get(req_url)
self.assertEqual(r.status_code, 200)
self.assertTrue(reject_url in unicontent(r))
self.client.logout()
# get reject page
login_testing_unauthorized(self, "secretary", reject_url)
login_testing_unauthorized(self, "reviewsecretary", reject_url)
r = self.client.get(reject_url)
self.assertEqual(r.status_code, 200)
self.assertTrue(unicode(review_req.reviewer.person) in unicontent(r))
@ -385,7 +385,7 @@ class ReviewTests(TestCase):
self.assertTrue("rejected" in e.desc)
self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="requested").count(), 1)
self.assertEqual(len(outbox), 1)
self.assertTrue("Test message" in unicode(outbox[0]))
self.assertTrue("Test message" in outbox[0].get_payload(decode=True).decode("utf-8"))
def make_test_mbox_tarball(self, review_req):
mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz")
@ -444,7 +444,7 @@ class ReviewTests(TestCase):
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)
login_testing_unauthorized(self, "reviewsecretary", url)
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
@ -534,7 +534,7 @@ class ReviewTests(TestCase):
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("This is a review" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url)
@ -574,7 +574,7 @@ class ReviewTests(TestCase):
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("This is a review" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url)
@ -628,13 +628,13 @@ class ReviewTests(TestCase):
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("reviewsecretary@example.com" in outbox[0]["To"])
self.assertTrue("partially" in outbox[0]["Subject"].lower())
self.assertTrue("new review request" in unicode(outbox[0]))
self.assertTrue("new review request" in outbox[0].get_payload(decode=True).decode("utf-8"))
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]))
self.assertTrue("This is a review" in outbox[1].get_payload(decode=True).decode("utf-8"))
first_review = review_req.review
first_reviewer = review_req.reviewer

View file

@ -114,7 +114,7 @@ class ReviewTests(TestCase):
deadline=datetime.date.today() + datetime.timedelta(days=30),
state_id="accepted",
reviewer=review_req1.reviewer,
requested_by=Person.objects.get(user__username="plain"),
requested_by=Person.objects.get(user__username="reviewer"),
)
UnavailablePeriod.objects.create(
@ -161,7 +161,7 @@ class ReviewTests(TestCase):
deadline=datetime.date.today() + datetime.timedelta(days=30),
state_id="accepted",
reviewer=review_req1.reviewer,
requested_by=Person.objects.get(user__username="plain"),
requested_by=Person.objects.get(user__username="reviewer"),
)
review_req3 = ReviewRequest.objects.create(
@ -170,7 +170,7 @@ class ReviewTests(TestCase):
type_id="early",
deadline=datetime.date.today() + datetime.timedelta(days=30),
state_id="requested",
requested_by=Person.objects.get(user__username="plain"),
requested_by=Person.objects.get(user__username="reviewer"),
)
# previous reviews
@ -290,17 +290,17 @@ class ReviewTests(TestCase):
self.assertEqual(len(outbox), 1)
self.assertTrue(group.list_email in outbox[0]["To"])
self.assertEqual(outbox[0]["subject"], "Test subject")
self.assertTrue("Test body" in unicode(outbox[0]))
self.assertTrue("Test body" in outbox[0].get_payload(decode=True).decode("utf-8"))
def test_change_reviewer_settings(self):
doc = make_test_data()
reviewer = Person.objects.get(name="Plain Man")
review_req = make_review_data(doc)
review_req.reviewer = reviewer.email_set.first()
review_req.reviewer = Email.objects.get(person__user__username="reviewer")
review_req.save()
reviewer = review_req.reviewer.person
url = urlreverse(ietf.group.views_review.change_reviewer_settings, kwargs={
"acronym": review_req.team.acronym,
"reviewer_email": review_req.reviewer_id,
@ -335,8 +335,9 @@ class ReviewTests(TestCase):
self.assertEqual(settings.remind_days_before_deadline, 6)
self.assertEqual(len(outbox), 1)
self.assertTrue("reviewer availability" in outbox[0]["subject"].lower())
self.assertTrue("frequency changed", unicode(outbox[0]).lower())
self.assertTrue("skip next", unicode(outbox[0]).lower())
msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower()
self.assertTrue("frequency changed", msg_content)
self.assertTrue("skip next", msg_content)
# add unavailable period
start_date = datetime.date.today() + datetime.timedelta(days=10)
@ -352,8 +353,9 @@ class ReviewTests(TestCase):
self.assertEqual(period.end_date, None)
self.assertEqual(period.availability, "unavailable")
self.assertEqual(len(outbox), 1)
self.assertTrue(start_date.isoformat(), unicode(outbox[0]).lower())
self.assertTrue("indefinite", unicode(outbox[0]).lower())
msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower()
self.assertTrue(start_date.isoformat(), msg_content)
self.assertTrue("indefinite", msg_content)
# end unavailable period
empty_outbox()
@ -367,8 +369,9 @@ class ReviewTests(TestCase):
period = reload_db_objects(period)
self.assertEqual(period.end_date, end_date)
self.assertEqual(len(outbox), 1)
self.assertTrue(start_date.isoformat(), unicode(outbox[0]).lower())
self.assertTrue("indefinite", unicode(outbox[0]).lower())
msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower()
self.assertTrue(start_date.isoformat(), msg_content)
self.assertTrue("indefinite", msg_content)
# delete unavailable period
empty_outbox()
@ -379,16 +382,17 @@ class ReviewTests(TestCase):
self.assertEqual(r.status_code, 302)
self.assertEqual(UnavailablePeriod.objects.filter(person=reviewer, team=review_req.team, start_date=start_date).count(), 0)
self.assertEqual(len(outbox), 1)
self.assertTrue(start_date.isoformat(), unicode(outbox[0]).lower())
self.assertTrue(end_date.isoformat(), unicode(outbox[0]).lower())
msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower()
self.assertTrue(start_date.isoformat(), msg_content)
self.assertTrue(end_date.isoformat(), msg_content)
def test_reviewer_reminders(self):
doc = make_test_data()
reviewer = Person.objects.get(name="Plain Man")
review_req = make_review_data(doc)
reviewer = Person.objects.get(user__username="reviewer")
settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
settings.remind_days_before_deadline = 6
settings.save()
@ -411,4 +415,4 @@ class ReviewTests(TestCase):
empty_outbox()
email_reviewer_reminder(review_req)
self.assertEqual(len(outbox), 1)
self.assertTrue(review_req.doc_id in unicode(outbox[0]))
self.assertTrue(review_req.doc_id in outbox[0].get_payload(decode=True).decode("utf-8"))

View file

@ -342,12 +342,12 @@ class IetfAuthTests(TestCase):
def test_review_overview(self):
doc = make_test_data()
reviewer = Person.objects.get(name="Plain Man")
review_req = make_review_data(doc)
review_req.reviewer = reviewer.email_set.first()
review_req.reviewer = Email.objects.get(person__user__username="reviewer")
review_req.save()
reviewer = review_req.reviewer.person
UnavailablePeriod.objects.create(
team=review_req.team,
person=reviewer,

View file

@ -21,11 +21,19 @@ class StatisticsTests(TestCase):
login_testing_unauthorized(self, "secretary", url)
completion_url = urlreverse(ietf.stats.views.review_stats, kwargs={ "stats_type": "completion" })
r = self.client.get(url)
self.assertEqual(r.status_code, 302)
self.assertTrue(urlreverse(ietf.stats.views.review_stats, kwargs={ "stats_type": "completion" }) in r["Location"])
self.assertTrue(completion_url in r["Location"])
self.client.logout()
self.client.login(username="plain", password="plain+password")
r = self.client.get(completion_url)
self.assertEqual(r.status_code, 403)
# check tabular
self.client.login(username="secretary", password="secretary+password")
for stats_type in ["completion", "results", "states"]:
url = urlreverse(ietf.stats.views.review_stats, kwargs={ "stats_type": stats_type })
r = self.client.get(url)

View file

@ -3,7 +3,7 @@ import datetime, itertools, json, calendar
from django.shortcuts import render
from django.contrib.auth.decorators import login_required
from django.core.urlresolvers import reverse as urlreverse
from django.http import HttpResponseRedirect
from django.http import HttpResponseRedirect, HttpResponseForbidden
import dateutil.relativedelta
@ -134,6 +134,9 @@ def review_stats(request, stats_type=None, acronym=None):
if not r.group_id in secr_access:
reviewer_only_access.add(r.group_id)
if not secr_access and not reviewer_only_access:
return HttpResponseForbidden("You do not have the necessary permissions to view this page")
teams = [t for t in teams if t.pk in secr_access or t.pk in reviewer_only_access]
for t in reviewer_only_access:

View file

@ -401,10 +401,14 @@ def make_review_data(doc):
for t in ReviewTypeName.objects.filter(slug__in=["early", "lc", "telechat"]):
TypeUsedInReviewTeam.objects.create(team=team, type=t)
p = Person.objects.get(user__username="plain")
email = p.email_set.first()
Role.objects.create(name_id="reviewer", person=p, email=email, group=team)
ReviewerSettings.objects.create(team=team, person=p, min_interval=14, skip_next=0)
u = User.objects.create(username="reviewer")
u.set_password("reviewer+password")
u.save()
reviewer = Person.objects.create(name=u"Some Réviewer", ascii="Some Reviewer", user=u)
email = Email.objects.create(address="reviewer@example.com", person=reviewer)
Role.objects.create(name_id="reviewer", person=reviewer, email=email, group=team)
ReviewerSettings.objects.create(team=team, person=reviewer, min_interval=14, skip_next=0)
review_req = ReviewRequest.objects.create(
doc=doc,
@ -412,15 +416,19 @@ def make_review_data(doc):
type_id="early",
deadline=datetime.datetime.now() + datetime.timedelta(days=20),
state_id="accepted",
requested_by=p,
requested_by=reviewer,
reviewer=email,
)
p = Person.objects.get(user__username="marschairman")
Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
p = Person.objects.get(user__username="secretary")
Role.objects.create(name_id="secr", person=p, email=p.email_set.first(), group=team)
u = User.objects.create(username="reviewsecretary")
u.set_password("reviewsecretary+password")
u.save()
reviewsecretary = Person.objects.create(name=u"Réview Secretary", ascii="Review Secretary", user=u)
reviewsecretary_email = Email.objects.create(address="reviewsecretary@example.com", person=reviewsecretary)
Role.objects.create(name_id="secr", person=reviewsecretary, email=reviewsecretary_email, group=team)
return review_req