diff --git a/ietf/bin/send-review-reminders b/ietf/bin/send-review-reminders index 36eb5a235..eb21325fa 100755 --- a/ietf/bin/send-review-reminders +++ b/ietf/bin/send-review-reminders @@ -19,17 +19,19 @@ django.setup() import datetime from ietf.review.utils import ( - review_requests_needing_reviewer_reminder, email_reviewer_reminder, - review_requests_needing_secretary_reminder, email_secretary_reminder, + review_assignments_needing_reviewer_reminder, email_reviewer_reminder, + review_assignments_needing_secretary_reminder, email_secretary_reminder, ) today = datetime.date.today() -for review_req in review_requests_needing_reviewer_reminder(today): - email_reviewer_reminder(review_req) - print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.reviewer.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) +for assignment in review_assignments_needing_reviewer_reminder(today): + email_reviewer_reminder(assignment.review_request) + for review_assignment in assignments.review_req.reviewassignment_set.all(): + print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_assignment.reviewer.address, assignment.review_req.doc_id, assignment.review_req.team.acronym, assignment.review_req.pk)) -for review_req, secretary_role in review_requests_needing_secretary_reminder(today): - email_secretary_reminder(review_req, secretary_role) - print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.secretary_role.email.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) +for assignment, secretary_role in review_assignments_needing_secretary_reminder(today): + email_secretary_reminder(assignment.review_request, secretary_role) + review_req = assignment.review_request + print("Emailed reminder to {} for review of {} in {} (req. id {})".format(secretary_role.email.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) diff --git a/ietf/dbtemplate/migrations/0003_adjust_review_templates.py b/ietf/dbtemplate/migrations/0003_adjust_review_templates.py new file mode 100644 index 000000000..5a5ffc48b --- /dev/null +++ b/ietf/dbtemplate/migrations/0003_adjust_review_templates.py @@ -0,0 +1,118 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-05 11:39 +from __future__ import unicode_literals + +from django.db import migrations + +def forward(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate', 'DBTemplate') + DBTemplate.objects.filter(id=186).update(content="""I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: {{ assignment.review_request.doc.name }}-?? +Reviewer: {{ assignment.reviewer.person.plain_name }} +Review Date: {{ today }} +IETF LC End Date: {% if assignment.review_request.doc.most_recent_ietflc %}{{ assignment.review_request.doc.most_recent_ietflc.expires|date:"Y-m-d" }}{% else %}None{% endif %} +IESG Telechat date: {{ assignment.review_request.doc.telechat_date|default:'Not scheduled for a telechat' }} + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: +""") + + DBTemplate.objects.filter(id=187).update(content="""I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: {{ assignment.review_request.doc.name }}-?? +Reviewer: {{ assignment.reviewer.person.plain_name }} +Review Date: {{ today }} +IETF LC End Date: {% if assignment.review_req.doc.most_recent_ietflc %}{{ assignment.review_request.doc.most_recent_ietflc.expires|date:"Y-m-d" }}{% else %}None{% endif %} +IESG Telechat date: {{ assignment.review_request.doc.telechat_date|default:'Not scheduled for a telechat' }} + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +""") + +def reverse(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate','DBTemplate') + DBTemplate.objects.filter(id=186).update(content="""I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: {{ review_req.doc.name }}-?? +Reviewer: {{ review_req.reviewer.person.plain_name }} +Review Date: {{ today }} +IETF LC End Date: {% if review_req.doc.most_recent_ietflc %}{{ review_req.doc.most_recent_ietflc.expires|date:"Y-m-d" }}{% else %}None{% endif %} +IESG Telechat date: {{ review_req.doc.telechat_date|default:'Not scheduled for a telechat' }} + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +""") + DBTemplate.objects.filter(id=187).update(content="""I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: {{ review_req.doc.name }}-?? +Reviewer: {{ review_req.reviewer.person.plain_name }} +Review Date: {{ today }} +IETF LC End Date: {% if review_req.doc.most_recent_ietflc %}{{ review_req.doc.most_recent_ietflc.expires|date:"Y-m-d" }}{% else %}None{% endif %} +IESG Telechat date: {{ review_req.doc.telechat_date|default:'Not scheduled for a telechat' }} + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +""") + +class Migration(migrations.Migration): + + dependencies = [ + ('dbtemplate', '0002_auto_20180220_1052'), + ('doc','0011_reviewassignmentdocevent'), + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/dbtemplate/migrations/0004_adjust_assignment_email_summary_templates.py b/ietf/dbtemplate/migrations/0004_adjust_assignment_email_summary_templates.py new file mode 100644 index 000000000..0c4463a74 --- /dev/null +++ b/ietf/dbtemplate/migrations/0004_adjust_assignment_email_summary_templates.py @@ -0,0 +1,289 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-13 13:41 +from __future__ import unicode_literals + +from django.db import migrations + +def forward(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate','DBTemplate') + + DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc_id }}-{% if r.review_request..requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request..doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} +""") + + DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments + +Hi all, + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{r.review_request.type.name|ljust:"10"}}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc_id }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% if r.earlier_review_mark %} {{ r.earlier_review_mark }}{% endif %}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %} +The LC and Telechat review templates are included below: +------------------------------------------------------- + +-- Begin LC Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End LC Template -- + +-- Begin Telechat Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End Telechat Template -- +{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments + +Review instructions and related resources are at: +http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }}{{ r.earlier_review|yesno:'R, , ' }}{% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc_id }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +Review instructions and related resources are at: + + +The following reviewers have assignments:{% for r in review_assignments %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.review_request.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.review_request.doc_id }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + +def reverse(apps, schema_editor): + DBTemplate = apps.get_model('dbtemplate','DBTemplate') + + DBTemplate.objects.filter(pk=182).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +The following reviewers have assignments:{% for r in review_requests %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} +""") + + DBTemplate.objects.filter(pk=183).update(content="""{% autoescape off %}Subject: Review Assignments + +Hi all, + +The following reviewers have assignments:{% for r in review_requests %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer Type LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.deadline|date:"Y-m-d" }}{% else %}{{r.type.name|ljust:"10"}}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.earlier_review_mark %} {{ r.earlier_review_mark }}{% endif %}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %} +The LC and Telechat review templates are included below: +------------------------------------------------------- + +-- Begin LC Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please treat these comments just +like any other last call comments. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End LC Template -- + +-- Begin Telechat Template -- +I am the assigned Gen-ART reviewer for this draft. The General Area +Review Team (Gen-ART) reviews all IETF documents being processed +by the IESG for the IETF Chair. Please wait for direction from your +document shepherd or AD before posting a new version of the draft. + +For more information, please see the FAQ at + +. + +Document: +Reviewer: +Review Date: +IETF LC End Date: +IESG Telechat date: (if known) + +Summary: + +Major issues: + +Minor issues: + +Nits/editorial comments: + +-- End Telechat Template -- +{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=184).update(content="""{% autoescape off %}Subject: Assignments + +Review instructions and related resources are at: +http://tools.ietf.org/area/sec/trac/wiki/SecDirReview{% for r in review_requests %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }}{{ r.earlier_review|yesno:'R, , ' }}{% if r.section == 'Early review requests:' %}{{ r.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% endfor %} + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + DBTemplate.objects.filter(pk=185).update(content="""{% autoescape off %}Subject: Open review assignments in {{group.acronym}} + +Review instructions and related resources are at: + + +The following reviewers have assignments:{% for r in review_requests %}{% ifchanged r.section %} + +{{r.section}} + +{% if r.section == 'Early review requests:' %}Reviewer Due Draft{% else %}Reviewer LC end Draft{% endif %}{% endifchanged %} +{{ r.reviewer.person.plain_name|ljust:"22" }} {% if r.section == 'Early review requests:' %}{{ r.deadline|date:"Y-m-d" }}{% else %}{{ r.lastcall_ends|default:"None " }}{% endif %} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %} {{ r.earlier_review_mark }}{% endfor %} + +* Other revision previously reviewed +** This revision already reviewed + +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} + +""") + + +class Migration(migrations.Migration): + + dependencies = [ + ('dbtemplate', '0003_adjust_review_templates'), + ] + + operations = [ + migrations.RunPython(forward,reverse), + ] diff --git a/ietf/doc/admin.py b/ietf/doc/admin.py index 55285b49d..d61ccb191 100644 --- a/ietf/doc/admin.py +++ b/ietf/doc/admin.py @@ -5,7 +5,8 @@ from models import (StateType, State, RelatedDocument, DocumentAuthor, Document, DocHistoryAuthor, DocHistory, DocAlias, DocReminder, DocEvent, NewRevisionDocEvent, StateDocEvent, ConsensusDocEvent, BallotType, BallotDocEvent, WriteupDocEvent, LastCallDocEvent, TelechatDocEvent, BallotPositionDocEvent, ReviewRequestDocEvent, InitialReviewDocEvent, - AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL) + AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL, + ReviewAssignmentDocEvent ) class StateTypeAdmin(admin.ModelAdmin): @@ -143,6 +144,7 @@ admin.site.register(WriteupDocEvent, DocEventAdmin) admin.site.register(LastCallDocEvent, DocEventAdmin) admin.site.register(TelechatDocEvent, DocEventAdmin) admin.site.register(ReviewRequestDocEvent, DocEventAdmin) +admin.site.register(ReviewAssignmentDocEvent, DocEventAdmin) admin.site.register(InitialReviewDocEvent, DocEventAdmin) admin.site.register(AddedMessageEvent, DocEventAdmin) admin.site.register(SubmissionDocEvent, DocEventAdmin) diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 10d866f7b..f5f669181 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -209,6 +209,12 @@ class ConflictReviewFactory(BaseDocumentFactory): else: obj.set_state(State.objects.get(type_id='conflrev',slug='iesgeval')) +# This is very skeletal. It is enough for the tests that use it now, but when it's needed, it will need to be improved with, at least, a group generator that backs the object with a review team. +class ReviewFactory(BaseDocumentFactory): + type_id = 'review' + name = factory.LazyAttribute(lambda o: 'review-doesnotexist-00-%s-%s'%(o.group.acronym,datetime.date.today().isoformat())) + group = factory.SubFactory('ietf.group.factories.GroupFactory',type_id='review') + class DocAliasFactory(factory.DjangoModelFactory): class Meta: model = DocAlias diff --git a/ietf/doc/migrations/0011_reviewassignmentdocevent.py b/ietf/doc/migrations/0011_reviewassignmentdocevent.py new file mode 100644 index 000000000..63df9a962 --- /dev/null +++ b/ietf/doc/migrations/0011_reviewassignmentdocevent.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2019-01-11 11:22 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('review', '0009_refactor_review_request'), + ('name', '0005_reviewassignmentstatename'), + ('doc', '0010_auto_20190225_1302'), + ] + + operations = [ + migrations.CreateModel( + name='ReviewAssignmentDocEvent', + fields=[ + ('docevent_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='doc.DocEvent')), + ('review_assignment', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='review.ReviewAssignment')), + ('state', ietf.utils.models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='name.ReviewAssignmentStateName')), + ], + bases=('doc.docevent',), + ), + ] diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 47d0fa81f..484122922 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -20,7 +20,7 @@ import debug # pyflakes:ignore from ietf.group.models import Group from ietf.name.models import ( DocTypeName, DocTagName, StreamName, IntendedStdLevelName, StdLevelName, - DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName, FormalLanguageName, + DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName, ReviewAssignmentStateName, FormalLanguageName, DocUrlTagName) from ietf.person.models import Email, Person from ietf.person.utils import get_active_ads @@ -1149,6 +1149,10 @@ class ReviewRequestDocEvent(DocEvent): review_request = ForeignKey('review.ReviewRequest') state = ForeignKey(ReviewRequestStateName, blank=True, null=True) +class ReviewAssignmentDocEvent(DocEvent): + review_assignment = ForeignKey('review.ReviewAssignment') + state = ForeignKey(ReviewAssignmentStateName, blank=True, null=True) + # charter events class InitialReviewDocEvent(DocEvent): expires = models.DateTimeField(blank=True, null=True) diff --git a/ietf/doc/resources.py b/ietf/doc/resources.py index 62f928ced..64fd71a15 100644 --- a/ietf/doc/resources.py +++ b/ietf/doc/resources.py @@ -12,7 +12,7 @@ from ietf.doc.models import (BallotType, DeletedEvent, StateType, State, Documen TelechatDocEvent, DocReminder, LastCallDocEvent, NewRevisionDocEvent, WriteupDocEvent, InitialReviewDocEvent, DocHistoryAuthor, BallotDocEvent, RelatedDocument, RelatedDocHistory, BallotPositionDocEvent, AddedMessageEvent, SubmissionDocEvent, - ReviewRequestDocEvent, EditedAuthorsDocEvent, DocumentURL) + ReviewRequestDocEvent, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -652,3 +652,32 @@ class DocumentURLResource(ModelResource): api.doc.register(DocumentURLResource()) + + +from ietf.person.resources import PersonResource +from ietf.review.resources import ReviewAssignmentResource +from ietf.name.resources import ReviewAssignmentStateNameResource +class ReviewAssignmentDocEventResource(ModelResource): + by = ToOneField(PersonResource, 'by') + doc = ToOneField(DocumentResource, 'doc') + docevent_ptr = ToOneField(DocEventResource, 'docevent_ptr') + review_assignment = ToOneField(ReviewAssignmentResource, 'review_assignment') + state = ToOneField(ReviewAssignmentStateNameResource, 'state', null=True) + class Meta: + queryset = ReviewAssignmentDocEvent.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewassignmentdocevent' + filtering = { + "id": ALL, + "time": ALL, + "type": ALL, + "rev": ALL, + "desc": ALL, + "by": ALL_WITH_RELATIONS, + "doc": ALL_WITH_RELATIONS, + "docevent_ptr": ALL_WITH_RELATIONS, + "review_assignment": ALL_WITH_RELATIONS, + "state": ALL_WITH_RELATIONS, + } +api.doc.register(ReviewAssignmentDocEventResource()) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 24b229243..34de0d551 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -17,14 +17,14 @@ from pyquery import PyQuery import debug # pyflakes:ignore import ietf.review.mailarch -from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory -from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewRequestDocEvent +from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, ReviewFactory +from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewAssignmentDocEvent from ietf.group.factories import RoleFactory, ReviewTeamFactory from ietf.group.models import Group from ietf.message.models import Message -from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName +from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName from ietf.person.models import Email, Person -from ietf.review.factories import ReviewRequestFactory +from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewWish, UnavailablePeriod, NextReviewerInTeam) from ietf.review.utils import reviewer_rotation_list, possibly_advance_next_reviewer_for_team @@ -60,7 +60,8 @@ class ReviewTests(TestCase): RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') RoleFactory(group=review_team3,person__user__username='reviewsecretary3',person__user__email='reviewsecretary3@example.com',name_id='secr') - ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request = req, reviewer = rev_role.person.email_set.first(), state_id='accepted') url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) login_testing_unauthorized(self, "ad", url) @@ -135,7 +136,8 @@ class ReviewTests(TestCase): doc = WgDraftFactory(group__acronym='mars',rev='01') review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request=review_req, reviewer=rev_role.person.email_set.first(), state_id='accepted') # move the review request to a doubly-replaced document to # check we can fish it out @@ -150,13 +152,15 @@ class ReviewTests(TestCase): r = self.client.get(url) self.assertEqual(r.status_code, 200) content = unicontent(r) + #debug.show('unicontent(r)') self.assertTrue("{} Review".format(review_req.type.name) in content) def test_review_request(self): doc = WgDraftFactory(group__acronym='mars',rev='01') review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request = review_req, reviewer = rev_role.person.email_set.first(), state_id='accepted') url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -178,8 +182,8 @@ class ReviewTests(TestCase): review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) - + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request=review_req, state_id='accepted', reviewer=rev_role.person.email_set.first()) close_url = urlreverse('ietf.doc.views_review.close_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -287,7 +291,8 @@ class ReviewTests(TestCase): settings, _ = ReviewerSettings.objects.get_or_create(team=team, person=reviewers[2]) settings.min_interval = 30 settings.save() - ReviewRequest.objects.create(team=team, doc=doc, type_id="early", state_id="accepted", deadline=today, requested_by=reviewers[0], reviewer=reviewers[2].email_set.first()) + req = ReviewRequest.objects.create(team=team, doc=doc, type_id="early", state_id="assigned", deadline=today, requested_by=reviewers[0]) + ReviewAssignmentFactory(review_request=req, state_id="accepted", reviewer = reviewers[2].email_set.first(),assigned_on = req.time) possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[3].pk) self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4]) self.assertEqual(get_skip_next(reviewers[0]), 0) @@ -314,17 +319,25 @@ class ReviewTests(TestCase): doc.save_with_history([DocEvent.objects.create(doc=doc, rev=doc.rev, type="changed_document", by=Person.objects.get(user__username="secretary"), desc="Test")]) # previous review - ReviewRequest.objects.create( + req = ReviewRequestFactory( time=datetime.datetime.now() - datetime.timedelta(days=100), requested_by=Person.objects.get(name="(System)"), doc=doc, - type=ReviewTypeName.objects.get(slug="early"), + type_id='early', team=review_req.team, - state=ReviewRequestStateName.objects.get(slug="completed"), - result_id='serious-issues', - reviewed_rev="01", + state_id='assigned', + requested_rev="01", deadline=datetime.date.today() - datetime.timedelta(days=80), + ) + ReviewAssignmentFactory( + review_request = req, + state_id='completed', + result_id='serious-issues', reviewer=reviewer_email, + reviewed_rev="01", + review = ReviewFactory(), + assigned_on=req.time, + completed_on=req.time + datetime.timedelta(days=10), ) reviewer_settings = ReviewerSettings.objects.get(person__email=reviewer_email, team=review_req.team) @@ -387,37 +400,25 @@ class ReviewTests(TestCase): self.assertEqual(r.status_code, 302) review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "requested") - self.assertEqual(review_req.reviewer, reviewer) + self.assertEqual(review_req.state_id, "assigned") + self.assertEqual(review_req.reviewassignment_set.count(),1) + assignment = review_req.reviewassignment_set.first() + self.assertEqual(assignment.reviewer, reviewer) + self.assertEqual(assignment.state_id, "assigned") self.assertEqual(len(outbox), 1) 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 - empty_outbox() - review_req.state = ReviewRequestStateName.objects.get(slug="accepted") - review_req.save() - reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team, person=rotation_list[1]).first() - r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk, "add_skip": 1 }) - self.assertEqual(r.status_code, 302) - - review_req = reload_db_objects(review_req) - 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 outbox[0].get_payload(decode=True).decode("utf-8")) - self.assertTrue("assigned" in outbox[1].get_payload(decode=True).decode("utf-8")) - self.assertEqual(ReviewerSettings.objects.get(person=reviewer.person).skip_next, 1) - def test_accept_reviewer_assignment(self): doc = WgDraftFactory(group__acronym='mars',rev='01') review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='requested',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned', reviewer=rev_role.person.email_set.first()) url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) - username = review_req.reviewer.person.user.username + username = assignment.reviewer.person.user.username self.client.login(username=username, password=username + "+password") r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -428,21 +429,22 @@ class ReviewTests(TestCase): r = self.client.post(url, { "action": "accept" }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "accepted") + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "accepted") def test_reject_reviewer_assignment(self): doc = WgDraftFactory(group__acronym='mars',rev='01') review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + assignment = ReviewAssignmentFactory(review_request = review_req, reviewer=rev_role.person.email_set.first(), state_id='accepted') - reject_url = urlreverse('ietf.doc.views_review.reject_reviewer_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk }) + reject_url = urlreverse('ietf.doc.views_review.reject_reviewer_assignment', kwargs={ "name": doc.name, "assignment_id": assignment.pk }) # follow link - req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": assignment.review_request.pk }) self.client.login(username="reviewsecretary", password="reviewsecretary+password") r = self.client.get(req_url) self.assertEqual(r.status_code, 200) @@ -453,19 +455,18 @@ class ReviewTests(TestCase): 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)) + self.assertTrue(unicode(assignment.reviewer.person) in unicontent(r)) # reject empty_outbox() r = self.client.post(reject_url, { "action": "reject", "message_to_secretary": "Test message" }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "rejected") + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "rejected") e = doc.latest_event() self.assertEqual(e.type, "closed_review_request") 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 outbox[0].get_payload(decode=True).decode("utf-8")) @@ -509,7 +510,8 @@ class ReviewTests(TestCase): review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + assignment = ReviewAssignmentFactory(review_request=review_req, reviewer=rev_role.person.email_set.first(), state_id='accepted') # test URL construction query_urls = ietf.review.mailarch.construct_query_urls(review_req) @@ -525,7 +527,7 @@ class ReviewTests(TestCase): 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 }) + url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "assignment_id": assignment.pk }) login_testing_unauthorized(self, "reviewsecretary", url) r = self.client.get(url) @@ -555,7 +557,7 @@ class ReviewTests(TestCase): f.write('Content-Type: text/html\n\n
No results found
') ietf.review.mailarch.construct_query_urls = lambda review_req, query=None: { "query_data_url": "file://" + os.path.abspath(no_result_path) } - url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "request_id": review_req.pk }) + url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "assignment_id": assignment.pk }) r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -572,18 +574,19 @@ class ReviewTests(TestCase): review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + assignment = ReviewAssignmentFactory(review_request=review_req, state_id='accepted', reviewer=rev_role.person.email_set.first()) for r in ReviewResultName.objects.filter(slug__in=("issues", "ready")): review_req.team.reviewteamsettings.review_results.add(r) - url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk }) + url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "assignment_id": review_req.pk }) - return review_req, url + return assignment, url def test_complete_review_upload_content(self): - review_req, url = self.setup_complete_review_test() + assignment, url = self.setup_complete_review_test() - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) # get r = self.client.get(url) @@ -611,9 +614,9 @@ class ReviewTests(TestCase): test_file.name = "unnamed" r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "upload", "review_content": "", "review_url": "", @@ -621,25 +624,25 @@ class ReviewTests(TestCase): }) 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) + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "completed") + self.assertEqual(assignment.result_id, "ready") + self.assertEqual(assignment.reviewed_rev, assignment.review_request.doc.rev) + self.assertTrue(assignment.review_request.team.acronym.lower() in assignment.review.name) + self.assertTrue(assignment.review_request.doc.rev in assignment.review.name) - with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: self.assertEqual(f.read(), "This is a review\nwith two lines") self.assertEqual(len(outbox), 1) - self.assertIn(review_req.team.list_email, outbox[0]["To"]) - self.assertIn(review_req.reviewer.person.plain_name(), parseaddr(outbox[0]["From"])[0]) - self.assertIn("This is a review", outbox[0].get_payload(decode=True).decode("utf-8")) + self.assertTrue(assignment.review_request.team.list_email in outbox[0]["To"]) + self.assertTrue(assignment.reviewer.role_set.filter(group=assignment.review_request.team,name='reviewer').first().email.address in outbox[0]["From"]) + 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) + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in assignment.review.external_url) # Check that the review has the reviewer as author - self.assertEqual(review_req.reviewer, review_req.review.documentauthor_set.first().email) + self.assertEqual(assignment.reviewer, assignment.review.documentauthor_set.first().email) # Check that we have a copy of the outgoing message msgid = outbox[0]["Message-ID"] @@ -650,25 +653,25 @@ class ReviewTests(TestCase): self.assertEqual(outbox[0].get_payload(decode=True).decode(str(outbox[0].get_charset())), message.body) # check the review document page - url = urlreverse('ietf.doc.views_doc.document_main', kwargs={ "name": review_req.review.name }) + url = urlreverse('ietf.doc.views_doc.document_main', kwargs={ "name": assignment.review.name }) r = self.client.get(url) self.assertEqual(r.status_code, 200) content = unicontent(r) - self.assertTrue("{} Review".format(review_req.type.name) in content) + self.assertTrue("{} Review".format(assignment.review_request.type.name) in content) self.assertTrue("This is a review" in content) def test_complete_review_enter_content(self): - review_req, url = self.setup_complete_review_test() + assignment, url = self.setup_complete_review_test() - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -676,32 +679,33 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "completed") + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "completed") + self.assertNotEqual(assignment.completed_on, None) - with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, assignment.review.name + ".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(assignment.review_request.team.list_email in outbox[0]["To"]) 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) + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in assignment.review.external_url) def test_complete_notify_ad_because_team_settings(self): - review_req, url = self.setup_complete_review_test() - review_req.team.reviewteamsettings.notify_ad_when.add(ReviewResultName.objects.get(slug='issues')) + assignment, url = self.setup_complete_review_test() + assignment.review_request.team.reviewteamsettings.notify_ad_when.add(ReviewResultName.objects.get(slug='issues')) # TODO - it's a little surprising that the factories so far didn't give this doc an ad - review_req.doc.ad = PersonFactory() - review_req.doc.save_with_history([DocEvent.objects.create(doc=review_req.doc, rev=review_req.doc.rev, by=review_req.reviewer.person, type='changed_document',desc='added an AD')]) - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + assignment.review_request.doc.ad = PersonFactory() + assignment.review_request.doc.save_with_history([DocEvent.objects.create(doc=assignment.review_request.doc, rev=assignment.review_request.doc.rev, by=assignment.reviewer.person, type='changed_document',desc='added an AD')]) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="issues").pk, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="issues").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -714,17 +718,17 @@ class ReviewTests(TestCase): self.assertIn('settings indicated', outbox[-1].get_payload(decode=True).decode("utf-8")) def test_complete_notify_ad_because_checkbox(self): - review_req, url = self.setup_complete_review_test() - review_req.doc.ad = PersonFactory() - review_req.doc.save_with_history([DocEvent.objects.create(doc=review_req.doc, rev=review_req.doc.rev, by=review_req.reviewer.person, type='changed_document',desc='added an AD')]) - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + assignment, url = self.setup_complete_review_test() + assignment.review_request.doc.ad = PersonFactory() + assignment.review_request.doc.save_with_history([DocEvent.objects.create(doc=assignment.review_request.doc, rev=assignment.review_request.doc.rev, by=assignment.reviewer.person, type='changed_document',desc='added an AD')]) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="issues").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="issues").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -746,16 +750,16 @@ class ReviewTests(TestCase): mock.return_value = response # Run the test - review_req, url = self.setup_complete_review_test() + assignment, url = self.setup_complete_review_test() - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "link", "review_content": response.content, "review_url": "http://example.com/testreview/", @@ -763,27 +767,27 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "completed") + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "completed") - with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, assignment.review.name + ".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) + self.assertTrue("http://example.com" in assignment.review.external_url) def test_partially_complete_review(self): - review_req, url = self.setup_complete_review_test() + assignment, url = self.setup_complete_review_test() - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) # partially complete empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="part-completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a review with a somewhat long line spanning over 80 characters to test word wrapping\nand another line", }) @@ -791,16 +795,15 @@ class ReviewTests(TestCase): - 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) + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "part-completed") + self.assertTrue(assignment.review_request.doc.rev in assignment.review.name) self.assertEqual(len(outbox), 2) self.assertTrue("reviewsecretary@example.com" in outbox[0]["To"]) self.assertTrue("partially" in outbox[0]["Subject"].lower()) - 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(assignment.review_request.team.list_email in outbox[1]["To"]) self.assertTrue("partial review" in outbox[1]["Subject"].lower()) body = outbox[1].get_payload(decode=True).decode("utf-8") self.assertTrue("This is a review" in body) @@ -810,30 +813,26 @@ class ReviewTests(TestCase): self.assertTrue(not any( len(line) > 100 for line in body.splitlines() )) self.assertTrue(any( len(line) > 80 for line in body.splitlines() )) - first_review = review_req.review - first_reviewer = review_req.reviewer + first_review = assignment.review # 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() + assignment = assignment.review_request.reviewassignment_set.create(state_id="assigned", reviewer=assignment.reviewer) - url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk }) + url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": assignment.review_request.doc.name, "assignment_id": assignment.pk }) r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is another review with a really, really, really, really, really, really, really, really, really, really long line.", }) 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 + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "completed") + self.assertTrue(assignment.review_request.doc.rev in assignment.review.name) + second_review = assignment.review self.assertTrue(first_review.name != second_review.name) self.assertTrue(second_review.name.endswith("-2")) # uniquified @@ -845,18 +844,18 @@ class ReviewTests(TestCase): def test_revise_review_enter_content(self): - review_req, url = self.setup_complete_review_test() - review_req.state = ReviewRequestStateName.objects.get(slug="no-response") - review_req.save() + assignment, url = self.setup_complete_review_test() + assignment.state = ReviewAssignmentStateName.objects.get(slug="no-response") + assignment.save() - login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -866,12 +865,12 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "completed") - event = ReviewRequestDocEvent.objects.get(type="closed_review_request", review_request=review_req) + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, "completed") + event = ReviewAssignmentDocEvent.objects.get(type="closed_review_request", review_assignment=assignment) self.assertEqual(event.time, datetime.datetime(2012, 12, 24, 12, 13, 14)) - with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: self.assertEqual(f.read(), "This is a review\nwith two lines") self.assertEqual(len(outbox), 0) @@ -879,9 +878,9 @@ class ReviewTests(TestCase): # revise again empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="ready").pk, - "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, - "reviewed_rev": review_req.doc.rev, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="ready").pk, + "state": ReviewAssignmentStateName.objects.get(slug="part-completed").pk, + "reviewed_rev": assignment.review_request.doc.rev, "review_submission": "enter", "review_content": "This is a revised review", "review_url": "", @@ -891,9 +890,9 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - review_req = reload_db_objects(review_req) - self.assertEqual(review_req.review.rev, "01") - event = ReviewRequestDocEvent.objects.get(type="closed_review_request", review_request=review_req) + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.review.rev, "01") + event = ReviewAssignmentDocEvent.objects.get(type="closed_review_request", review_assignment=assignment) self.assertEqual(event.time, datetime.datetime(2013, 12, 24, 11, 11, 11)) self.assertEqual(len(outbox), 0) @@ -903,7 +902,8 @@ class ReviewTests(TestCase): review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='assigned',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request = review_req, reviewer = rev_role.person.email_set.first(), state_id='accepted') url = urlreverse('ietf.doc.views_review.edit_comment', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -924,7 +924,8 @@ class ReviewTests(TestCase): review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) rev_role = RoleFactory(group=review_team,person__user__username='reviewer',person__user__email='reviewer@example.com',name_id='reviewer') RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr') - review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,reviewer=rev_role.person.email_set.first(),deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + review_req = ReviewRequestFactory(doc=doc,team=review_team,type_id='early',state_id='accepted',requested_by=rev_role.person,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) + ReviewAssignmentFactory(review_request = review_req, reviewer = rev_role.person.email_set.first(), state_id='accepted') url = urlreverse('ietf.doc.views_review.edit_deadline', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -942,3 +943,34 @@ class ReviewTests(TestCase): review_req = reload_db_objects(review_req) self.assertEqual(review_req.deadline,new_deadline) self.assertTrue('Deadline changed' in outbox[-1]['Subject']) + + def test_mark_no_response(self): + assignment = ReviewAssignmentFactory() + secr = RoleFactory(group=assignment.review_request.team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr').person + url = urlreverse('ietf.doc.views_review.mark_reviewer_assignment_no_response', kwargs={"name": assignment.review_request.doc.name, "assignment_id": assignment.pk}) + + login_testing_unauthorized(self, secr.user.username, url) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + r=self.client.post(url, data={"action":"noresponse"}) + self.assertEqual(r.status_code, 302) + + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, 'no-response') + + def test_withdraw_assignment(self): + assignment = ReviewAssignmentFactory() + secr = RoleFactory(group=assignment.review_request.team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr').person + url = urlreverse('ietf.doc.views_review.withdraw_reviewer_assignment', kwargs={"name": assignment.review_request.doc.name, "assignment_id": assignment.pk}) + + login_testing_unauthorized(self, secr.user.username, url) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + r=self.client.post(url, data={"action":"withdraw"}) + self.assertEqual(r.status_code, 302) + + assignment = reload_db_objects(assignment) + self.assertEqual(assignment.state_id, 'withdrawn') + diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index d3cba6ffa..23b217c50 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -7,9 +7,11 @@ urlpatterns = [ url(r'^(?P[0-9]+)/login/$', views_review.review_request_forced_login), url(r'^(?P[0-9]+)/close/$', views_review.close_request), url(r'^(?P[0-9]+)/assignreviewer/$', views_review.assign_reviewer), - url(r'^(?P[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment), - url(r'^(?P[0-9]+)/complete/$', views_review.complete_review), - url(r'^(?P[0-9]+)/searchmailarchive/$', views_review.search_mail_archive), + url(r'^(?P[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment), + url(r'^(?P[0-9]+)/complete/$', views_review.complete_review), + url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_reviewer_assignment), + url(r'^(?P[0-9]+)/noresponse/$', views_review.mark_reviewer_assignment_no_response), + url(r'^(?P[0-9]+)/searchmailarchive/$', views_review.search_mail_archive), url(r'^(?P[0-9]+)/editcomment/$', views_review.edit_comment), url(r'^(?P[0-9]+)/editdeadline/$', views_review.edit_deadline), ] diff --git a/ietf/doc/utils_search.py b/ietf/doc/utils_search.py index 6b61d73db..31b365e27 100644 --- a/ietf/doc/utils_search.py +++ b/ietf/doc/utils_search.py @@ -103,7 +103,7 @@ def fill_in_document_table_attributes(docs, have_telechat_date=False): if d.get_state_slug() != "rfc": d.milestones = [ m for (t, m) in sorted(((m.time, m) for m in d.groupmilestone_set.all() if m.state_id == "active")) ] - d.reviewed_by_teams = sorted(set(r.team.acronym for r in d.reviewrequest_set.filter(state__in=["requested","accepted","part-completed","completed"]).distinct().select_related('team'))) + d.reviewed_by_teams = sorted(set(r.team.acronym for r in d.reviewrequest_set.filter(state__in=["assigned","accepted","part-completed","completed"]).distinct().select_related('team'))) e = d.latest_event_cache.get('started_iesg_process', None) d.balloting_started = e.time if e else datetime.datetime.min diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index e4166edd5..e9597a506 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -64,8 +64,8 @@ from ietf.doc.mails import email_comment from ietf.mailtrigger.utils import gather_relevant_expansions from ietf.meeting.models import Session from ietf.meeting.utils import group_sessions, get_upcoming_manageable_sessions, sort_sessions -from ietf.review.models import ReviewRequest -from ietf.review.utils import can_request_review_of_doc, review_requests_to_list_for_docs +from ietf.review.models import ReviewAssignment +from ietf.review.utils import can_request_review_of_doc, review_assignments_to_list_for_docs from ietf.review.utils import no_review_from_teams_on_doc from ietf.utils import markup_txt, log from ietf.utils.text import maybe_split @@ -382,7 +382,7 @@ def document_main(request, name, rev=None): published = doc.latest_event(type="published_rfc") started_iesg_process = doc.latest_event(type="started_iesg_process") - review_requests = review_requests_to_list_for_docs([doc]).get(doc.pk, []) + review_assignments = review_assignments_to_list_for_docs([doc]).get(doc.pk, []) no_review_from_teams = no_review_from_teams_on_doc(doc, rev or doc.rev) return render(request, "doc/document_draft.html", @@ -446,7 +446,7 @@ def document_main(request, name, rev=None): search_archive=search_archive, actions=actions, presentations=presentations, - review_requests=review_requests, + review_assignments=review_assignments, no_review_from_teams=no_review_from_teams, )) @@ -598,11 +598,11 @@ def document_main(request, name, rev=None): # If we want to go back to using markup_txt.markup_unicode, call it explicitly here like this: # content = markup_txt.markup_unicode(content, split=False, width=80) - review_req = ReviewRequest.objects.filter(review=doc.name).first() + review_assignment = ReviewAssignment.objects.filter(review=doc.name).first() other_reviews = [] - if review_req: - other_reviews = [r for r in review_requests_to_list_for_docs([review_req.doc]).get(doc.pk, []) if r != review_req] + if review_assignment: + other_reviews = [r for r in review_assignments_to_list_for_docs([review_assignment.review_request.doc]).get(doc.pk, []) if r != review_assignment] return render(request, "doc/document_review.html", dict(doc=doc, @@ -611,7 +611,7 @@ def document_main(request, name, rev=None): revisions=revisions, latest_rev=latest_rev, snapshot=snapshot, - review_req=review_req, + review_req=review_assignment.review_request, other_reviews=other_reviews, )) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 9103a65c1..fb5a55039 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -20,9 +20,9 @@ from django.template.loader import render_to_string, TemplateDoesNotExist from django.urls import reverse as urlreverse from ietf.doc.models import (Document, NewRevisionDocEvent, State, DocAlias, - LastCallDocEvent, ReviewRequestDocEvent, DocumentAuthor) -from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName -from ietf.review.models import ReviewRequest + LastCallDocEvent, ReviewRequestDocEvent, ReviewAssignmentDocEvent, DocumentAuthor) +from ietf.name.models import ReviewRequestStateName, ReviewAssignmentStateName, ReviewResultName, DocTypeName +from ietf.review.models import ReviewRequest, ReviewAssignment from ietf.group.models import Group from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role from ietf.message.models import Message @@ -30,9 +30,9 @@ from ietf.message.utils import infer_message from ietf.person.fields import PersonEmailChoiceField, SearchablePersonField from ietf.review.utils import (active_review_teams, assign_review_request_to_reviewer, can_request_review_of_doc, can_manage_review_requests_for_team, - email_review_request_change, make_new_review_request_from_existing, - close_review_request_states, close_review_request, - setup_reviewer_field) + email_review_assignment_change, email_review_request_change, + close_review_request_states, + close_review_request, setup_reviewer_field) from ietf.review import mailarch from ietf.utils.fields import DatepickerDateField from ietf.utils.text import strip_prefix, xslugify @@ -184,47 +184,50 @@ def review_request(request, name, request_id): doc = get_object_or_404(Document, name=name) review_req = get_object_or_404(ReviewRequest, pk=request_id) - is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.person) can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) - can_close_request = (review_req.state_id in ["requested", "accepted"] + can_close_request = (review_req.state_id in ["requested", "assigned"] and (is_authorized_in_doc_stream(request.user, doc) or can_manage_request)) - can_assign_reviewer = (review_req.state_id in ["requested", "accepted"] + can_assign_reviewer = (review_req.state_id in ["requested", "assigned"] and can_manage_request) - can_accept_reviewer_assignment = (review_req.state_id == "requested" - and review_req.reviewer - and (is_reviewer or can_manage_request)) - - can_reject_reviewer_assignment = (review_req.state_id in ["requested", "accepted"] - and review_req.reviewer - and (is_reviewer or can_manage_request)) - - can_complete_review = (review_req.state_id in ["requested", "accepted", "overtaken", "no-response", "part-completed", "completed"] - and review_req.reviewer - and (is_reviewer or can_manage_request)) - can_edit_comment = can_request_review_of_doc(request.user, doc) + can_edit_deadline = can_edit_comment - if request.method == "POST" and request.POST.get("action") == "accept" and can_accept_reviewer_assignment: - review_req.state = ReviewRequestStateName.objects.get(slug="accepted") - review_req.save() + assignments = review_req.reviewassignment_set.all() + for assignment in assignments: + assignment.is_reviewer = user_is_person(request.user, assignment.reviewer.person) + assignment.can_accept_reviewer_assignment = (assignment.state_id == "assigned" + and (assignment.is_reviewer or can_manage_request)) + + assignment.can_reject_reviewer_assignment = (assignment.state_id in ["assigned", "accepted"] + and (assignment.is_reviewer or can_manage_request)) + + assignment.can_complete_review = (assignment.state_id in ["assigned", "accepted", "overtaken", "no-response", "part-completed", "completed"] + and (assignment.is_reviewer or can_manage_request)) + + # This implementation means if a reviewer accepts one assignment for a review_request, he accepts all assigned to him (for that request) + # This problematic - it's a bug (probably) for the same person to have more than one assignment for the same request. + # It is, however unintuitive, and acceptance should be refactored to be something that works on assignments, not requests + if request.method == "POST" and request.POST.get("action") == "accept": + for assignment in assignments: + if assignment.can_accept_reviewer_assignment: + assignment.state = ReviewAssignmentStateName.objects.get(slug="accepted") + assignment.save() return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) return render(request, 'doc/review/review_request.html', { 'doc': doc, 'review_req': review_req, 'can_close_request': can_close_request, - 'can_reject_reviewer_assignment': can_reject_reviewer_assignment, 'can_assign_reviewer': can_assign_reviewer, - 'can_accept_reviewer_assignment': can_accept_reviewer_assignment, - 'can_complete_review': can_complete_review, 'can_edit_comment': can_edit_comment, 'can_edit_deadline': can_edit_deadline, + 'assignments': assignments, }) @@ -245,7 +248,7 @@ class CloseReviewRequestForm(forms.Form): @login_required def close_request(request, name, request_id): doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "assigned"]) can_request = is_authorized_in_doc_stream(request.user, doc) can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) @@ -265,12 +268,13 @@ def close_request(request, name, request_id): return render(request, 'doc/review/close_request.html', { 'doc': doc, 'review_req': review_req, + 'assignments': review_req.reviewassignment_set.all(), 'form': form, }) class AssignReviewerForm(forms.Form): - reviewer = PersonEmailChoiceField(empty_label="(None)", required=False) + reviewer = PersonEmailChoiceField(label="Assign Additional Reviewer", empty_label="(None)", required=False) add_skip = forms.BooleanField(label='Skip next time', required=False) def __init__(self, review_req, *args, **kwargs): @@ -281,7 +285,7 @@ class AssignReviewerForm(forms.Form): @login_required def assign_reviewer(request, name, request_id): doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "assigned"]) if not can_manage_review_requests_for_team(request.user, review_req.team): return HttpResponseForbidden("You do not have permission to perform this action") @@ -300,6 +304,7 @@ def assign_reviewer(request, name, request_id): return render(request, 'doc/review/assign_reviewer.html', { 'doc': doc, 'review_req': review_req, + 'assignments': review_req.reviewassignment_set.all(), 'form': form, }) @@ -307,15 +312,15 @@ class RejectReviewerAssignmentForm(forms.Form): message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary if filled in", strip=False) @login_required -def reject_reviewer_assignment(request, name, request_id): +def reject_reviewer_assignment(request, name, assignment_id): doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) + review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"]) - if not review_req.reviewer: - return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + if not review_assignment.reviewer: + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) - is_reviewer = user_is_person(request.user, review_req.reviewer.person) - can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) + is_reviewer = user_is_person(request.user, review_assignment.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, review_assignment.review_request.team) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") @@ -323,47 +328,119 @@ def reject_reviewer_assignment(request, name, request_id): if request.method == "POST" and request.POST.get("action") == "reject": form = RejectReviewerAssignmentForm(request.POST) if form.is_valid(): - # reject the request - review_req.state = ReviewRequestStateName.objects.get(slug="rejected") - review_req.save() + # reject the assignment + review_assignment.state = ReviewAssignmentStateName.objects.get(slug="rejected") + review_assignment.save() - ReviewRequestDocEvent.objects.create( + ReviewAssignmentDocEvent.objects.create( type="closed_review_request", - doc=review_req.doc, - rev=review_req.doc.rev, + doc=review_assignment.review_request.doc, + rev=review_assignment.review_request.doc.rev, by=request.user.person, desc="Assignment of request for {} review by {} to {} was rejected".format( - review_req.type.name, - review_req.team.acronym.upper(), - review_req.reviewer.person, + review_assignment.review_request.type.name, + review_assignment.review_request.team.acronym.upper(), + review_assignment.reviewer.person, ), - review_request=review_req, - state=review_req.state, + review_assignment=review_assignment, + state=review_assignment.state, ) - # make a new unassigned review request - new_review_req = make_new_review_request_from_existing(review_req) - new_review_req.save() - msg = render_to_string("review/reviewer_assignment_rejected.txt", { "by": request.user.person, "message_to_secretary": form.cleaned_data.get("message_to_secretary") }) - email_review_request_change(request, review_req, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) + email_review_assignment_change(request, review_assignment, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) - return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) else: form = RejectReviewerAssignmentForm() return render(request, 'doc/review/reject_reviewer_assignment.html', { 'doc': doc, - 'review_req': review_req, + 'review_req': review_assignment.review_request, + 'assignments': review_assignment.review_request.reviewassignment_set.all(), 'form': form, }) +@login_required +def withdraw_reviewer_assignment(request, name, assignment_id): + get_object_or_404(Document, name=name) + review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"]) + + can_manage_request = can_manage_review_requests_for_team(request.user, review_assignment.review_request.team) + if not can_manage_request: + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST" and request.POST.get("action") == "withdraw": + review_assignment.state_id = 'withdrawn' + review_assignment.save() + + ReviewAssignmentDocEvent.objects.create( + type="closed_review_request", + doc=review_assignment.review_request.doc, + rev=review_assignment.review_request.doc.rev, + by=request.user.person, + desc="Assignment of request for {} review by {} to {} was withdrawn".format( + review_assignment.review_request.type.name, + review_assignment.review_request.team.acronym.upper(), + review_assignment.reviewer.person, + ), + review_assignment=review_assignment, + state=review_assignment.state, + ) + + msg = "Review assignment withdrawn by %s"%request.user.person + + email_review_assignment_change(request, review_assignment, "Reviewer assignment withdrawn", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) + + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) + + return render(request, 'doc/review/withdraw_reviewer_assignment.html', { + 'assignment': review_assignment, + }) + +@login_required +def mark_reviewer_assignment_no_response(request, name, assignment_id): + get_object_or_404(Document, name=name) + review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"]) + + can_manage_request = can_manage_review_requests_for_team(request.user, review_assignment.review_request.team) + if not can_manage_request: + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST" and request.POST.get("action") == "noresponse": + review_assignment.state_id = 'no-response' + review_assignment.save() + + ReviewAssignmentDocEvent.objects.create( + type="closed_review_request", + doc=review_assignment.review_request.doc, + rev=review_assignment.review_request.doc.rev, + by=request.user.person, + desc="Assignment of request for {} review by {} to {} was marked no-response".format( + review_assignment.review_request.type.name, + review_assignment.review_request.team.acronym.upper(), + review_assignment.reviewer.person, + ), + review_assignment=review_assignment, + state=review_assignment.state, + ) + + msg = "Review assignment marked 'No Response' by %s"%request.user.person + + email_review_assignment_change(request, review_assignment, "Reviewer assignment marked no-response", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) + + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) + + return render(request, 'doc/review/mark_reviewer_assignment_no_response.html', { + 'assignment': review_assignment, + }) + + class CompleteReviewForm(forms.Form): - state = forms.ModelChoiceField(queryset=ReviewRequestStateName.objects.filter(slug__in=("completed", "part-completed")).order_by("-order"), widget=forms.RadioSelect, initial="completed") + state = forms.ModelChoiceField(queryset=ReviewAssignmentStateName.objects.filter(slug__in=("completed", "part-completed")).order_by("-order"), widget=forms.RadioSelect, initial="completed") reviewed_rev = forms.CharField(label="Reviewed revision", max_length=4) result = forms.ModelChoiceField(queryset=ReviewResultName.objects.filter(used=True), widget=forms.RadioSelect, empty_label=None) ACTIONS = [ @@ -381,16 +458,16 @@ class CompleteReviewForm(forms.Form): cc = MultiEmailField(required=False, help_text="Email addresses to send to in addition to the review team list") email_ad = forms.BooleanField(label="Send extra email to the responsible AD suggesting early attention", required=False) - def __init__(self, review_req, is_reviewer, *args, **kwargs): - self.review_req = review_req + def __init__(self, assignment, is_reviewer, *args, **kwargs): + self.assignment = assignment super(CompleteReviewForm, self).__init__(*args, **kwargs) - doc = self.review_req.doc + doc = self.assignment.review_request.doc known_revisions = NewRevisionDocEvent.objects.filter(doc=doc).order_by("time", "id").values_list("rev", "time", flat=False) - revising_review = review_req.state_id not in ["requested", "accepted"] + revising_review = assignment.state_id not in ["assigned", "accepted"] if not revising_review: self.fields["state"].choices = [ @@ -402,7 +479,7 @@ class CompleteReviewForm(forms.Form): reviewed_rev_class = [] for r in known_revisions: last_version = r[0] - if r[1] < review_req.time: + if r[1] < assignment.review_request.time: kwargs["initial"]["reviewed_rev"] = r[0] reviewed_rev_class.append('reviewer-doc-past') else: @@ -426,13 +503,13 @@ class CompleteReviewForm(forms.Form): " ".join("{1}".format('', *r) for i, r in enumerate(known_revisions))) - self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings_review_results_set__group=review_req.team) + self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings_review_results_set__group=assignment.review_request.team) def format_submission_choice(label): if revising_review: label = label.replace(" (automatically posts to {mailing_list})", "") - return label.format(mailing_list=review_req.team.list_email or "[error: team has no mailing list set]") + return label.format(mailing_list=assignment.review_request.team.list_email or "[error: team has no mailing list set]") self.fields["review_submission"].choices = [ (k, format_submission_choice(label)) for k, label in self.fields["review_submission"].choices] @@ -443,7 +520,7 @@ class CompleteReviewForm(forms.Form): del self.fields["completion_time"] def clean_reviewed_rev(self): - return clean_doc_revision(self.review_req.doc, self.cleaned_data.get("reviewed_rev")) + return clean_doc_revision(self.assignment.review_request.doc, self.cleaned_data.get("reviewed_rev")) def clean_review_content(self): return self.cleaned_data["review_content"].replace("\r", "") @@ -461,7 +538,7 @@ class CompleteReviewForm(forms.Form): return url def clean(self): - if "@" in self.review_req.reviewer.person.ascii: + if "@" in self.assignment.reviewer.person.ascii: raise forms.ValidationError("Reviewer name must be filled in (the ASCII version is currently \"{}\" - since it contains an @ sign the name is probably still the original email address).".format(self.review_req.reviewer.person.ascii)) def require_field(f): @@ -477,40 +554,37 @@ class CompleteReviewForm(forms.Form): require_field("review_url") @login_required -def complete_review(request, name, request_id): +def complete_review(request, name, assignment_id): doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id) + assignment = get_object_or_404(ReviewAssignment, pk=assignment_id) - revising_review = review_req.state_id not in ["requested", "accepted"] + revising_review = assignment.state_id not in ["assigned", "accepted"] - if not review_req.reviewer: - return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) - - is_reviewer = user_is_person(request.user, review_req.reviewer.person) - can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) + is_reviewer = user_is_person(request.user, assignment.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, assignment.review_request.team) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") - (to, cc) = gather_address_lists('review_completed',review_req = review_req) + (to, cc) = gather_address_lists('review_completed', review_req = assignment.review_request) if request.method == "POST": - form = CompleteReviewForm(review_req, is_reviewer, + form = CompleteReviewForm(assignment, is_reviewer, request.POST, request.FILES) if form.is_valid(): review_submission = form.cleaned_data['review_submission'] - review = review_req.review + review = assignment.review if not review: # create review doc for i in range(1, 100): name_components = [ "review", - strip_prefix(review_req.doc.name, "draft-"), + strip_prefix(assignment.review_request.doc.name, "draft-"), form.cleaned_data["reviewed_rev"], - review_req.team.acronym, - review_req.type.slug, - xslugify(review_req.reviewer.person.ascii_parts()[3]), + assignment.review_request.team.acronym, + assignment.review_request.type.slug, + xslugify(assignment.reviewer.person.ascii_parts()[3]), datetime.date.today().isoformat(), ] if i > 1: @@ -523,10 +597,10 @@ def complete_review(request, name, request_id): break review.type = DocTypeName.objects.get(slug="review") - review.group = review_req.team + review.group = assignment.review_request.team review.rev = "00" if not review.rev else "{:02}".format(int(review.rev) + 1) - review.title = "{} Review of {}-{}".format(review_req.type.name, review_req.doc.name, form.cleaned_data["reviewed_rev"]) + review.title = "{} Review of {}-{}".format(assignment.review_request.type.name, assignment.review_request.doc.name, form.cleaned_data["reviewed_rev"]) review.time = datetime.datetime.now() if review_submission == "link": review.external_url = form.cleaned_data['review_url'] @@ -554,63 +628,55 @@ def complete_review(request, name, request_id): with open(filename, 'wb') as destination: destination.write(encoded_content) - # close review request - review_req.state = form.cleaned_data["state"] - review_req.reviewed_rev = form.cleaned_data["reviewed_rev"] - review_req.result = form.cleaned_data["result"] - review_req.review = review - review_req.save() - - need_to_email_review = review_submission != "link" and review_req.team.list_email and not revising_review - - desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format( - review_req.type.name, - review_req.team.acronym.upper(), - review_req.state.name, - review_req.result.name, - review_req.reviewer.person, - ) - if need_to_email_review: - desc += " " + "Sent review to list." - completion_datetime = datetime.datetime.now() if "completion_date" in form.cleaned_data: completion_datetime = datetime.datetime.combine(form.cleaned_data["completion_date"], form.cleaned_data.get("completion_time") or datetime.time.min) - close_event = ReviewRequestDocEvent.objects.filter(type="closed_review_request", review_request=review_req).first() - if not close_event: - close_event = ReviewRequestDocEvent(type="closed_review_request", review_request=review_req) + # complete assignment + assignment.state = form.cleaned_data["state"] + assignment.reviewed_rev = form.cleaned_data["reviewed_rev"] + assignment.result = form.cleaned_data["result"] + assignment.review = review + assignment.completed_on = completion_datetime + assignment.save() - close_event.doc = review_req.doc - close_event.rev = review_req.doc.rev + need_to_email_review = review_submission != "link" and assignment.review_request.team.list_email and not revising_review + + desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format( + assignment.review_request.type.name, + assignment.review_request.team.acronym.upper(), + assignment.state.name, + assignment.result.name, + assignment.reviewer.person, + ) + if need_to_email_review: + desc += " " + "Sent review to list." + + close_event = ReviewAssignmentDocEvent.objects.filter(type="closed_review_request", review_assignment=assignment).first() + if not close_event: + close_event = ReviewAssignmentDocEvent(type="closed_review_request", review_assignment=assignment) + + close_event.doc = assignment.review_request.doc + close_event.rev = assignment.review_request.doc.rev close_event.by = request.user.person close_event.desc = desc - close_event.state = review_req.state + close_event.state = assignment.state close_event.time = completion_datetime close_event.save() - if review_req.state_id == "part-completed" and not revising_review: - existing_open_reqs = ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state__in=("requested", "accepted")) + if assignment.state_id == "part-completed" and not revising_review: + existing_assignments = ReviewAssignment.objects.filter(review_request__doc=assignment.review_request.doc, review_request__team=assignment.review_request.team, state__in=("assigned", "accepted", "completed")) - new_review_req_url = new_review_req = None - if not existing_open_reqs: - new_review_req = make_new_review_request_from_existing(review_req) - new_review_req.save() - - new_review_req_url = urlreverse("ietf.doc.views_review.review_request", kwargs={ "name": new_review_req.doc.name, "request_id": new_review_req.pk }) - new_review_req_url = request.build_absolute_uri(new_review_req_url) - - subject = "Review of {}-{} completed partially".format(review_req.doc.name, review_req.reviewed_rev) + subject = "Review of {}-{} completed partially".format(assignment.review_request.doc.name, assignment.reviewed_rev) msg = render_to_string("review/partially_completed_review.txt", { - "new_review_req_url": new_review_req_url, - "existing_open_reqs": existing_open_reqs, + "existing_assignments": existing_assignments, "by": request.user.person, }) - email_review_request_change(request, review_req, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False, notify_requested_by=False) + email_review_assignment_change(request, assignment, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False, notify_requested_by=False) - role = request.user.person.role_set.filter(group=review_req.team,name='reviewer').first() + role = request.user.person.role_set.filter(group=assignment.review_request.team,name='reviewer').first() if role and role.email.active: author_email = role.email frm = role.formatted_email() @@ -621,10 +687,10 @@ def complete_review(request, name, request_id): if need_to_email_review: # email the review - subject = "{} {} {} of {}-{}".format(review_req.team.acronym.capitalize(),review_req.type.name.lower(),"partial review" if review_req.state_id == "part-completed" else "review", review_req.doc.name, review_req.reviewed_rev) - related_groups = [ review_req.team, ] - if review_req.doc.group: - related_groups.append(review_req.doc.group) + subject = "{} {} {} of {}-{}".format(assignment.review_request.team.acronym.capitalize(),assignment.review_request.type.name.lower(),"partial review" if assignment.state_id == "part-completed" else "review", assignment.review_request.doc.name, assignment.reviewed_rev) + related_groups = [ assignment.review_request.team, ] + if assignment.review_request.doc.group: + related_groups.append(assignment.review_request.doc.group) msg = Message.objects.create( by=request.user.person, subject=subject, @@ -632,26 +698,26 @@ def complete_review(request, name, request_id): to=", ".join(to), cc=form.cleaned_data["cc"], body = render_to_string("review/completed_review.txt", { - "review_req": review_req, + "assignment": assignment, "content": encoded_content.decode("utf-8"), }), ) msg.related_groups.add(*related_groups) - msg.related_docs.add(review_req.doc) + msg.related_docs.add(assignment.review_request.doc) msg = send_mail_message(request, msg) - list_name = mailarch.list_name_from_email(review_req.team.list_email) + list_name = mailarch.list_name_from_email(assignment.review_request.team.list_email) if list_name: review.external_url = mailarch.construct_message_url(list_name, email.utils.unquote(msg["Message-ID"])) review.save_with_history([close_event]) - if form.cleaned_data['email_ad'] or review_req.result in review_req.team.reviewteamsettings.notify_ad_when.all(): - (to, cc) = gather_address_lists('review_notify_ad',review_req = review_req).as_strings() + if form.cleaned_data['email_ad'] or assignment.result in assignment.review_request.team.reviewteamsettings.notify_ad_when.all(): + (to, cc) = gather_address_lists('review_notify_ad',review_req = assignment.review_request).as_strings() msg_txt = render_to_string("review/notify_ad.txt", { "to": to, "cc": cc, - "review_req": review_req, + "assignment": assignment, "settings": settings, "explicit_request": form.cleaned_data['email_ad'], }) @@ -660,42 +726,41 @@ def complete_review(request, name, request_id): msg.save() send_mail_message(request, msg) - return redirect("ietf.doc.views_doc.document_main", name=review_req.review.name) + return redirect("ietf.doc.views_doc.document_main", name=assignment.review.name) else: initial={ - "reviewed_rev": review_req.reviewed_rev, - "result": review_req.result_id, + "reviewed_rev": assignment.reviewed_rev, + "result": assignment.result_id, "cc": ", ".join(cc), } try: - initial['review_content'] = render_to_string('/group/%s/review/content_templates/%s.txt' % (review_req.team.acronym, review_req.type.slug), {'review_req':review_req,'today':datetime.date.today()}) + initial['review_content'] = render_to_string('/group/%s/review/content_templates/%s.txt' % (assignment.review_request.team.acronym, assignment.review_request.type.slug), {'assignment':assignment,'today':datetime.date.today()}) except TemplateDoesNotExist: pass - form = CompleteReviewForm(review_req, is_reviewer, initial=initial) + form = CompleteReviewForm(assignment, is_reviewer, initial=initial) - mail_archive_query_urls = mailarch.construct_query_urls(review_req) + mail_archive_query_urls = mailarch.construct_query_urls(assignment.review_request) return render(request, 'doc/review/complete_review.html', { 'doc': doc, - 'review_req': review_req, + 'assignment': assignment, 'form': form, 'mail_archive_query_urls': mail_archive_query_urls, 'revising_review': revising_review, }) -def search_mail_archive(request, name, request_id): - #doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id) +def search_mail_archive(request, name, assignment_id): + assignment = get_object_or_404(ReviewAssignment, pk=assignment_id) - is_reviewer = user_is_person(request.user, review_req.reviewer.person) - can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) + is_reviewer = user_is_person(request.user, assignment.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, assignment.review_request.team) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") - res = mailarch.construct_query_urls(review_req, query=request.GET.get("query")) + res = mailarch.construct_query_urls(assignment.review_request, query=request.GET.get("query")) if not res: return JsonResponse({ "error": "Couldn't do lookup in mail archive - don't know where to look"}) @@ -760,7 +825,7 @@ def edit_deadline(request, name, request_id): if form.is_valid(): if form.cleaned_data['deadline'] != old_deadline: form.save() - subject = "Deadline changed: {} {} review of {}-{}".format(review_req.team.acronym.capitalize(),review_req.type.name.lower(), review_req.doc.name, review_req.reviewed_rev) + subject = "Deadline changed: {} {} review of {}-{}".format(review_req.team.acronym.capitalize(),review_req.type.name.lower(), review_req.doc.name, review_req.requested_rev) msg = render_to_string("review/deadline_changed.txt", { "review_req": review_req, "old_deadline": old_deadline, diff --git a/ietf/group/forms.py b/ietf/group/forms.py index 1cbc56ee8..635312263 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -239,8 +239,6 @@ class ManageReviewRequestForm(forms.Form): close_initial = None if review_req.pk is None: close_initial = "no-review-version" - elif review_req.reviewer: - close_initial = "no-response" else: close_initial = "overtaken" diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 92bed8aab..021172a98 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -9,26 +9,27 @@ from ietf.utils.test_utils import login_testing_unauthorized, TestCase, uniconte from ietf.doc.models import TelechatDocEvent from ietf.group.models import Role from ietf.iesg.models import TelechatDate -from ietf.person.models import Email, Person -from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings +from ietf.person.models import Person +from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings from ietf.review.utils import ( suggested_review_requests_for_team, - review_requests_needing_reviewer_reminder, email_reviewer_reminder, - review_requests_needing_secretary_reminder, email_secretary_reminder, + review_assignments_needing_reviewer_reminder, email_reviewer_reminder, + review_assignments_needing_secretary_reminder, email_secretary_reminder, reviewer_rotation_list, ) -from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName +from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName import ietf.group.views from ietf.utils.mail import outbox, empty_outbox from ietf.dbtemplate.factories import DBTemplateFactory from ietf.person.factories import PersonFactory, EmailFactory from ietf.doc.factories import DocumentFactory from ietf.group.factories import RoleFactory, ReviewTeamFactory -from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory +from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, ReviewAssignmentFactory class ReviewTests(TestCase): def test_review_requests(self): - review_req = ReviewRequestFactory(reviewer=EmailFactory()) + review_req = ReviewRequestFactory(state_id='assigned') + assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned', reviewer=EmailFactory(), assigned_on = review_req.time) group = review_req.team for url in [urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }), @@ -36,7 +37,7 @@ class ReviewTests(TestCase): r = self.client.get(url) self.assertEqual(r.status_code, 200) self.assertIn(review_req.doc.name, unicontent(r)) - self.assertIn(unicode(review_req.reviewer.person), unicontent(r)) + self.assertIn(assignment.reviewer.person.__unicode__(), unicontent(r)) url = urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }) @@ -50,7 +51,8 @@ class ReviewTests(TestCase): self.assertTrue(review_req.doc.name in unicontent(r)) def test_suggested_review_requests(self): - review_req = ReviewRequestFactory() + review_req = ReviewRequestFactory(state_id='assigned') + assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned') doc = review_req.doc team = review_req.team @@ -89,22 +91,23 @@ class ReviewTests(TestCase): self.assertEqual(list(suggested_review_requests_for_team(team)), []) # blocked by versioned refusal - review_req.reviewed_rev = doc.rev - review_req.state = ReviewRequestStateName.objects.get(slug="no-review-document") + review_req.state = ReviewRequestStateName.objects.get(slug="no-review-version") review_req.save() self.assertEqual(list(suggested_review_requests_for_team(team)), []) # blocked by completion - review_req.state = ReviewRequestStateName.objects.get(slug="completed") + review_req.state = ReviewRequestStateName.objects.get(slug="assigned") review_req.save() + assignment.state = ReviewAssignmentStateName.objects.get(slug="completed") + assignment.reviewed_rev = review_req.doc.rev + assignment.save() self.assertEqual(list(suggested_review_requests_for_team(team)), []) # ... but not to previous version - review_req.reviewed_rev = prev_rev - review_req.state = ReviewRequestStateName.objects.get(slug="completed") - review_req.save() + assignment.reviewed_rev = prev_rev + assignment.save() self.assertEqual(len(suggested_review_requests_for_team(team)), 1) @@ -112,17 +115,19 @@ class ReviewTests(TestCase): team = ReviewTeamFactory() reviewer = RoleFactory(name_id='reviewer',group=team,person__user__username='reviewer').person ReviewerSettingsFactory(person=reviewer,team=team) - review_req1 = ReviewRequestFactory(state_id='completed',team=team,reviewer=reviewer.email()) + review_req1 = ReviewRequestFactory(state_id='completed',team=team) + ReviewAssignmentFactory(review_request = review_req1, reviewer=reviewer.email()) PersonFactory(user__username='plain') - ReviewRequest.objects.create( - doc=review_req1.doc, - team=review_req1.team, - type_id="early", - deadline=datetime.date.today() + datetime.timedelta(days=30), - state_id="accepted", - reviewer=review_req1.reviewer, - requested_by=Person.objects.get(user__username="reviewer"), + ReviewAssignmentFactory( + review_request__doc=review_req1.doc, + review_request__team=review_req1.team, + review_request__type_id="early", + review_request__deadline=datetime.date.today() + datetime.timedelta(days=30), + review_request__state_id="assigned", + review_request__requested_by=Person.objects.get(user__username="reviewer"), + state_id = "accepted", + reviewer=reviewer.email_set.first(), ) UnavailablePeriod.objects.create( @@ -169,169 +174,70 @@ class ReviewTests(TestCase): def test_manage_review_requests(self): group = ReviewTeamFactory() - reviewer = RoleFactory(name_id='reviewer',group=group,person__user__username='reviewer').person + RoleFactory(name_id='reviewer',group=group,person__user__username='reviewer').person marsperson = RoleFactory(name_id='reviewer',group=group,person=PersonFactory(name=u"Mars Anders Chairman",user__username='marschairman')).person - review_req1 = ReviewRequestFactory(doc__pages=2,doc__shepherd=marsperson.email(),reviewer=reviewer.email(),team=group) + review_req1 = ReviewRequestFactory(doc__pages=2,doc__shepherd=marsperson.email(),team=group) + review_req2 = ReviewRequestFactory(team=group) + review_req3 = ReviewRequestFactory(team=group) RoleFactory(name_id='chair',group=review_req1.doc.group,person=marsperson) - doc = review_req1.doc - url = urlreverse(ietf.group.views.manage_review_requests, kwargs={ 'acronym': group.acronym, "assignment_status": "assigned" }) - - login_testing_unauthorized(self, "secretary", url) - - assigned_url = urlreverse(ietf.group.views.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "assigned" }) unassigned_url = urlreverse(ietf.group.views.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "unassigned" }) - - review_req2 = ReviewRequest.objects.create( - doc=review_req1.doc, - team=review_req1.team, - type_id="early", - deadline=datetime.date.today() + datetime.timedelta(days=30), - state_id="accepted", - reviewer=review_req1.reviewer, - requested_by=Person.objects.get(user__username="reviewer"), - ) - - review_req3 = ReviewRequest.objects.create( - doc=review_req1.doc, - team=review_req1.team, - type_id="early", - deadline=datetime.date.today() + datetime.timedelta(days=30), - state_id="requested", - requested_by=Person.objects.get(user__username="reviewer"), - ) - - # previous reviews - ReviewRequest.objects.create( - time=datetime.datetime.now() - datetime.timedelta(days=100), - requested_by=Person.objects.get(name="(System)"), - doc=doc, - type=ReviewTypeName.objects.get(slug="early"), - team=review_req1.team, - state=ReviewRequestStateName.objects.get(slug="completed"), - result=ReviewResultName.objects.get(slug="ready-nits"), - reviewed_rev="01", - deadline=datetime.date.today() - datetime.timedelta(days=80), - reviewer=review_req1.reviewer, - ) - - ReviewRequest.objects.create( - time=datetime.datetime.now() - datetime.timedelta(days=100), - requested_by=Person.objects.get(name="(System)"), - doc=doc, - type=ReviewTypeName.objects.get(slug="early"), - team=review_req1.team, - state=ReviewRequestStateName.objects.get(slug="completed"), - result=ReviewResultName.objects.get(slug="ready"), - reviewed_rev="01", - deadline=datetime.date.today() - datetime.timedelta(days=80), - reviewer=review_req1.reviewer, - ) + login_testing_unauthorized(self, "secretary", unassigned_url) # Need one more person in review team one so we can test incrementing skip_count without immediately decrementing it another_reviewer = PersonFactory.create(name = u"Extra TestReviewer") # needs to be lexically greater than the exsting one another_reviewer.role_set.create(name_id='reviewer', email=another_reviewer.email(), group=review_req1.team) - + ReviewerSettingsFactory(team=review_req3.team, person = another_reviewer) + yet_another_reviewer = PersonFactory.create(name = u"YetAnotherExtra TestReviewer") # needs to be lexically greater than the exsting one + yet_another_reviewer.role_set.create(name_id='reviewer', email=yet_another_reviewer.email(), group=review_req1.team) + ReviewerSettingsFactory(team=review_req3.team, person = yet_another_reviewer) + # get - r = self.client.get(assigned_url) + r = self.client.get(unassigned_url) self.assertEqual(r.status_code, 200) self.assertTrue(review_req1.doc.name in unicontent(r)) - # can't save assigned: conflict - new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman") - # provoke conflict by posting bogus data - r = self.client.post(assigned_url, { - "reviewrequest": [str(review_req1.pk), str(review_req2.pk), str(123456)], - - # close - "r{}-existing_reviewer".format(review_req1.pk): "123456", - "r{}-action".format(review_req1.pk): "close", - "r{}-close".format(review_req1.pk): "no-response", - - # assign - "r{}-existing_reviewer".format(review_req2.pk): "123456", - "r{}-action".format(review_req2.pk): "assign", - "r{}-reviewer".format(review_req2.pk): new_reviewer.pk, - - "action": "save-continue", - }) - self.assertEqual(r.status_code, 200) - content = unicontent(r).lower() - self.assertTrue("1 request closed" in content) - self.assertTrue("2 requests changed assignment" in content) - - # can't save unassigned: conflict - r = self.client.post(unassigned_url, { - "reviewrequest": [str(123456)], - "action": "save-continue", - }) - self.assertEqual(r.status_code, 200) - content = unicontent(r).lower() - self.assertTrue("1 request opened" in content) - - # close and reassign assigned - new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman") - r = self.client.post(assigned_url, { - "reviewrequest": [str(review_req1.pk), str(review_req2.pk)], - - # close - "r{}-existing_reviewer".format(review_req1.pk): review_req1.reviewer_id or "", - "r{}-action".format(review_req1.pk): "close", - "r{}-close".format(review_req1.pk): "no-response", - - # assign - "r{}-existing_reviewer".format(review_req2.pk): review_req2.reviewer_id or "", - "r{}-action".format(review_req2.pk): "assign", - "r{}-reviewer".format(review_req2.pk): new_reviewer.pk, - "r{}-add_skip".format(review_req2.pk) : 1, - - "action": "save", - }) - self.assertEqual(r.status_code, 302) - - # no change on unassigned + # Test that conflicts are detected r = self.client.post(unassigned_url, { "reviewrequest": [str(review_req3.pk)], - # no change - "r{}-existing_reviewer".format(review_req3.pk): review_req3.reviewer_id or "", - "r{}-action".format(review_req3.pk): "", - "r{}-close".format(review_req3.pk): "no-response", - "r{}-reviewer".format(review_req3.pk): "", + "r{}-existing_reviewer".format(review_req3.pk): "", + "r{}-action".format(review_req3.pk): "assign", + "r{}-reviewer".format(review_req3.pk): another_reviewer.email_set.first().pk, + "r{}-add_skip".format(review_req3.pk): 1, + + "action": "save", + }) + self.assertEqual(r.status_code, 200) + content = unicontent(r).lower() + self.assertTrue("2 requests opened" in content) + + r = self.client.post(unassigned_url, { + "reviewrequest": [str(review_req1.pk),str(review_req2.pk),str(review_req3.pk)], + + "r{}-existing_reviewer".format(review_req3.pk): "", + "r{}-action".format(review_req3.pk): "assign", + "r{}-reviewer".format(review_req3.pk): another_reviewer.email_set.first().pk, + "r{}-add_skip".format(review_req3.pk): 1, "action": "save", }) self.assertEqual(r.status_code, 302) - review_req1, review_req2, review_req3 = reload_db_objects(review_req1, review_req2, review_req3) - self.assertEqual(review_req1.state_id, "no-response") - self.assertEqual(review_req2.state_id, "requested") - self.assertEqual(review_req2.reviewer, new_reviewer) - settings = ReviewerSettings.objects.filter(team=review_req2.team, person=new_reviewer.person).first() + review_req3 = reload_db_objects(review_req3) + settings = ReviewerSettings.objects.filter(team=review_req3.team, person=another_reviewer).first() self.assertEqual(settings.skip_next,1) - self.assertEqual(review_req3.state_id, "requested") - - r = self.client.post(assigned_url, { - "reviewrequest": [str(review_req2.pk)], - "r{}-existing_reviewer".format(review_req2.pk): review_req2.reviewer_id or "", - "r{}-action".format(review_req2.pk): "assign", - "r{}-reviewer".format(review_req2.pk): "", - "r{}-add_skip".format(review_req2.pk) : 0, - "action": "save", - }) - self.assertEqual(r.status_code, 302) - review_req2 = reload_db_objects(review_req2) - self.assertEqual(review_req2.state_id, "requested") - self.assertEqual(review_req2.reviewer, None) + self.assertEqual(review_req3.state_id, "assigned") def test_email_open_review_assignments(self): - review_req1 = ReviewRequestFactory(reviewer=EmailFactory(person__user__username='marschairman')) + review_req1 = ReviewRequestFactory() + ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman')) DBTemplateFactory.create(path='/group/defaults/email/open_assignments.txt', type_id='django', content = """ {% autoescape off %} Reviewer Deadline Draft - {% for r in review_requests %}{{ r.reviewer.person.plain_name|ljust:"22" }} {{ r.deadline|date:"Y-m-d" }} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %} + {% for r in review_assignments %}{{ r.reviewer.person.plain_name|ljust:"22" }} {{ r.review_request.deadline|date:"Y-m-d" }} {{ r.review_request.doc_id }}-{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}{{ r.review_request.doc.rev }}{% endif %} {% endfor %} {% if rotation_list %}Next in the reviewer rotation: @@ -376,15 +282,14 @@ class ReviewTests(TestCase): def test_change_reviewer_settings(self): reviewer = ReviewerSettingsFactory(person__user__username='reviewer',expertise='Some expertise').person - review_req = ReviewRequestFactory(reviewer=reviewer.email()) - RoleFactory(name_id='reviewer',group=review_req.team,person=review_req.reviewer.person) + review_req = ReviewRequestFactory() + assignment = ReviewAssignmentFactory(review_request=review_req,reviewer=reviewer.email()) + RoleFactory(name_id='reviewer',group=review_req.team,person=assignment.reviewer.person) RoleFactory(name_id='secr',group=review_req.team) - reviewer = review_req.reviewer.person - url = urlreverse(ietf.group.views.change_reviewer_settings, kwargs={ "acronym": review_req.team.acronym, - "reviewer_email": review_req.reviewer_id, + "reviewer_email": assignment.reviewer_id, }) login_testing_unauthorized(self, reviewer.user.username, url) @@ -392,7 +297,7 @@ class ReviewTests(TestCase): url = urlreverse(ietf.group.views.change_reviewer_settings, kwargs={ "group_type": review_req.team.type_id, "acronym": review_req.team.acronym, - "reviewer_email": review_req.reviewer_id, + "reviewer_email": assignment.reviewer_id, }) # get @@ -451,7 +356,7 @@ class ReviewTests(TestCase): msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower() self.assertTrue(start_date.isoformat(), msg_content) self.assertTrue("indefinite", msg_content) - self.assertEqual(period.reason, "Whimsy") + self.assertEqual(period.reason, "Whimsy") # end unavailable period empty_outbox() @@ -526,8 +431,9 @@ class ReviewTests(TestCase): self.assertEqual(settings.remind_days_before_deadline, 6) def test_review_reminders(self): - review_req = ReviewRequestFactory() + review_req = ReviewRequestFactory(state_id='assigned') reviewer = RoleFactory(name_id='reviewer',group=review_req.team,person__user__username='reviewer').person + assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on = review_req.time, reviewer=reviewer.email_set.first()) RoleFactory(name_id='secr',group=review_req.team,person__user__username='reviewsecretary') ReviewerSettingsFactory(team = review_req.team, person = reviewer) @@ -551,23 +457,23 @@ class ReviewTests(TestCase): review_req.save() # reviewer - needing_reminders = review_requests_needing_reviewer_reminder(today - datetime.timedelta(days=1)) + needing_reminders = review_assignments_needing_reviewer_reminder(today - datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) - needing_reminders = review_requests_needing_reviewer_reminder(today) - self.assertEqual(list(needing_reminders), [review_req]) + needing_reminders = review_assignments_needing_reviewer_reminder(today) + self.assertEqual(list(needing_reminders), [assignment]) - needing_reminders = review_requests_needing_reviewer_reminder(today + datetime.timedelta(days=1)) + needing_reminders = review_assignments_needing_reviewer_reminder(today + datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) # secretary - needing_reminders = review_requests_needing_secretary_reminder(today - datetime.timedelta(days=1)) + needing_reminders = review_assignments_needing_secretary_reminder(today - datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) - needing_reminders = review_requests_needing_secretary_reminder(today) - self.assertEqual(list(needing_reminders), [(review_req, secretary_role)]) + needing_reminders = review_assignments_needing_secretary_reminder(today) + self.assertEqual(list(needing_reminders), [(assignment, secretary_role)]) - needing_reminders = review_requests_needing_secretary_reminder(today + datetime.timedelta(days=1)) + needing_reminders = review_assignments_needing_secretary_reminder(today + datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) # email reviewer diff --git a/ietf/group/urls.py b/ietf/group/urls.py index 6f02b8b27..3a9a70235 100644 --- a/ietf/group/urls.py +++ b/ietf/group/urls.py @@ -41,7 +41,7 @@ info_detail_urls = [ url(r'^archives/$', views.derived_archives), url(r'^photos/$', views.group_photos), url(r'^reviews/$', views.review_requests), - url(r'^reviews/manage/(?Passigned|unassigned)/$', views.manage_review_requests), + url(r'^reviews/manage/(?Punassigned)/$', views.manage_review_requests), url(r'^reviews/email-assignments/$', views.email_open_review_assignments), url(r'^reviewers/$', views.reviewer_overview), url(r'^reviewers/(?P[\w%+-.@]+)/settings/$', views.change_reviewer_settings), diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 8ae517dd2..2fb088b51 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -223,7 +223,7 @@ def construct_group_menu_context(request, group, selected, group_type, others): if group.features.has_reviews and can_manage_review_requests_for_team(request.user, group): import ietf.group.views actions.append((u"Manage unassigned reviews", urlreverse(ietf.group.views.manage_review_requests, kwargs=dict(assignment_status="unassigned", **kwargs)))) - actions.append((u"Manage assigned reviews", urlreverse(ietf.group.views.manage_review_requests, kwargs=dict(assignment_status="assigned", **kwargs)))) + #actions.append((u"Manage assigned reviews", urlreverse(ietf.group.views.manage_review_requests, kwargs=dict(assignment_status="assigned", **kwargs)))) if Role.objects.filter(name="secr", group=group, person__user=request.user).exists(): actions.append((u"Secretary settings", urlreverse(ietf.group.views.change_review_secretary_settings, kwargs=kwargs))) diff --git a/ietf/group/views.py b/ietf/group/views.py index c6a405439..a60ff5d90 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -86,7 +86,7 @@ from ietf.meeting.helpers import get_meeting from ietf.meeting.utils import group_sessions from ietf.name.models import GroupTypeName, StreamName from ietf.person.models import Email -from ietf.review.models import ReviewRequest, ReviewerSettings, ReviewSecretarySettings +from ietf.review.models import ReviewRequest, ReviewAssignment, ReviewerSettings, ReviewSecretarySettings from ietf.review.utils import (can_manage_review_requests_for_team, can_access_review_stats_for_team, @@ -99,7 +99,7 @@ from ietf.review.utils import (can_manage_review_requests_for_team, current_unavailable_periods_for_reviewers, email_reviewer_availability_change, reviewer_rotation_list, - latest_review_requests_for_reviewers, + latest_review_assignments_for_reviewers, augment_review_requests_with_events, get_default_filter_re, days_needed_to_fulfill_min_interval_for_reviewers, @@ -108,7 +108,7 @@ from ietf.doc.models import LastCallDocEvent -from ietf.name.models import ReviewRequestStateName +from ietf.name.models import ReviewAssignmentStateName from ietf.utils.mail import send_mail_text, parse_preformatted from ietf.ietfauth.utils import user_is_person @@ -1263,27 +1263,26 @@ def group_menu_data(request): # --- Review views ----------------------------------------------------- def get_open_review_requests_for_team(team, assignment_status=None): - open_review_requests = ReviewRequest.objects.filter( - team=team, - state__in=("requested", "accepted") + open_review_requests = ReviewRequest.objects.filter(team=team).filter( + Q(state_id='requested') | Q(state_id='assigned',reviewassignment__state__in=('assigned','accepted')) ).prefetch_related( - "reviewer__person", "type", "state", "doc", "doc__states", + "type", "state", "doc", "doc__states", ).order_by("-time", "-id") if assignment_status == "unassigned": - open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(reviewer=None)) + open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(state_id='requested')) elif assignment_status == "assigned": - open_review_requests = list(open_review_requests.exclude(reviewer=None)) + open_review_requests = list(open_review_requests.filter(state_id='assigned')) else: open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests) - today = datetime.date.today() - unavailable_periods = current_unavailable_periods_for_reviewers(team) - for r in open_review_requests: - if r.reviewer: - r.reviewer_unavailable = any(p.availability == "unavailable" - for p in unavailable_periods.get(r.reviewer.person_id, [])) - r.due = max(0, (today - r.deadline).days) + #today = datetime.date.today() + #unavailable_periods = current_unavailable_periods_for_reviewers(team) + #for r in open_review_requests: + #if r.reviewer: + # r.reviewer_unavailable = any(p.availability == "unavailable" + # for p in unavailable_periods.get(r.reviewer.person_id, [])) + #r.due = max(0, (today - r.deadline).days) return open_review_requests @@ -1292,25 +1291,19 @@ def review_requests(request, acronym, group_type=None): if not group.features.has_reviews: raise Http404 - assigned_review_requests = [] - unassigned_review_requests = [] + unassigned_review_requests = [r for r in get_open_review_requests_for_team(group) if not r.state_id=='assigned'] - for r in get_open_review_requests_for_team(group): - if r.reviewer: - assigned_review_requests.append(r) - else: - unassigned_review_requests.append(r) + open_review_assignments = list(ReviewAssignment.objects.filter(review_request__team=group, state_id__in=('assigned','accepted')).order_by('-assigned_on')) + today = datetime.date.today() + unavailable_periods = current_unavailable_periods_for_reviewers(group) + for a in open_review_assignments: + a.reviewer_unavailable = any(p.availability == "unavailable" + for p in unavailable_periods.get(a.reviewer.person_id, [])) + a.due = max(0, (today - a.review_request.deadline).days) - open_review_requests = [ - ("Unassigned", unassigned_review_requests), - ("Assigned", assigned_review_requests), - ] + closed_review_assignments = ReviewAssignment.objects.filter(review_request__team=group).exclude(state_id__in=('assigned','accepted')).prefetch_related("state","result").order_by('-assigned_on') - closed_review_requests = ReviewRequest.objects.filter( - team=group, - ).exclude( - state__in=("requested", "accepted") - ).prefetch_related("reviewer__person", "type", "state", "doc", "result").order_by("-time", "-id") + closed_review_requests = ReviewRequest.objects.filter(team=group).exclude(state__in=("requested", "assigned")).prefetch_related("type", "state", "doc").order_by("-time", "-id") since_choices = [ (None, "1 month"), @@ -1338,10 +1331,14 @@ def review_requests(request, acronym, group_type=None): | Q(reviewrequestdocevent__isnull=True, time__gte=datetime.date.today() - date_limit) ).distinct() + closed_review_assignments = closed_review_assignments.filter(completed_on__gte = datetime.date.today() - date_limit) + return render(request, 'group/review_requests.html', construct_group_menu_context(request, group, "review requests", group_type, { - "open_review_requests": open_review_requests, + "unassigned_review_requests": unassigned_review_requests, + "open_review_assignments": open_review_assignments, "closed_review_requests": closed_review_requests, + "closed_review_assignments": closed_review_assignments, "since_choices": since_choices, "since": since, "can_manage_review_requests": can_manage_review_requests_for_team(request.user, group), @@ -1365,8 +1362,8 @@ def reviewer_overview(request, acronym, group_type=None): today = datetime.date.today() - req_data_for_reviewers = latest_review_requests_for_reviewers(group) - review_state_by_slug = { n.slug: n for n in ReviewRequestStateName.objects.all() } + req_data_for_reviewers = latest_review_assignments_for_reviewers(group) + assignment_state_by_slug = { n.slug: n for n in ReviewAssignmentStateName.objects.all() } days_needed = days_needed_to_fulfill_min_interval_for_reviewers(group) @@ -1387,15 +1384,16 @@ def reviewer_overview(request, acronym, group_type=None): person.busy = person.id in days_needed + # TODO - What is this MAX_CLOSED_REQS trying to accomplish? MAX_CLOSED_REQS = 10 days_since = 9999 req_data = req_data_for_reviewers.get(person.pk, []) - open_reqs = sum(1 for d in req_data if d.state in ["requested", "accepted"]) + open_reqs = sum(1 for d in req_data if d.state in ["assigned", "accepted"]) latest_reqs = [] for d in req_data: - if d.state in ["requested", "accepted"] or len(latest_reqs) < MAX_CLOSED_REQS + open_reqs: - latest_reqs.append((d.req_pk, d.doc, d.reviewed_rev, d.assigned_time, d.deadline, - review_state_by_slug.get(d.state), + if d.state in ["assigned", "accepted"] or len(latest_reqs) < MAX_CLOSED_REQS + open_reqs: + latest_reqs.append((d.assignment_pk, d.doc, d.reviewed_rev, d.assigned_time, d.deadline, + assignment_state_by_slug.get(d.state), int(math.ceil(d.assignment_to_closure_days)) if d.assignment_to_closure_days is not None else None)) if d.state in ["completed", "completed_in_time", "completed_late"]: if d.assigned_time is not None: @@ -1474,23 +1472,6 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status= for r in opened_reqs: review_requests_dict[r].form.add_error(None, "New request.") - for req in review_requests: - existing_reviewer = request.POST.get(req.form.prefix + "-existing_reviewer") - if existing_reviewer is None: - continue - - if existing_reviewer != unicode(req.reviewer_id or ""): - msg = "Assignment was changed." - a = req.form["action"].value() - if a == "assign": - msg += " Didn't assign reviewer." - elif a == "close": - msg += " Didn't close request." - req.form.add_error(None, msg) - req.form.data[req.form.prefix + "-action"] = "" # cancel the action - - newly_assigned += 1 - form_results = [] for req in review_requests: form_results.append(req.form.is_valid()) @@ -1565,37 +1546,35 @@ def email_open_review_assignments(request, acronym, group_type=None): if not can_manage_review_requests_for_team(request.user, group): return HttpResponseForbidden("You do not have permission to perform this action") - review_requests = list(ReviewRequest.objects.filter( - team=group, - state__in=("requested", "accepted"), - ).exclude( - reviewer=None, - ).prefetch_related("reviewer", "type", "state", "doc").distinct().order_by("reviewer","-deadline")) + review_assignments = list(ReviewAssignment.objects.filter( + review_request__team=group, + state__in=("assigned", "accepted"), + ).prefetch_related("reviewer", "review_request__type", "state", "review_request__doc").distinct().order_by("reviewer","-review_request__deadline")) - review_requests.sort(key=lambda r:r.reviewer.person.last_name()+r.reviewer.person.first_name()) + review_assignments.sort(key=lambda r:r.reviewer.person.last_name()+r.reviewer.person.first_name()) - for r in review_requests: - if r.doc.telechat_date(): - r.section = 'For telechat %s' % r.doc.telechat_date().isoformat() + for r in review_assignments: + if r.review_request.doc.telechat_date(): + r.section = 'For telechat %s' % r.review_request.doc.telechat_date().isoformat() r.section_order='0'+r.section - elif r.type_id == 'early': + elif r.review_request.type_id == 'early': r.section = 'Early review requests:' r.section_order='2' else: r.section = 'Last calls:' r.section_order='1' - e = r.doc.latest_event(LastCallDocEvent, type="sent_last_call") + e = r.review_request.doc.latest_event(LastCallDocEvent, type="sent_last_call") r.lastcall_ends = e and e.expires.date().isoformat() - r.earlier_review = ReviewRequest.objects.filter(doc=r.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed") + r.earlier_review = ReviewAssignment.objects.filter(review_request__doc=r.review_request.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed") if r.earlier_review: - req_rev = r.requested_rev or r.doc.rev + req_rev = r.review_request.requested_rev or r.review_request.doc.rev earlier_review_rev = r.earlier_review.aggregate(Max('reviewed_rev'))['reviewed_rev__max'] if req_rev == earlier_review_rev: r.earlier_review_mark = '**' else: r.earlier_review_mark = '*' - review_requests.sort(key=lambda r: r.section_order) + review_assignments.sort(key=lambda r: r.section_order) back_url = request.GET.get("next") if not back_url: @@ -1630,7 +1609,7 @@ def email_open_review_assignments(request, acronym, group_type=None): template = DBTemplate.objects.get(path="/group/defaults/email/open_assignments.txt") partial_msg = render_to_string(template.path, { - "review_requests": review_requests, + "review_assignments": review_assignments, "rotation_list": reviewer_rotation_list(group)[:10], "group" : group, }) @@ -1651,7 +1630,7 @@ def email_open_review_assignments(request, acronym, group_type=None): return render(request, 'group/email_open_review_assignments.html', { 'group': group, - 'review_requests': review_requests, + 'review_assignments': review_assignments, 'form': form, 'back_url': back_url, }) @@ -1737,14 +1716,14 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None): # the secretary might need to reassign # assignments, so mention the current ones - review_reqs = ReviewRequest.objects.filter(state__in=["requested", "accepted"], reviewer=reviewer_role.email, team=group) + review_assignments = ReviewAssignment.objects.filter(state__in=["assigned", "accepted"], reviewer=reviewer_role.email, review_request__team=group) msg += "\n\n" - if review_reqs: + if review_assignments: msg += "{} is currently assigned to review:".format(reviewer_role.person) - for r in review_reqs: + for r in review_assignments: msg += "\n\n" - msg += "{} (deadline: {})".format(r.doc_id, r.deadline.isoformat()) + msg += "{} (deadline: {})".format(r.review_request.doc_id, r.review_request.deadline.isoformat()) else: msg += "{} does not have any assignments currently.".format(reviewer_role.person) diff --git a/ietf/iesg/agenda.py b/ietf/iesg/agenda.py index a221c1d59..46a22adf3 100644 --- a/ietf/iesg/agenda.py +++ b/ietf/iesg/agenda.py @@ -12,7 +12,7 @@ import debug # pyflakes:ignore from ietf.doc.models import Document, LastCallDocEvent, ConsensusDocEvent from ietf.doc.utils_search import fill_in_telechat_date from ietf.iesg.models import TelechatDate, TelechatAgendaItem -from ietf.review.utils import review_requests_to_list_for_docs +from ietf.review.utils import review_assignments_to_list_for_docs def get_agenda_date(date=None): if not date: @@ -156,7 +156,7 @@ def fill_in_agenda_docs(date, sections, docs=None): docs = docs.select_related("stream", "group").distinct() fill_in_telechat_date(docs) - review_requests_for_docs = review_requests_to_list_for_docs(docs) + review_assignments_for_docs = review_assignments_to_list_for_docs(docs) for doc in docs: if doc.telechat_date() != date: @@ -182,7 +182,7 @@ def fill_in_agenda_docs(date, sections, docs=None): if e and (e.consensus != None): doc.consensus = "Yes" if e.consensus else "No" - doc.review_requests = review_requests_for_docs.get(doc.pk, []) + doc.review_assignments = review_assignments_for_docs.get(doc.pk, []) elif doc.type_id == "conflrev": doc.conflictdoc = doc.relateddocument_set.get(relationship__slug='conflrev').target.document elif doc.type_id == "charter": diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index fbc5831ee..e1ff2ed50 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -22,7 +22,7 @@ from ietf.ietfauth.htpasswd import update_htpasswd_file from ietf.mailinglists.models import Subscribed from ietf.person.models import Person, Email, PersonalApiKey, PERSON_API_KEY_ENDPOINTS from ietf.person.factories import PersonFactory, EmailFactory -from ietf.review.factories import ReviewRequestFactory +from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory from ietf.review.models import ReviewWish, UnavailablePeriod from ietf.utils.decorators import skip_coverage @@ -358,11 +358,12 @@ class IetfAuthTests(TestCase): self.assertTrue(self.username_in_htpasswd_file(user.username)) def test_review_overview(self): - review_req = ReviewRequestFactory(reviewer=EmailFactory(person__user__username='reviewer')) - RoleFactory(name_id='reviewer',group=review_req.team,person=review_req.reviewer.person) + review_req = ReviewRequestFactory() + assignment = ReviewAssignmentFactory(review_request=review_req,reviewer=EmailFactory(person__user__username='reviewer')) + RoleFactory(name_id='reviewer',group=review_req.team,person=assignment.reviewer.person) doc = review_req.doc - reviewer = review_req.reviewer.person + reviewer = assignment.reviewer.person UnavailablePeriod.objects.create( team=review_req.team, diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index dd1dbda97..4fe2e1349 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -65,7 +65,7 @@ from ietf.ietfauth.htpasswd import update_htpasswd_file from ietf.ietfauth.utils import role_required from ietf.mailinglists.models import Subscribed, Whitelisted from ietf.person.models import Person, Email, Alias, PersonalApiKey -from ietf.review.models import ReviewRequest, ReviewerSettings, ReviewWish +from ietf.review.models import ReviewerSettings, ReviewWish, ReviewAssignment from ietf.review.utils import unavailable_periods_to_list, get_default_filter_re from ietf.doc.fields import SearchableDocumentField from ietf.utils.decorators import person_required @@ -433,18 +433,18 @@ class AddReviewWishForm(forms.Form): @login_required def review_overview(request): - open_review_requests = ReviewRequest.objects.filter( + open_review_assignments = ReviewAssignment.objects.filter( reviewer__person__user=request.user, - state__in=["requested", "accepted"], + state__in=["assigned", "accepted"], ) today = Date.today() - for r in open_review_requests: - r.due = max(0, (today - r.deadline).days) + for r in open_review_assignments: + r.due = max(0, (today - r.review_request.deadline).days) - closed_review_requests = ReviewRequest.objects.filter( + closed_review_assignments = ReviewAssignment.objects.filter( reviewer__person__user=request.user, state__in=["no-response", "part-completed", "completed"], - ).order_by("-time")[:20] + ).order_by("-review_request__time")[:20] teams = Group.objects.filter(role__name="reviewer", role__person__user=request.user, state="active") @@ -483,8 +483,8 @@ def review_overview(request): review_wishes = ReviewWish.objects.filter(person__user=request.user).prefetch_related("team") return render(request, 'ietfauth/review_overview.html', { - 'open_review_requests': open_review_requests, - 'closed_review_requests': closed_review_requests, + 'open_review_assignments': open_review_assignments, + 'closed_review_assignments': closed_review_assignments, 'teams': teams, 'review_wishes': review_wishes, 'review_wish_form': review_wish_form, diff --git a/ietf/name/admin.py b/ietf/name/admin.py index 9eb0f5cc5..3924b7910 100644 --- a/ietf/name/admin.py +++ b/ietf/name/admin.py @@ -9,7 +9,7 @@ from ietf.name.models import ( LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName, ReviewRequestStateName, ReviewResultName, ReviewTypeName, RoleName, RoomResourceName, SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, TopicAudienceName, - DocUrlTagName) + DocUrlTagName, ReviewAssignmentStateName) from ietf.stats.models import CountryAlias @@ -68,6 +68,7 @@ admin.site.register(LiaisonStatementTagName, NameAdmin) admin.site.register(MeetingTypeName, NameAdmin) admin.site.register(NomineePositionStateName, NameAdmin) admin.site.register(ReviewRequestStateName, NameAdmin) +admin.site.register(ReviewAssignmentStateName, NameAdmin) admin.site.register(ReviewResultName, NameAdmin) admin.site.register(ReviewTypeName, NameAdmin) admin.site.register(RoleName, NameAdmin) diff --git a/ietf/name/migrations/0005_reviewassignmentstatename.py b/ietf/name/migrations/0005_reviewassignmentstatename.py new file mode 100644 index 000000000..dbf253e51 --- /dev/null +++ b/ietf/name/migrations/0005_reviewassignmentstatename.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2019-01-04 13:59 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0004_add_prefix_to_doctypenames'), + ] + + operations = [ + migrations.CreateModel( + name='ReviewAssignmentStateName', + fields=[ + ('slug', models.CharField(max_length=32, primary_key=True, serialize=False)), + ('name', models.CharField(max_length=255)), + ('desc', models.TextField(blank=True)), + ('used', models.BooleanField(default=True)), + ('order', models.IntegerField(default=0)), + ], + options={ + 'ordering': ['order', 'name'], + 'abstract': False, + }, + ), + ] diff --git a/ietf/name/migrations/0006_adjust_statenames.py b/ietf/name/migrations/0006_adjust_statenames.py new file mode 100644 index 000000000..18c464a8c --- /dev/null +++ b/ietf/name/migrations/0006_adjust_statenames.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2019-01-04 14:02 +from __future__ import unicode_literals + +from django.db import migrations + +def forward(apps, schema_editor): + ReviewRequestStateName = apps.get_model('name','ReviewRequestStateName') + ReviewAssignmentStateName = apps.get_model('name','ReviewAssignmentStateName') + + # TODO: Remove these newly unused states in a future release + ReviewRequestStateName.objects.filter(slug__in=['accepted', 'rejected', 'no-response', 'part-completed', 'completed', 'unknown']).update(used=False) + ReviewRequestStateName.objects.create(slug = 'assigned', name = 'Assigned', desc = 'The ReviewRequest has been assigned to at least one reviewer' , used = True, order = 0) + + assignment_states = [ + { 'slug': 'assigned', + 'name': 'Assigned', + 'desc': 'The review has been assigned to this reviewer', + 'used': True, + 'order': 0 + }, + { 'slug':'accepted', + 'name':'Accepted', + 'desc':'The reviewer has accepted the assignment', + 'used': True, + 'order':0 + }, + { 'slug':'rejected', + 'name':'Rejected', + 'desc':'The reviewer has rejected the assignment', + 'used': True, + 'order':0 + }, + { 'slug':'withdrawn', + 'name':'Withdrawn by Team', + 'desc':'The team secretary has withdrawn the assignment', + 'used': True, + 'order':0 + }, + { 'slug':'overtaken', + 'name':'Overtaken By Events', + 'desc':'The review was abandoned because of circumstances', + 'used': True, + 'order':0 + }, + { 'slug':'no-response', + 'name':'No Response', + 'desc':'The reviewer did not provide a review by the deadline', + 'used': True, + 'order':0 + }, + { 'slug':'part-completed', + 'name':'Partially Completed', + 'desc':'The reviewer partially completed the assignment', + 'used': True, + 'order':0 + }, + { 'slug':'completed', + 'name':'Completed', + 'desc':'The reviewer completed the assignment', + 'used': True, + 'order':0 + }, + { 'slug':'unknown', + 'name':'Unknown', + 'desc':'The assignment is was imported from an earlier database and its state could not be computed', + 'used':'True', + 'order':0 + } + ] + + for entry in assignment_states: + ReviewAssignmentStateName.objects.create(**entry) + + +def reverse(apps, schema_editor): + ReviewRequestStateName = apps.get_model('name','ReviewRequestStateName') + ReviewAssignmentStateName = apps.get_model('name','ReviewAssignmentStateName') + ReviewRequestStateName.objects.filter(slug__in=['accepted', 'rejected', 'no-response', 'part-completed', 'completed', 'unknown']).update(used=True) + ReviewRequestStateName.objects.filter(slug='assigned').update(used=False) + ReviewAssignmentStateName.objects.update(used=False) + + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0005_reviewassignmentstatename'), + ('doc','0011_reviewassignmentdocevent'), + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/name/models.py b/ietf/name/models.py index ac96ab9bc..5a257c22f 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -95,8 +95,9 @@ class LiaisonStatementEventTypeName(NameModel): class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" class ReviewRequestStateName(NameModel): - """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, - No Response, No Review of Version, No Review of Document, Partially Completed, Completed""" + """Requested, Assigned, Withdrawn, Overtaken By Events, No Review of Version, No Review of Document""" +class ReviewAssignmentStateName(NameModel): + """Accepted, Rejected, Withdrawn, Overtaken By Events, No Response, Partially Completed, Completed""" class ReviewTypeName(NameModel): """Early Review, Last Call, Telechat""" class ReviewResultName(NameModel): diff --git a/ietf/name/resources.py b/ietf/name/resources.py index 76dca2a15..83f8d0b6f 100644 --- a/ietf/name/resources.py +++ b/ietf/name/resources.py @@ -14,8 +14,9 @@ from ietf.name.models import ( AgendaTypeName, BallotPositionName, ConstraintNam ImportantDateName, IntendedStdLevelName, IprDisclosureStateName, IprEventTypeName, IprLicenseTypeName, LiaisonStatementEventTypeName, LiaisonStatementPurposeName, LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName, - ReviewRequestStateName, ReviewResultName, ReviewTypeName, RoleName, RoomResourceName, - SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, TopicAudienceName, ) + ReviewAssignmentStateName, ReviewRequestStateName, ReviewResultName, ReviewTypeName, + RoleName, RoomResourceName, SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, + TopicAudienceName, ) class TimeSlotTypeNameResource(ModelResource): class Meta: @@ -428,6 +429,20 @@ class ReviewRequestStateNameResource(ModelResource): } api.name.register(ReviewRequestStateNameResource()) +class ReviewAssignmentStateNameResource(ModelResource): + class Meta: + cache = SimpleCache() + queryset = ReviewAssignmentStateName.objects.all() + #resource_name = 'reviewassignmentstatename' + filtering = { + "slug": ALL, + "name": ALL, + "desc": ALL, + "used": ALL, + "order": ALL, + } +api.name.register(ReviewAssignmentStateNameResource()) + class ReviewTypeNameResource(ModelResource): class Meta: cache = SimpleCache() @@ -567,3 +582,5 @@ class AgendaTypeNameResource(ModelResource): "order": ALL, } api.name.register(AgendaTypeNameResource()) + + diff --git a/ietf/review/admin.py b/ietf/review/admin.py index 691cee738..4fbe41059 100644 --- a/ietf/review/admin.py +++ b/ietf/review/admin.py @@ -3,7 +3,7 @@ import simple_history from django.contrib import admin from ietf.review.models import (ReviewerSettings, ReviewSecretarySettings, UnavailablePeriod, - ReviewWish, NextReviewerInTeam, ReviewRequest, ReviewTeamSettings ) + ReviewWish, NextReviewerInTeam, ReviewRequest, ReviewAssignment, ReviewTeamSettings ) class ReviewerSettingsAdmin(simple_history.admin.SimpleHistoryAdmin): def acronym(self, obj): @@ -53,14 +53,23 @@ admin.site.register(NextReviewerInTeam, NextReviewerInTeamAdmin) class ReviewRequestAdmin(admin.ModelAdmin): list_display = ["doc", "time", "type", "team", "deadline"] list_display_links = ["doc"] - list_filter = ["team", "type", "state", "result"] + list_filter = ["team", "type", "state"] ordering = ["-id"] - raw_id_fields = ["doc", "team", "requested_by", "reviewer", "review"] + raw_id_fields = ["doc", "team", "requested_by"] date_hierarchy = "time" - search_fields = ["doc__name", "reviewer__person__name"] + search_fields = ["doc__name"] admin.site.register(ReviewRequest, ReviewRequestAdmin) +class ReviewAssignmentAdmin(admin.ModelAdmin): + list_display = ["review_request", "reviewer", "assigned_on", "result"] + list_filter = ["result", "state"] + ordering = ["-id"] + raw_id_fields = ["reviewer", "result"] + search_fields = ["review_request__doc__name"] + +admin.site.register(ReviewAssignment, ReviewAssignmentAdmin) + class ReviewTeamSettingsAdmin(admin.ModelAdmin): list_display = ["group", ] search_fields = ["group__acronym", ] diff --git a/ietf/review/factories.py b/ietf/review/factories.py index 80353b4be..7e7a2480f 100644 --- a/ietf/review/factories.py +++ b/ietf/review/factories.py @@ -1,7 +1,7 @@ import factory import datetime -from ietf.review.models import ReviewTeamSettings, ReviewRequest, ReviewerSettings +from ietf.review.models import ReviewTeamSettings, ReviewRequest, ReviewAssignment, ReviewerSettings from ietf.name.models import ReviewTypeName, ReviewResultName class ReviewTeamSettingsFactory(factory.DjangoModelFactory): @@ -39,6 +39,15 @@ class ReviewRequestFactory(factory.DjangoModelFactory): deadline = datetime.datetime.today()+datetime.timedelta(days=14) requested_by = factory.SubFactory('ietf.person.factories.PersonFactory') +class ReviewAssignmentFactory(factory.DjangoModelFactory): + class Meta: + model = ReviewAssignment + + review_request = factory.SubFactory('ietf.review.factories.ReviewRequestFactory') + state_id = 'assigned' + reviewer = factory.SubFactory('ietf.person.factories.EmailFactory') + assigned_on = datetime.datetime.now() + class ReviewerSettingsFactory(factory.DjangoModelFactory): class Meta: model = ReviewerSettings diff --git a/ietf/review/migrations/0009_refactor_review_request.py b/ietf/review/migrations/0009_refactor_review_request.py new file mode 100644 index 000000000..766dc209e --- /dev/null +++ b/ietf/review/migrations/0009_refactor_review_request.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2019-01-04 14:27 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('doc', '0009_move_non_url_externalurls_to_uploaded_filename'), + ('name', '0005_reviewassignmentstatename'), + ('person', '0008_auto_20181014_1448'), + ('review', '0008_remove_reviewrequest_old_id'), + ] + + operations = [ + migrations.CreateModel( + name='ReviewAssignment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('assigned_on', models.DateTimeField(blank=True, null=True)), + ('completed_on', models.DateTimeField(blank=True, null=True)), + ('reviewed_rev', models.CharField(blank=True, max_length=16, verbose_name=b'reviewed revision')), + ('mailarch_url', models.URLField(blank=True, null=True)), + ('result', ietf.utils.models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='name.ReviewResultName')), + ('review', ietf.utils.models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='doc.Document')), + ], + ), + migrations.RenameField( + model_name='reviewrequest', + old_name='result', + new_name='unused_result', + ), + migrations.RenameField( + model_name='reviewrequest', + old_name='review', + new_name='unused_review', + ), + migrations.RenameField( + model_name='reviewrequest', + old_name='reviewed_rev', + new_name='unused_reviewed_rev', + ), + migrations.RenameField( + model_name='reviewrequest', + old_name='reviewer', + new_name='unused_reviewer', + ), + migrations.AddField( + model_name='reviewassignment', + name='review_request', + field=ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='review.ReviewRequest'), + ), + migrations.AddField( + model_name='reviewassignment', + name='reviewer', + field=ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='person.Email'), + ), + migrations.AddField( + model_name='reviewassignment', + name='state', + field=ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='name.ReviewAssignmentStateName'), + ), + ] diff --git a/ietf/review/migrations/0010_populate_review_assignments.py b/ietf/review/migrations/0010_populate_review_assignments.py new file mode 100644 index 000000000..f18b63d61 --- /dev/null +++ b/ietf/review/migrations/0010_populate_review_assignments.py @@ -0,0 +1,121 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2019-01-04 14:34 +from __future__ import unicode_literals + +from tqdm import tqdm + +from django.db import migrations + +def assigned_time(model, request): + e = model.objects.filter(doc=request.doc, type="assigned_review_request", review_request=request).order_by('-time', '-id').first() + return e.time if e and e.time else None + +def done_time(model, request): + if request.unused_review and request.unused_review.time: + return request.unused_review.time + e = model.objects.filter(doc=request.doc, type="closed_review_request", review_request=request).order_by('-time', '-id').first() + time = e.time if e and e.time else None + return time if time else request.time + +def map_request_state_to_assignment_state(req_state_id): + if req_state_id == 'requested': + return 'assigned' + elif req_state_id in ('no-review-document', 'no-review-version'): + return 'withdrawn' + else: + return req_state_id + +def forward(apps, schema_editor): + ReviewRequest = apps.get_model('review', 'ReviewRequest') + ReviewAssignment = apps.get_model('review', 'ReviewAssignment') + Document = apps.get_model('doc', 'Document') + ReviewRequestDocEvent = apps.get_model('doc','ReviewRequestDocEvent') + ReviewAssignmentDocEvent = apps.get_model('doc','ReviewAssignmentDocEvent') + + print('') # introduce a newline before tqdm starts writing + + for request in tqdm(ReviewRequest.objects.exclude(unused_reviewer__isnull=True)): + assignment_state = map_request_state_to_assignment_state(request.state_id) + if not (assignment_state in (u'assigned', u'accepted', u'completed', u'no-response', u'overtaken', u'part-completed', u'rejected', u'withdrawn', u'unknown')): + print ("Trouble with review_request",request.pk,"with state",request.state_id) + exit(-1) + ReviewAssignment.objects.create( + review_request = request, + state_id = assignment_state, + reviewer = request.unused_reviewer, + assigned_on = assigned_time(ReviewRequestDocEvent, request), + completed_on = done_time(ReviewRequestDocEvent, request), + review = request.unused_review, + reviewed_rev = request.unused_reviewed_rev, + result = request.unused_result, + mailarch_url = request.unused_review and request.unused_review.external_url, + ) + Document.objects.filter(type_id='review').update(external_url='') + ReviewRequest.objects.filter(state_id__in=('accepted', 'rejected', 'no-response', 'part-completed', 'completed', 'unknown')).update(state_id='assigned') + ReviewRequest.objects.filter(state_id='requested',unused_reviewer__isnull=False).update(state_id='assigned') + + for req_event in tqdm(ReviewRequestDocEvent.objects.filter(type="assigned_review_request",review_request__unused_reviewer__isnull=False)): + ReviewAssignmentDocEvent.objects.create( + time = req_event.time, + type = req_event.type, + by = req_event.by, + doc = req_event.doc, + rev = req_event.rev, + desc = req_event.desc, + review_assignment = req_event.review_request.reviewassignment_set.first(), + state_id = 'assigned' + ) + + for req_event in tqdm(ReviewRequestDocEvent.objects.filter(type="closed_review_request", + state_id__in=('completed', 'no-response', 'part-completed', 'rejected', 'unknown', 'withdrawn'), + review_request__unused_reviewer__isnull=False)): + ReviewAssignmentDocEvent.objects.create( + time = req_event.time, + type = req_event.type, + by = req_event.by, + doc = req_event.doc, + rev = req_event.rev, + desc = req_event.desc, + review_assignment = req_event.review_request.reviewassignment_set.first(), + state_id = req_event.state_id + ) + + ReviewRequestDocEvent.objects.filter(type="closed_review_request", + state_id__in=('completed', 'no-response', 'part-completed', 'rejected', 'unknown', 'withdrawn'), + review_request__unused_reviewer__isnull=False).delete() + + +def reverse(apps, schema_editor): + ReviewAssignment = apps.get_model('review', 'ReviewAssignment') + ReviewRequestDocEvent = apps.get_model('doc','ReviewRequestDocEvent') + ReviewAssignmentDocEvent = apps.get_model('doc','ReviewAssignmentDocEvent') + + for assignment in tqdm(ReviewAssignment.objects.all()): + assignment.review_request.unused_review.external_url = assignment.mailarch_url + assignment.review_request.unused_review.save() + assignment.review_request.state_id = assignment.state_id + assignment.review_request.save() + + for asgn_event in tqdm(ReviewAssignmentDocEvent.objects.filter(state_id__in=('completed', 'no-response', 'part-completed', 'rejected', 'unknown', 'withdrawn'))): + ReviewRequestDocEvent.objects.create( + time = asgn_event.time, + type = asgn_event.type, + by = asgn_event.by, + doc = asgn_event.doc, + rev = asgn_event.rev, + desc = asgn_event.desc, + review_request = asgn_event.review_request, + state_id = asgn_event.state_id + ) + ReviewAssignmentDocEvent.objects.all().delete() + +class Migration(migrations.Migration): + + dependencies = [ + ('review', '0009_refactor_review_request'), + ('doc','0011_reviewassignmentdocevent') + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/review/models.py b/ietf/review/models.py index d4a89619d..dab758d50 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -7,7 +7,7 @@ from django.db import models from ietf.doc.models import Document from ietf.group.models import Group from ietf.person.models import Person, Email -from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName +from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName, ReviewAssignmentStateName from ietf.utils.validators import validate_regular_expression_string from ietf.utils.models import ForeignKey, OneToOneField @@ -105,9 +105,7 @@ class NextReviewerInTeam(models.Model): verbose_name_plural = "next reviewer in team settings" class ReviewRequest(models.Model): - """Represents a request for a review and the process it goes through. - There should be one ReviewRequest entered for each combination of - document, rev, and reviewer.""" + """Represents a request for a review and the process it goes through.""" state = ForeignKey(ReviewRequestStateName) # Fields filled in on the initial record creation - these @@ -121,34 +119,38 @@ class ReviewRequest(models.Model): requested_rev = models.CharField(verbose_name="requested revision", max_length=16, blank=True, help_text="Fill in if a specific revision is to be reviewed, e.g. 02") comment = models.TextField(verbose_name="Requester's comments and instructions", max_length=2048, blank=True, help_text="Provide any additional information to show to the review team secretary and reviewer", default='') - # Fields filled in as reviewer is assigned and as the review is - # uploaded. Once these are filled in and we progress beyond being - # requested/assigned, any changes to the assignment happens by - # closing down the current request and making a new one, copying - # the request-part fields above. - reviewer = ForeignKey(Email, blank=True, null=True) + # Moved to class ReviewAssignment: + # These exist only to facilitate data migrations. They will be removed in the next release. + unused_reviewer = ForeignKey(Email, blank=True, null=True) - review = OneToOneField(Document, blank=True, null=True) - reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) - result = ForeignKey(ReviewResultName, blank=True, null=True) + unused_review = OneToOneField(Document, blank=True, null=True) + unused_reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) + unused_result = ForeignKey(ReviewResultName, blank=True, null=True) def __unicode__(self): return u"%s review on %s by %s %s" % (self.type, self.doc, self.team, self.state) - def other_requests(self): - return self.doc.reviewrequest_set.exclude(id=self.id) + def all_completed_assignments_for_doc(self): + return ReviewAssignment.objects.filter(review_request__doc=self.doc, state__in=['completed','part-completed']) - def other_completed_requests(self): - return self.other_requests().filter(state_id__in=['completed','part-completed']) + def request_closed_time(self): + return self.doc.request_closed_time(self) or self.time + +class ReviewAssignment(models.Model): + """ One of possibly many reviews assigned in response to a ReviewRequest """ + review_request = ForeignKey(ReviewRequest) + state = ForeignKey(ReviewAssignmentStateName) + reviewer = ForeignKey(Email) + assigned_on = models.DateTimeField(blank=True, null=True) + completed_on = models.DateTimeField(blank=True, null=True) + review = OneToOneField(Document, blank=True, null=True) + reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) + result = ForeignKey(ReviewResultName, blank=True, null=True) + mailarch_url = models.URLField(blank=True, null = True) + + def __unicode__(self): + return u"Assignment for %s (%s) : %s %s of %s" % (self.reviewer.person, self.state, self.review_request.team.acronym, self.review_request.type, self.review_request.doc) - def review_done_time(self): - # First check if this is completed review having review and if so take time from there. - if self.review and self.review.time: - return self.review.time - # If not, then it is closed review, so it either has event in doc or if not then take - # time from the request. - time = self.doc.request_closed_time(self) - return time if time else self.time def get_default_review_types(): return ReviewTypeName.objects.filter(slug__in=['early','lc','telechat']) diff --git a/ietf/review/resources.py b/ietf/review/resources.py index 059b801c5..abbf10202 100644 --- a/ietf/review/resources.py +++ b/ietf/review/resources.py @@ -7,7 +7,7 @@ from tastypie.cache import SimpleCache from ietf import api from ietf.api import ToOneField # pyflakes:ignore -from ietf.review.models import (ReviewerSettings, ReviewRequest, +from ietf.review.models import (ReviewerSettings, ReviewRequest, ReviewAssignment, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewSecretarySettings, ReviewTeamSettings, HistoricalReviewerSettings ) @@ -45,9 +45,6 @@ class ReviewRequestResource(ModelResource): doc = ToOneField(DocumentResource, 'doc') team = ToOneField(GroupResource, 'team') requested_by = ToOneField(PersonResource, 'requested_by') - reviewer = ToOneField(EmailResource, 'reviewer', null=True) - review = ToOneField(DocumentResource, 'review', null=True) - result = ToOneField(ReviewResultNameResource, 'result', null=True) class Meta: queryset = ReviewRequest.objects.all() serializer = api.Serializer() @@ -59,17 +56,38 @@ class ReviewRequestResource(ModelResource): "deadline": ALL, "requested_rev": ALL, "comment": ALL, - "reviewed_rev": ALL, "state": ALL_WITH_RELATIONS, "type": ALL_WITH_RELATIONS, "doc": ALL_WITH_RELATIONS, "team": ALL_WITH_RELATIONS, "requested_by": ALL_WITH_RELATIONS, + } +api.review.register(ReviewRequestResource()) + +from ietf.name.resources import ReviewAssignmentStateNameResource +class ReviewAssignmentResource(ModelResource): + review_request = ToOneField(ReviewRequest, 'review_request') + state = ToOneField(ReviewAssignmentStateNameResource, 'state') + reviewer = ToOneField(EmailResource, 'reviewer', null=True) + review = ToOneField(DocumentResource, 'review', null=True) + result = ToOneField(ReviewResultNameResource, 'result', null=True) + class Meta: + queryset = ReviewAssignment.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewassignment' + filtering = { + "id": ALL, + "reviewed_rev": ALL, + "mailarch_url": ALL, + "assigned_on": ALL, + "completed_on": ALL, + "state": ALL_WITH_RELATIONS, "reviewer": ALL_WITH_RELATIONS, "review": ALL_WITH_RELATIONS, "result": ALL_WITH_RELATIONS, } -api.review.register(ReviewRequestResource()) +api.review.register(ReviewAssignmentResource()) from ietf.person.resources import PersonResource from ietf.group.resources import GroupResource diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 32994b329..aa9fc888d 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -13,13 +13,13 @@ from django.contrib.sites.models import Site import debug # pyflakes:ignore from ietf.group.models import Group, Role -from ietf.doc.models import (Document, ReviewRequestDocEvent, State, +from ietf.doc.models import (Document, ReviewRequestDocEvent, ReviewAssignmentDocEvent, State, LastCallDocEvent, TelechatDocEvent, DocumentAuthor, DocAlias) from ietf.iesg.models import TelechatDate from ietf.person.models import Person from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream -from ietf.review.models import (ReviewRequest, ReviewRequestStateName, ReviewTypeName, +from ietf.review.models import (ReviewRequest, ReviewAssignment, ReviewRequestStateName, ReviewTypeName, ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewTeamSettings, ReviewSecretarySettings) from ietf.utils.mail import send_mail, get_email_addresses_from_text @@ -29,7 +29,7 @@ def active_review_teams(): return Group.objects.filter(reviewteamsettings__isnull=False,state="active") def close_review_request_states(): - return ReviewRequestStateName.objects.filter(used=True).exclude(slug__in=["requested", "accepted", "rejected", "part-completed", "completed"]) + return ReviewRequestStateName.objects.filter(used=True).exclude(slug__in=["requested", "assigned"]) def can_request_review_of_doc(user, doc): if not user.is_authenticated: @@ -55,14 +55,14 @@ def can_access_review_stats_for_team(user, team): return (Role.objects.filter(name__in=("secr", "reviewer"), person__user=user, group=team).exists() or has_role(user, ["Secretariat", "Area Director"])) -def review_requests_to_list_for_docs(docs): - request_qs = ReviewRequest.objects.filter( - state__in=["requested", "accepted", "part-completed", "completed"], +def review_assignments_to_list_for_docs(docs): + assignment_qs = ReviewAssignment.objects.filter( + state__in=["assigned", "accepted", "part-completed", "completed"], ).prefetch_related("result") doc_names = [d.name for d in docs] - return extract_revision_ordered_review_requests_for_documents_and_replaced(request_qs, doc_names) + return extract_revision_ordered_review_assignments_for_documents_and_replaced(assignment_qs, doc_names) def augment_review_requests_with_events(review_reqs): req_dict = { r.pk: r for r in review_reqs } @@ -72,7 +72,7 @@ def augment_review_requests_with_events(review_reqs): def no_review_from_teams_on_doc(doc, rev): return Group.objects.filter( reviewrequest__doc__name=doc.name, - reviewrequest__reviewed_rev=rev, + reviewrequest__requested_rev=rev, reviewrequest__state__slug="no-review-version", ).distinct() @@ -145,9 +145,9 @@ def days_needed_to_fulfill_min_interval_for_reviewers(team): """Returns person_id -> days needed until min_interval is fulfilled for reviewer (in case it is necessary to wait, otherwise reviewer is absent in result).""" - latest_assignments = dict(ReviewRequest.objects.filter( - team=team, - ).values_list("reviewer__person").annotate(Max("time"))) + latest_assignments = dict(ReviewAssignment.objects.filter( + review_request__team=team, + ).values_list("reviewer__person").annotate(Max("assigned_on"))) min_intervals = dict(ReviewerSettings.objects.filter(team=team).values_list("person_id", "min_interval")) @@ -166,40 +166,39 @@ def days_needed_to_fulfill_min_interval_for_reviewers(team): return res -ReviewRequestData = namedtuple("ReviewRequestData", [ - "req_pk", "doc", "doc_pages", "req_time", "state", "assigned_time", "deadline", "reviewed_rev", "result", "team", "reviewer", +ReviewAssignmentData = namedtuple("ReviewAssignmentData", [ + "assignment_pk", "doc", "doc_pages", "req_time", "state", "assigned_time", "deadline", "reviewed_rev", "result", "team", "reviewer", "late_days", "request_to_assignment_days", "assignment_to_closure_days", "request_to_closure_days"]) -def extract_review_request_data(teams=None, reviewers=None, time_from=None, time_to=None, ordering=[]): - """Yield data on each review request, sorted by (*ordering, time) +def extract_review_assignment_data(teams=None, reviewers=None, time_from=None, time_to=None, ordering=[]): + """Yield data on each review assignment, sorted by (*ordering, assigned_on) for easy use with itertools.groupby. Valid entries in *ordering are "team" and "reviewer".""" filters = Q() if teams: - filters &= Q(team__in=teams) + filters &= Q(review_request__team__in=teams) if reviewers: filters &= Q(reviewer__person__in=reviewers) if time_from: - filters &= Q(time__gte=time_from) + filters &= Q(review_request__time__gte=time_from) if time_to: - filters &= Q(time__lte=time_to) + filters &= Q(review_request__time__lte=time_to) - # we may be dealing with a big bunch of data, so treat it carefully - event_qs = ReviewRequest.objects.filter(filters) + # This doesn't do the left-outer join on docevent that the previous code did. These variables could be renamed + event_qs = ReviewAssignment.objects.filter(filters) - # left outer join with RequestRequestDocEvent for request/assign/close time event_qs = event_qs.values_list( - "pk", "doc", "doc__pages", "time", "state", "deadline", "reviewed_rev", "result", "team", - "reviewer__person", "reviewrequestdocevent__time", "reviewrequestdocevent__type" + "pk", "review_request__doc", "review_request__doc__pages", "review_request__time", "state", "review_request__deadline", "reviewed_rev", "result", "review_request__team", + "reviewer__person", "assigned_on", "completed_on" ) - event_qs = event_qs.order_by(*[o.replace("reviewer", "reviewer__person") for o in ordering] + ["time", "pk", "-reviewrequestdocevent__time"]) + event_qs = event_qs.order_by(*[o.replace("reviewer", "reviewer__person").replace("team","review_request__team") for o in ordering] + ["review_request__time", "assigned_on", "pk", "completed_on"]) def positive_days(time_from, time_to): if time_from is None or time_to is None: @@ -212,33 +211,31 @@ def extract_review_request_data(teams=None, reviewers=None, time_from=None, time else: return 0.0 - for _, events in itertools.groupby(event_qs.iterator(), lambda t: t[0]): - requested_time = assigned_time = closed_time = None + requested_time = assigned_time = closed_time = None - for e in events: - req_pk, doc, doc_pages, req_time, state, deadline, reviewed_rev, result, team, reviewer, event_time, event_type = e + for assignment in event_qs: - if event_type == "requested_review" and requested_time is None: - requested_time = event_time - elif event_type == "assigned_review_request" and assigned_time is None: - assigned_time = event_time - elif event_type == "closed_review_request" and closed_time is None: - closed_time = event_time + assignment_pk, doc, doc_pages, req_time, state, deadline, reviewed_rev, result, team, reviewer, assigned_on, completed_on = assignment + + requested_time = req_time + assigned_time = assigned_on + closed_time = completed_on late_days = positive_days(datetime.datetime.combine(deadline, datetime.time.max), closed_time) request_to_assignment_days = positive_days(requested_time, assigned_time) assignment_to_closure_days = positive_days(assigned_time, closed_time) request_to_closure_days = positive_days(requested_time, closed_time) - d = ReviewRequestData(req_pk, doc, doc_pages, req_time, state, assigned_time, deadline, reviewed_rev, result, team, reviewer, + d = ReviewAssignmentData(assignment_pk, doc, doc_pages, req_time, state, assigned_time, deadline, reviewed_rev, result, team, reviewer, late_days, request_to_assignment_days, assignment_to_closure_days, request_to_closure_days) yield d -def aggregate_raw_period_review_request_stats(review_request_data, count=None): + +def aggregate_raw_period_review_assignment_stats(review_assignment_data, count=None): """Take a sequence of review request data from - extract_review_request_data and aggregate them.""" + extract_review_assignment_data and aggregate them.""" state_dict = defaultdict(int) late_state_dict = defaultdict(int) @@ -246,8 +243,8 @@ def aggregate_raw_period_review_request_stats(review_request_data, count=None): assignment_to_closure_days_list = [] assignment_to_closure_days_count = 0 - for (req_pk, doc, doc_pages, req_time, state, assigned_time, deadline, reviewed_rev, result, team, reviewer, - late_days, request_to_assignment_days, assignment_to_closure_days, request_to_closure_days) in review_request_data: + for (assignment_pk, doc, doc_pages, req_time, state, assigned_time, deadline, reviewed_rev, result, team, reviewer, + late_days, request_to_assignment_days, assignment_to_closure_days, request_to_closure_days) in review_assignment_data: if count == "pages": c = doc_pages else: @@ -266,7 +263,7 @@ def aggregate_raw_period_review_request_stats(review_request_data, count=None): return state_dict, late_state_dict, result_dict, assignment_to_closure_days_list, assignment_to_closure_days_count -def sum_period_review_request_stats(raw_aggregation): +def sum_period_review_assignment_stats(raw_aggregation): """Compute statistics from aggregated review request data for one aggregation point.""" state_dict, late_state_dict, result_dict, assignment_to_closure_days_list, assignment_to_closure_days_count = raw_aggregation @@ -274,11 +271,11 @@ def sum_period_review_request_stats(raw_aggregation): res["state"] = state_dict res["result"] = result_dict - res["open"] = sum(state_dict.get(s, 0) for s in ("requested", "accepted")) + res["open"] = sum(state_dict.get(s, 0) for s in ("assigned", "accepted")) res["completed"] = sum(state_dict.get(s, 0) for s in ("completed", "part-completed")) res["not_completed"] = sum(state_dict.get(s, 0) for s in state_dict if s in ("rejected", "withdrawn", "overtaken", "no-response")) - res["open_late"] = sum(late_state_dict.get(s, 0) for s in ("requested", "accepted")) + res["open_late"] = sum(late_state_dict.get(s, 0) for s in ("assigned", "accepted")) res["open_in_time"] = res["open"] - res["open_late"] res["completed_late"] = sum(late_state_dict.get(s, 0) for s in ("completed", "part-completed")) res["completed_in_time"] = res["completed"] - res["completed_late"] @@ -287,7 +284,7 @@ def sum_period_review_request_stats(raw_aggregation): return res -def sum_raw_review_request_aggregations(raw_aggregations): +def sum_raw_review_assignment_aggregations(raw_aggregations): """Collapse a sequence of aggregations into one aggregation.""" state_dict = defaultdict(int) late_state_dict = defaultdict(int) @@ -309,34 +306,61 @@ def sum_raw_review_request_aggregations(raw_aggregations): return state_dict, late_state_dict, result_dict, assignment_to_closure_days_list, assignment_to_closure_days_count -def latest_review_requests_for_reviewers(team, days_back=365): - """Collect and return stats for reviewers on latest requests, in - extract_review_request_data format.""" +def latest_review_assignments_for_reviewers(team, days_back=365): + """Collect and return stats for reviewers on latest assignments, in + extract_review_assignment_data format.""" - extracted_data = extract_review_request_data( + extracted_data = extract_review_assignment_data( teams=[team], time_from=datetime.date.today() - datetime.timedelta(days=days_back), ordering=["reviewer"], ) - req_data_for_reviewers = { + assignment_data_for_reviewers = { reviewer: list(reversed(list(req_data_items))) for reviewer, req_data_items in itertools.groupby(extracted_data, key=lambda data: data.reviewer) } - return req_data_for_reviewers + return assignment_data_for_reviewers -def make_new_review_request_from_existing(review_req): - obj = ReviewRequest() - obj.time = review_req.time - obj.type = review_req.type - obj.doc = review_req.doc - obj.team = review_req.team - obj.deadline = review_req.deadline - obj.requested_rev = review_req.requested_rev - obj.requested_by = review_req.requested_by - obj.state = ReviewRequestStateName.objects.get(slug="requested") - return obj +def email_review_assignment_change(request, review_assignment, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by): + + system_email = Person.objects.get(name="(System)").formatted_email() + + to = set() + + def extract_email_addresses(objs): + for o in objs: + if o and o.person!=by: + e = o.formatted_email() + if e != system_email: + to.add(e) + + if notify_secretary: + rts = ReviewTeamSettings.objects.filter(group=review_assignment.review_request.team).first() + if rts and rts.secr_mail_alias and rts.secr_mail_alias.strip() != '': + for addr in get_email_addresses_from_text(rts.secr_mail_alias): + to.add(addr) + else: + extract_email_addresses(Role.objects.filter(name="secr", group=review_assignment.review_request.team).distinct()) + if notify_reviewer: + extract_email_addresses([review_assignment.reviewer]) + if notify_requested_by: + extract_email_addresses([review_assignment.review_request.requested_by.email()]) + + if not to: + return + + to = list(to) + + url = urlreverse("ietf.doc.views_review.review_request_forced_login", kwargs={ "name": review_assignment.review_request.doc.name, "request_id": review_assignment.review_request.pk }) + url = request.build_absolute_uri(url) + send_mail(request, to, request.user.person.formatted_email(), subject, "review/review_request_changed.txt", { + "review_req_url": url, + "review_req": review_assignment.review_request, + "msg": msg, + }) + def email_review_request_change(request, review_req, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by): @@ -362,7 +386,8 @@ def email_review_request_change(request, review_req, subject, msg, by, notify_se else: extract_email_addresses(Role.objects.filter(name="secr", group=review_req.team).distinct()) if notify_reviewer: - extract_email_addresses([review_req.reviewer]) + for assignment in review_req.reviewassignment_set.all(): + extract_email_addresses([assignment.reviewer]) if notify_requested_by: extract_email_addresses([review_req.requested_by.email()]) @@ -423,24 +448,21 @@ def email_reviewer_availability_change(request, team, reviewer_role, msg, by): }) def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=False): - assert review_req.state_id in ("requested", "accepted") + assert review_req.state_id in ("requested", "assigned") - if reviewer == review_req.reviewer: + if review_req.reviewassignment_set.filter(reviewer=reviewer).exists(): return - if review_req.reviewer: - email_review_request_change( - request, review_req, - "Unassigned from review of %s" % review_req.doc.name, - "%s has cancelled your assignment to the review." % request.user.person, - by=request.user.person, notify_secretary=False, notify_reviewer=True, notify_requested_by=False) + # Note that assigning a review no longer unassigns other reviews - review_req.state = ReviewRequestStateName.objects.get(slug="requested") - review_req.reviewer = reviewer - review_req.save() + if review_req.state_id != 'assigned': + review_req.state_id = 'assigned' + review_req.save() + + assignment = review_req.reviewassignment_set.create(state_id='assigned', reviewer = reviewer, assigned_on = datetime.datetime.now()) - if review_req.reviewer: - possibly_advance_next_reviewer_for_team(review_req.team, review_req.reviewer.person_id, add_skip) + if reviewer: + possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip) ReviewRequestDocEvent.objects.create( type="assigned_review_request", @@ -450,26 +472,40 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa desc="Request for {} review by {} is assigned to {}".format( review_req.type.name, review_req.team.acronym.upper(), - review_req.reviewer.person if review_req.reviewer else "(None)", + reviewer.person if reviewer else "(None)", ), review_request=review_req, - state=None, + state_id='assigned', + ) + + ReviewAssignmentDocEvent.objects.create( + type="assigned_review_request", + doc=review_req.doc, + rev=review_req.doc.rev, + by=request.user.person, + desc="Request for {} review by {} is assigned to {}".format( + review_req.type.name, + review_req.team.acronym.upper(), + reviewer.person if reviewer else "(None)", + ), + review_assignment=assignment, + state_id='assigned', ) msg = "%s has assigned you as a reviewer for this document." % request.user.person.ascii - prev_team_reviews = ReviewRequest.objects.filter( - doc=review_req.doc, + prev_team_reviews = ReviewAssignment.objects.filter( + review_request__doc=review_req.doc, state="completed", - team=review_req.team, + review_request__team=review_req.team, ) if prev_team_reviews.exists(): msg = msg + '\n\nThis team has completed other reviews of this document:\n' - for req in prev_team_reviews: + for assignment in prev_team_reviews: msg += u'%s %s -%s %s\n'% ( - req.review_done_time().strftime('%d %b %Y'), - req.reviewer.person.ascii, - req.reviewed_rev or req.requested_rev, - req.result.name, + assignment.completed_on.strftime('%d %b %Y'), + assignment.reviewer.person.ascii, + assignment.reviewed_rev or assignment.review_request.requested_rev, + assignment.result.name, ) email_review_request_change( @@ -526,8 +562,9 @@ def close_review_request(request, review_req, close_state): suggested_req = review_req.pk is None review_req.state = close_state - if close_state.slug == "no-review-version": - review_req.reviewed_rev = review_req.requested_rev or review_req.doc.rev # save rev for later reference +# This field no longer exists, and it's not clear what the later reference was... +# if close_state.slug == "no-review-version": +# review_req.reviewed_rev = review_req.requested_rev or review_req.doc.rev # save rev for later reference review_req.save() if not suggested_req: @@ -542,6 +579,19 @@ def close_review_request(request, review_req, close_state): state=review_req.state, ) + for assignment in review_req.reviewassignment_set.filter(state_id__in=['assigned','accepted']): + assignment.state_id = 'withdrawn' + assignment.save() + ReviewAssignmentDocEvent.objects.create( + type='closed_review_request', + doc=review_req.doc, + rev=review_req.doc.rev, + by=request.user.person, + desc="Request closed, assignment withdrawn: {} {} {} review".format(assignment.reviewer.person.plain_name, assignment.review_request.type.name, assignment.review_request.team.acronym.upper()), + review_assignment=assignment, + state=assignment.state, + ) + email_review_request_change( request, review_req, "Closed review request for {}: {}".format(review_req.doc.name, close_state.name), @@ -636,7 +686,7 @@ def suggested_review_requests_for_team(team): seen_deadlines[doc_pk] = deadline - # filter those with existing requests + # filter those with existing explicit requests existing_requests = defaultdict(list) for r in ReviewRequest.objects.filter(doc__in=requests.iterkeys(), team=team): existing_requests[r.doc_id].append(r) @@ -646,18 +696,67 @@ def suggested_review_requests_for_team(team): return False no_review_document = existing.state_id == "no-review-document" - pending = (existing.state_id in ("requested", "accepted") + no_review_rev = ( existing.state_id == "no-review-version") and (not existing.requested_rev or existing.requested_rev == request.doc.rev) + pending = (existing.state_id == "assigned" + and existing.reviewassignment_set.filter(state_id__in=("assigned", "accepted")).exists() and (not existing.requested_rev or existing.requested_rev == request.doc.rev)) - completed_or_closed = (existing.state_id not in ("part-completed", "rejected", "overtaken", "no-response") - and existing.reviewed_rev == request.doc.rev) + request_closed = existing.state_id not in ('requested','assigned') + # at least one assignment was completed for the requested version or the current doc version if no specific version was requested: + some_assignment_completed = existing.reviewassignment_set.filter(reviewed_rev=existing.requested_rev or existing.doc.rev, state_id='completed').exists() - return no_review_document or pending or completed_or_closed + return any([no_review_document, no_review_rev, pending, request_closed, some_assignment_completed]) res = [r for r in requests.itervalues() if not any(blocks(e, r) for e in existing_requests[r.doc_id])] res.sort(key=lambda r: (r.deadline, r.doc_id), reverse=True) return res +def extract_revision_ordered_review_assignments_for_documents_and_replaced(review_assignment_queryset, names): + """Extracts all review assignments for document names (including replaced ancestors), return them neatly sorted.""" + + names = set(names) + + replaces = extract_complete_replaces_ancestor_mapping_for_docs(names) + + assignments_for_each_doc = defaultdict(list) + for r in review_assignment_queryset.filter(review_request__doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-reviewed_rev","-assigned_on", "-id").iterator(): + assignments_for_each_doc[r.review_request.doc_id].append(r) + + # now collect in breadth-first order to keep the revision order intact + res = defaultdict(list) + for name in names: + front = replaces.get(name, []) + res[name].extend(assignments_for_each_doc.get(name, [])) + + seen = set() + + while front: + replaces_assignments = [] + next_front = [] + for replaces_name in front: + if replaces_name in seen: + continue + + seen.add(replaces_name) + + assignments = assignments_for_each_doc.get(replaces_name, []) + if assignments: + replaces_assignments.append(assignments) + + next_front.extend(replaces.get(replaces_name, [])) + + # in case there are multiple replaces, move the ones with + # the latest reviews up front + replaces_assignments.sort(key=lambda l: l[0].assigned_on, reverse=True) + + for assignments in replaces_assignments: + res[name].extend(assignments) + + # move one level down + front = next_front + + return res + def extract_revision_ordered_review_requests_for_documents_and_replaced(review_request_queryset, names): """Extracts all review requests for document names (including replaced ancestors), return them neatly sorted.""" @@ -666,7 +765,7 @@ def extract_revision_ordered_review_requests_for_documents_and_replaced(review_r replaces = extract_complete_replaces_ancestor_mapping_for_docs(names) requests_for_each_doc = defaultdict(list) - for r in review_request_queryset.filter(doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-reviewed_rev", "-time", "-id").iterator(): + for r in review_request_queryset.filter(doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-time", "-id").iterator(): requests_for_each_doc[r.doc_id].append(r) # now collect in breadth-first order to keep the revision order intact @@ -704,10 +803,12 @@ def extract_revision_ordered_review_requests_for_documents_and_replaced(review_r return res +# TODO : Change this field to deal with multiple already assigned reviewers def setup_reviewer_field(field, review_req): field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team) - if review_req.reviewer: - field.initial = review_req.reviewer_id + one_assignment = review_req.reviewassignment_set.first() + if one_assignment: + field.initial = one_assignment.reviewer_id choices = make_assignment_choices(field.queryset, review_req) if not field.required: @@ -752,15 +853,15 @@ def make_assignment_choices(email_queryset, review_req): # previous review of document has_reviewed_previous = ReviewRequest.objects.filter( doc=doc, - reviewer__person__in=possible_person_ids, - state="completed", + reviewassignment__reviewer__person__in=possible_person_ids, + reviewassignment__state="completed", team=team, - ) + ).distinct() if review_req.pk is not None: has_reviewed_previous = has_reviewed_previous.exclude(pk=review_req.pk) - has_reviewed_previous = set(has_reviewed_previous.values_list("reviewer__person", flat=True)) + has_reviewed_previous = set(has_reviewed_previous.values_list("reviewassignment__reviewer__person", flat=True)) # review wishes wish_to_review = set(ReviewWish.objects.filter(team=team, person__in=possible_person_ids, doc=doc).values_list("person", flat=True)) @@ -780,7 +881,7 @@ def make_assignment_choices(email_queryset, review_req): unavailable_periods = current_unavailable_periods_for_reviewers(team) # reviewers statistics - req_data_for_reviewers = latest_review_requests_for_reviewers(team) + assignment_data_for_reviewers = latest_review_assignments_for_reviewers(team) ranking = [] for e in possible_emails: @@ -835,21 +936,21 @@ def make_assignment_choices(email_queryset, review_req): # stats stats = [] - req_data = req_data_for_reviewers.get(e.person_id, []) + assignment_data = assignment_data_for_reviewers.get(e.person_id, []) - currently_open = sum(1 for d in req_data if d.state in ["requested", "accepted"]) - pages = sum(rd.doc_pages for rd in req_data if rd.state in ["requested", "accepted"]) + currently_open = sum(1 for d in assignment_data if d.state in ["assigned", "accepted"]) + pages = sum(rd.doc_pages for rd in assignment_data if rd.state in ["assigned", "accepted"]) if currently_open > 0: stats.append("currently {count} open, {pages} pages".format(count=currently_open, pages=pages)) - could_have_completed = [d for d in req_data if d.state in ["part-completed", "completed", "no-response"]] + could_have_completed = [d for d in assignment_data if d.state in ["part-completed", "completed", "no-response"]] if could_have_completed: - no_response = len([d for d in req_data if d.state == 'no-response']) + no_response = len([d for d in assignment_data if d.state == 'no-response']) if no_response: stats.append("%s no response" % no_response) - part_completed = len([d for d in req_data if d.state == 'part-completed']) + part_completed = len([d for d in assignment_data if d.state == 'part-completed']) if part_completed: stats.append("%s partially complete" % part_completed) - completed = len([d for d in req_data if d.state == 'completed']) + completed = len([d for d in assignment_data if d.state == 'completed']) if completed: stats.append("%s fully completed" % completed) @@ -870,21 +971,19 @@ def make_assignment_choices(email_queryset, review_req): return [(r["email"].pk, r["label"]) for r in ranking] -def review_requests_needing_reviewer_reminder(remind_date): - reqs_qs = ReviewRequest.objects.filter( - state__in=("requested", "accepted"), +def review_assignments_needing_reviewer_reminder(remind_date): + assignment_qs = ReviewAssignment.objects.filter( + state__in=("assigned", "accepted"), reviewer__person__reviewersettings__remind_days_before_deadline__isnull=False, - reviewer__person__reviewersettings__team=F("team"), - ).exclude( - reviewer=None - ).values_list("pk", "deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct() + reviewer__person__reviewersettings__team=F("review_request__team"), + ).values_list("pk", "review_request__deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct() - req_pks = [] - for r_pk, deadline, remind_days in reqs_qs: + assignment_pks = [] + for a_pk, deadline, remind_days in assignment_qs: if (deadline - remind_date).days == remind_days: - req_pks.append(r_pk) + assignment_pks.append(a_pk) - return ReviewRequest.objects.filter(pk__in=req_pks).select_related("reviewer", "reviewer__person", "state", "team") + return ReviewAssignment.objects.filter(pk__in=assignment_pks).select_related("reviewer", "reviewer__person", "state", "review_request__team") def email_reviewer_reminder(review_request): team = review_request.team @@ -911,24 +1010,24 @@ def email_reviewer_reminder(review_request): "remind_days": remind_days, }) -def review_requests_needing_secretary_reminder(remind_date): - reqs_qs = ReviewRequest.objects.filter( - state__in=("requested", "accepted"), - team__role__person__reviewsecretarysettings__remind_days_before_deadline__isnull=False, - team__role__person__reviewsecretarysettings__team=F("team"), +def review_assignments_needing_secretary_reminder(remind_date): + assignment_qs = ReviewAssignment.objects.filter( + state__in=("assigned", "accepted"), + review_request__team__role__person__reviewsecretarysettings__remind_days_before_deadline__isnull=False, + review_request__team__role__person__reviewsecretarysettings__team=F("review_request__team"), ).exclude( reviewer=None - ).values_list("pk", "deadline", "team__role", "team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct() + ).values_list("pk", "review_request__deadline", "review_request__team__role", "review_request__team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct() - req_pks = {} - for r_pk, deadline, secretary_role_pk, remind_days in reqs_qs: + assignment_pks = {} + for a_pk, deadline, secretary_role_pk, remind_days in assignment_qs: if (deadline - remind_date).days == remind_days: - req_pks[r_pk] = secretary_role_pk + assignment_pks[a_pk] = secretary_role_pk - review_reqs = { r.pk: r for r in ReviewRequest.objects.filter(pk__in=req_pks.keys()).select_related("reviewer", "reviewer__person", "state", "team") } - secretary_roles = { r.pk: r for r in Role.objects.filter(pk__in=req_pks.values()).select_related("email", "person") } + review_assignments = { a.pk: a for a in ReviewAssignment.objects.filter(pk__in=assignment_pks.keys()).select_related("reviewer", "reviewer__person", "state", "review_request__team") } + secretary_roles = { r.pk: r for r in Role.objects.filter(pk__in=assignment_pks.values()).select_related("email", "person") } - return [ (review_reqs[req_pk], secretary_roles[secretary_role_pk]) for req_pk, secretary_role_pk in req_pks.iteritems() ] + return [ (review_assignments[a_pk], secretary_roles[secretary_role_pk]) for a_pk, secretary_role_pk in assignment_pks.iteritems() ] def email_secretary_reminder(review_request, secretary_role): team = review_request.team diff --git a/ietf/static/ietf/css/ietf.css b/ietf/static/ietf/css/ietf.css index ad2a0c9aa..19485abcf 100644 --- a/ietf/static/ietf/css/ietf.css +++ b/ietf/static/ietf/css/ietf.css @@ -451,7 +451,11 @@ table tbody.meta th:nth-child(2), table tbody.meta td:nth-child(2) { width: 9em; } - +table tbody.panel-meta th, table tbody.panel-meta td { border-top: 0; } +table tbody.panel-meta th:first-child, table tbody.panel-meta td:first-child { + text-align: right; + width: 9em; +} td.area-director div { border-bottom: solid #ccc 1px; } diff --git a/ietf/stats/tests.py b/ietf/stats/tests.py index ee7571f4b..13656a723 100644 --- a/ietf/stats/tests.py +++ b/ietf/stats/tests.py @@ -4,6 +4,8 @@ from mock import patch from pyquery import PyQuery from requests import Response +import debug # pyflakes:ignore + from django.urls import reverse as urlreverse from django.contrib.auth.models import User @@ -18,7 +20,7 @@ from ietf.meeting.factories import MeetingFactory from ietf.person.factories import PersonFactory from ietf.person.models import Person, Email from ietf.name.models import FormalLanguageName, DocRelationshipName, CountryName -from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory +from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, ReviewAssignmentFactory from ietf.stats.models import MeetingRegistration, CountryAlias from ietf.stats.utils import get_meeting_registration_data @@ -157,7 +159,8 @@ class StatisticsTests(TestCase): def test_review_stats(self): reviewer = PersonFactory() - review_req = ReviewRequestFactory(reviewer=reviewer.email_set.first()) + review_req = ReviewRequestFactory(state_id='assigned') + ReviewAssignmentFactory(review_request=review_req, state_id='assigned', reviewer=reviewer.email_set.first()) RoleFactory(group=review_req.team,name_id='reviewer',person=reviewer) ReviewerSettingsFactory(team=review_req.team, person=reviewer) PersonFactory(user__username='plain') diff --git a/ietf/stats/views.py b/ietf/stats/views.py index f2848b36f..4eeac6506 100644 --- a/ietf/stats/views.py +++ b/ietf/stats/views.py @@ -19,15 +19,15 @@ from django.utils.text import slugify import debug # pyflakes:ignore -from ietf.review.utils import (extract_review_request_data, - aggregate_raw_period_review_request_stats, - ReviewRequestData, - sum_period_review_request_stats, - sum_raw_review_request_aggregations) +from ietf.review.utils import (extract_review_assignment_data, + aggregate_raw_period_review_assignment_stats, + ReviewAssignmentData, + sum_period_review_assignment_stats, + sum_raw_review_assignment_aggregations) from ietf.submit.models import Submission from ietf.group.models import Role, Group from ietf.person.models import Person -from ietf.name.models import ReviewRequestStateName, ReviewResultName, CountryName, DocRelationshipName +from ietf.name.models import ReviewResultName, CountryName, DocRelationshipName, ReviewAssignmentStateName from ietf.person.name import plain_name from ietf.doc.models import DocAlias, Document, State, DocEvent from ietf.meeting.models import Meeting @@ -1020,7 +1020,7 @@ def review_stats(request, stats_type=None, acronym=None): possible_stats_types = [ ("completion", "Completion status"), ("results", "Review results"), - ("states", "Request states"), + ("states", "Assignment states"), ] if level == "team": @@ -1094,7 +1094,7 @@ def review_stats(request, stats_type=None, acronym=None): query_reviewers = None group_by_objs = { t.pk: t for t in query_teams } - group_by_index = ReviewRequestData._fields.index("team") + group_by_index = ReviewAssignmentData._fields.index("team") elif level == "reviewer": for t in teams: @@ -1105,9 +1105,9 @@ def review_stats(request, stats_type=None, acronym=None): return HttpResponseRedirect(urlreverse(review_stats)) query_reviewers = list(Person.objects.filter( - email__reviewrequest__time__gte=from_time, - email__reviewrequest__time__lte=to_time, - email__reviewrequest__team=reviewers_for_team, + email__reviewassignment__review_request__time__gte=from_time, + email__reviewassignment__review_request__time__lte=to_time, + email__reviewassignment__review_request__team=reviewers_for_team, **reviewer_filter_args.get(t.pk, {}) ).distinct()) query_reviewers.sort(key=lambda p: p.last_name()) @@ -1115,7 +1115,7 @@ def review_stats(request, stats_type=None, acronym=None): query_teams = [t] group_by_objs = { r.pk: r for r in query_reviewers } - group_by_index = ReviewRequestData._fields.index("reviewer") + group_by_index = ReviewAssignmentData._fields.index("reviewer") # now filter and aggregate the data possible_teams = possible_completion_types = possible_results = possible_states = None @@ -1139,22 +1139,23 @@ def review_stats(request, stats_type=None, acronym=None): ) query_teams = [t for t in query_teams if t.acronym in selected_teams] - extracted_data = extract_review_request_data(query_teams, query_reviewers, from_time, to_time) + extracted_data = extract_review_assignment_data(query_teams, query_reviewers, from_time, to_time) - req_time_index = ReviewRequestData._fields.index("req_time") + req_time_index = ReviewAssignmentData._fields.index("req_time") def time_key_fn(t): d = t[req_time_index].date() #d -= datetime.timedelta(days=d.weekday()) # weekly - d -= datetime.timedelta(days=d.day) # monthly + # NOTE: Earlier releases had an off-by-one error here - some stat counts may move a month. + d -= datetime.timedelta(days=d.day-1) # monthly return d found_results = set() found_states = set() aggrs = [] for d, request_data_items in itertools.groupby(extracted_data, key=time_key_fn): - raw_aggr = aggregate_raw_period_review_request_stats(request_data_items, count=count) - aggr = sum_period_review_request_stats(raw_aggr) + raw_aggr = aggregate_raw_period_review_assignment_stats(request_data_items, count=count) + aggr = sum_period_review_assignment_stats(raw_aggr) aggrs.append((d, aggr)) @@ -1164,7 +1165,7 @@ def review_stats(request, stats_type=None, acronym=None): found_states.add(slug) results = ReviewResultName.objects.filter(slug__in=found_results) - states = ReviewRequestStateName.objects.filter(slug__in=found_states) + states = ReviewAssignmentStateName.objects.filter(slug__in=found_states) # choice @@ -1213,7 +1214,7 @@ def review_stats(request, stats_type=None, acronym=None): }]) else: # tabular data - extracted_data = extract_review_request_data(query_teams, query_reviewers, from_time, to_time, ordering=[level]) + extracted_data = extract_review_assignment_data(query_teams, query_reviewers, from_time, to_time, ordering=[level]) data = [] @@ -1221,10 +1222,10 @@ def review_stats(request, stats_type=None, acronym=None): found_states = set() raw_aggrs = [] for group_pk, request_data_items in itertools.groupby(extracted_data, key=lambda t: t[group_by_index]): - raw_aggr = aggregate_raw_period_review_request_stats(request_data_items, count=count) + raw_aggr = aggregate_raw_period_review_assignment_stats(request_data_items, count=count) raw_aggrs.append(raw_aggr) - aggr = sum_period_review_request_stats(raw_aggr) + aggr = sum_period_review_assignment_stats(raw_aggr) # skip zero-valued rows if aggr["open"] == 0 and aggr["completed"] == 0 and aggr["not_completed"] == 0: @@ -1241,12 +1242,12 @@ def review_stats(request, stats_type=None, acronym=None): # add totals row if len(raw_aggrs) > 1: - totals = sum_period_review_request_stats(sum_raw_review_request_aggregations(raw_aggrs)) + totals = sum_period_review_assignment_stats(sum_raw_review_assignment_aggregations(raw_aggrs)) totals["obj"] = "Totals" data.append(totals) results = ReviewResultName.objects.filter(slug__in=found_results) - states = ReviewRequestStateName.objects.filter(slug__in=found_states) + states = ReviewAssignmentStateName.objects.filter(slug__in=found_states) # massage states/results breakdowns for template rendering for aggr in data: diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index d8c4ad95c..884b2db37 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -204,14 +204,14 @@ {% endif %} {% endfor %} - {% if review_requests or can_request_review %} + {% if review_assignments or can_request_review %} Reviews - {% for review_request in review_requests %} - {% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} + {% for review_assignment in review_assignments %} + {% include "doc/review_assignment_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} {% endfor %} {% if no_review_from_teams %} diff --git a/ietf/templates/doc/document_review.html b/ietf/templates/doc/document_review.html index 263ee5cfd..efa13552f 100644 --- a/ietf/templates/doc/document_review.html +++ b/ietf/templates/doc/document_review.html @@ -43,8 +43,8 @@ Other reviews - {% for review_request in other_reviews %} - {% include "doc/review_request_summary.html" with current_doc_name=review_req.doc_id current_rev=review_req.reviewed_rev %} + {% for review_assignment in other_reviews %} + {% include "doc/review_assignment_summary.html" with current_doc_name=review_assignemnt.review_request.doc_id current_rev=review_assignment.reviewed_rev %} {% endfor %} diff --git a/ietf/templates/doc/review/complete_review.html b/ietf/templates/doc/review/complete_review.html index 12dda2b60..cfc586529 100644 --- a/ietf/templates/doc/review/complete_review.html +++ b/ietf/templates/doc/review/complete_review.html @@ -11,14 +11,14 @@ {% block content %} {% origin %}

{% if revising_review %}Revise{% else %}Complete{% endif %} review
- {{ review_req.doc.name }} + {{ assignment.review_request.doc.name }}

-

Review type: {{ review_req.team.acronym }} - {{ review_req.type }} review
-
Requested version for review: {{ review_req.requested_rev|default:"Current" }}
-
Requsted: {{ review_req.time|date:"Y-m-d" }}
-
Reviewer: {{ review_req.reviewer.person.name }}
+
Review type: {{ assignment.review_request.team.acronym }} - {{ assignment.review_request.type }} review
+
Requested version for review: {{ assignment.review_request.requested_rev|default:"Current" }}
+
Requested: {{ assignment.review_request.time|date:"Y-m-d" }}
+
Reviewer: {{ assignment.reviewer.person.name }}

{% if not revising_review %} @@ -37,7 +37,7 @@ {% bootstrap_form form layout="horizontal" %} {% buttons %} - Cancel + Cancel {% endbuttons %} @@ -95,7 +95,7 @@ {% block js %} {% endblock %} diff --git a/ietf/templates/doc/review/mark_reviewer_assignment_no_response.html b/ietf/templates/doc/review/mark_reviewer_assignment_no_response.html new file mode 100644 index 000000000..aa23b3c6a --- /dev/null +++ b/ietf/templates/doc/review/mark_reviewer_assignment_no_response.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2019, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Mark review assignment for {{ assignment.review_request.doc.name }} as No Response{% endblock %} + +{% block content %} + {% origin %} +

No-Response: Review assignment
{{ assignment.review_request.doc.name }}

+ +

Mark review assignment for {{ assignment.reviewer.person }} as No Response

+ +
+ {% csrf_token %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/reject_reviewer_assignment.html b/ietf/templates/doc/review/reject_reviewer_assignment.html index 93380a405..d70ce0770 100644 --- a/ietf/templates/doc/review/reject_reviewer_assignment.html +++ b/ietf/templates/doc/review/reject_reviewer_assignment.html @@ -10,7 +10,7 @@ {% include "doc/review/request_info.html" %} -

{{ review_req.reviewer.person }} is currently assigned to do the review. Do you want to reject this assignment?

+

Do you want to reject this assignment?

{% csrf_token %} diff --git a/ietf/templates/doc/review/request_info.html b/ietf/templates/doc/review/request_info.html index a59c607c0..78205bd5c 100644 --- a/ietf/templates/doc/review/request_info.html +++ b/ietf/templates/doc/review/request_info.html @@ -1,4 +1,4 @@ -{# Copyright The IETF Trust 2017, All Rights Reserved #}{% load origin %}{% origin %} +{# Copyright The IETF Trust 2017, All Rights Reserved #}{% load origin bootstrap3 %}{% origin %} @@ -68,14 +68,20 @@ {% endif %} + {% if doc.time %} - + + + + {% endif %} + + + + @@ -92,115 +98,107 @@ {% endif %} - - - - - - - - - - - - - - - - - - - - - {% if review_req.review and review_req.review.external_url %} - - - - - - {% endif %} - - {% if review_req.reviewed_rev %} - - - - - - {% endif %} - - {% if review_req.result %} - - - - - - {% endif %} - - {% if doc.time %} - - - - - - {% endif %} - - {% if review_req.state_id == "completed" or review_req.state_id == "part-completed" %} - - - - - - {% endif %} -
Other ReviewsDraft last updated{{ doc.time|date:"Y-m-d" }}
Completed reviews - {% for req in review_req.other_completed_requests %} - {% if req.reviewer == review_req.reviewer %}{% endif %} - {{req.team.acronym|capfirst}} {{req.type.name}} review of -{{req.reviewed_rev}} by {{req.reviewer.person.plain_name}} {% if req.reviewed_rev != req.doc.rev %} (diff){% endif %}
- {% if req.reviewer == review_req.reviewer %}{% endif %} + {% for a in review_req.all_completed_assignments_for_doc %} + {{a.review_request.team.acronym|capfirst}} {{a.review_request.type.name}} review of -{{a.reviewed_rev}} by {{a.reviewer.person.plain_name}} {% if a.reviewed_rev != a.review_request.doc.rev %} (diff){% endif %}
{% endfor %}
ReviewState{{ review_req.state.name }} - {% if snapshot %} - Snapshot - {% endif %} -
Reviewer - {% if review_req.reviewer %} - {{ review_req.reviewer.person }} - {% else %} - None assigned yet - {% endif %} - - {% if can_assign_reviewer %} - {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer - {% endif %} - - {% if review_req.reviewer %} - {% if can_reject_reviewer_assignment or can_accept_reviewer_assignment %} -
- {% if review_req.state_id == "requested"%} - Assignment not accepted yet: - {% else %} - Assignment accepted: - {% endif %} - - {% if can_reject_reviewer_assignment %} - Reject - {% endif %} - - {% if can_accept_reviewer_assignment %} - {% csrf_token %} - {% endif %} -
- {% endif %} - {% endif %} -
Review - {% if review_req.review %} - {{ review_req.review.name }} - {% elif review_req.state_id == "requested" or review_req.state_id == "accepted" %} - Not completed yet - {% else %} - Not available - {% endif %} - - {% if can_complete_review %} - {% if review_req.state_id == "requested" or review_req.state_id == "accepted" %}Complete review{% else %}Correct review{% endif %} - {% endif %} -
Posted at - {{ review_req.review.external_url }} -
Reviewed rev.{{ review_req.reviewed_rev }} {% if review_req.reviewed_rev != review_req.doc.rev %}(document currently at {{ review_req.doc.rev }}){% endif %}
Review result{{ review_req.result.name }}
Draft last updated{{ doc.time|date:"Y-m-d" }}
Review completed: - {{ review_req.review_done_time|date:"Y-m-d" }} -
+

Assignments

+ {% if can_assign_reviewer %} +

+ Assign reviewer +

+ {% endif %} + + {% for assignment in assignments %} +
+
+ {{ assignment.reviewer.person }} + {% if assignment.can_reject_reviewer_assignment or assignment.can_accept_reviewer_assignment %} +
+ {% if assignment.state_id == "assigned"%} + Assignment not accepted yet: + {% else %} + Assignment accepted: + {% endif %} + {% if assignment.can_reject_reviewer_assignment %} + Reject + {% endif %} + {% if assignment.can_accept_reviewer_assignment %} +
{% csrf_token %}
+ {% endif %} +
+ {% endif %} +
+
+ + + + + + + + {% if assignment.state_id != "withdrawn" and assignment.state_id != "no-response" and assignment.state_id != "rejected" %} + + + + + {% endif %} + + {% if assignment.review and assignment.review.external_url %} + + + + + {% endif %} + + {% if assignment.reviewed_rev %} + + + + + {% endif %} + + {% if assignment.result %} + + + + + {% endif %} + + + {% if assignment.state_id == "completed" or assignment.state_id == "part-completed" %} + + + + + {% endif %} + +
State{{ assignment.state.name }} + {% if snapshot %} + Snapshot + {% endif %} +
Review + {% if assignment.review %} + {{ assignment.review.name }} + {% elif assignment.state_id == "assigned" or assignment.state_id == "accepted" %} + Not completed yet + {% else %} + Not available + {% endif %} + + {% if assignment.can_complete_review %} + {% if assignment.state_id == "assigned" or assignment.state_id == "accepted" %}Complete review{% else %}Correct review{% endif %} + {% endif %} + + {% if assignment.state_id == "assigned" or assignment.state_id == "accepted" %} + {% if can_assign_reviewer %} + No response + Withdraw + {% endif %} + {% endif %} +
Posted at + {{ assignment.review.external_url }} +
Reviewed rev.{{ assignment.reviewed_rev }} {% if assignment.reviewed_rev != review_req.doc.rev %}(document currently at {{ review_req.doc.rev }}){% endif %}
Review result{{ assignment.result.name }}
Review completed: + {{ assignment.completed_on|date:"Y-m-d" }} +
+
+
+ {% endfor %} diff --git a/ietf/templates/doc/review/withdraw_reviewer_assignment.html b/ietf/templates/doc/review/withdraw_reviewer_assignment.html new file mode 100644 index 000000000..1c2496ff2 --- /dev/null +++ b/ietf/templates/doc/review/withdraw_reviewer_assignment.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2019, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Withdraw review assignment for {{ assignment.review_request.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Withdraw review assignment
{{ assignment.review_request.doc.name }}

+ +

Withdraw review assignment for {{ assignment.reviewer.person }}

+ +
+ {% csrf_token %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review_assignment_summary.html b/ietf/templates/doc/review_assignment_summary.html new file mode 100644 index 000000000..e6e9bcca6 --- /dev/null +++ b/ietf/templates/doc/review_assignment_summary.html @@ -0,0 +1,12 @@ +
+ {% if review_assignment.state_id == "completed" or review_assignment.state_id == "part-completed" %} + + {{ review_assignment.review_request.team.acronym|upper }} {{ review_assignment.review_request.type.name }} Review{% if review_assignment.reviewed_rev and review_assignment.reviewed_rev != current_rev or review_assignment.review_request.doc_id != current_doc_name %} (of {% if review_assignment.review_request.doc_id != current_doc_name %}{{ review_assignment.review_request.doc_id }}{% endif %}-{{ review_assignment.reviewed_rev }}){% endif %}{% if review_assignment.result %}: + {{ review_assignment.result.name }}{% endif %} {% if review_assignment.state_id == "part-completed" %}(partially completed){% endif %} + + {% else %} + + {{ review_assignment.review_request.team.acronym|upper }} {{ review_assignment.review_request.type.name }} Review + - due: {{ review_assignment.review_request.deadline|date:"Y-m-d" }} + {% endif %} +
diff --git a/ietf/templates/doc/review_request_summary.html b/ietf/templates/doc/review_request_summary.html deleted file mode 100644 index e06b8176a..000000000 --- a/ietf/templates/doc/review_request_summary.html +++ /dev/null @@ -1,12 +0,0 @@ -
- {% if review_request.state_id == "completed" or review_request.state_id == "part-completed" %} - - {{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review{% if review_request.reviewed_rev and review_request.reviewed_rev != current_rev or review_request.doc_id != current_doc_name %} (of {% if review_request.doc_id != current_doc_name %}{{ review_request.doc_id }}{% endif %}-{{ review_request.reviewed_rev }}){% endif %}{% if review_request.result %}: - {{ review_request.result.name }}{% endif %} {% if review_request.state_id == "part-completed" %}(partially completed){% endif %} - - {% else %} - - {{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review - - due: {{ review_request.deadline|date:"Y-m-d" }} - {% endif %} -
diff --git a/ietf/templates/group/email_open_review_assignments.html b/ietf/templates/group/email_open_review_assignments.html index 74dcc5b03..4ac55fc46 100644 --- a/ietf/templates/group/email_open_review_assignments.html +++ b/ietf/templates/group/email_open_review_assignments.html @@ -11,7 +11,7 @@

Email summary of assigned review requests for {{ group.acronym }}

- {% if review_requests %} + {% if review_assignments %} {% if closed_review_requests %} - +

Closed review requests

+
@@ -91,9 +134,7 @@ - - @@ -103,27 +144,49 @@ - - + - {% endfor %}
RequestRequested Deadline ClosedReviewer StateResult
{{ r.type }} X{{ r.time|date:"Y-m-d" }} by {{ r.requested_by.plain_name }} {{ r.deadline|date:"Y-m-d" }}{{ r.review_done_time|date:"Y-m-d" }} - {% if r.reviewer %} - {{ r.reviewer.person }} - {% else %} - not yet assigned - {% endif %} - {{ r.request_closed_time|date:"Y-m-d" }} {{ r.state.name }} - {% if r.result %} - {{ r.result.name }} - {% endif %} -
- - {% else %} -

No closed requests found.

+ {% endif %} + + {% if closed_review_assignments %} +

Closed review assignments

+ + + + + + + + + + + + + + + {% for a in closed_review_assignments %} + + + + + + + + + + + {% endfor %} + +
RequestTypeAssignedDeadlineClosedReviewerStateResult
{{ a.review_request.doc.name }}{% if a.review_request.requested_rev %}-{{ a.review_request.requested_rev }}{% endif %}{{ a.review_request.type }}{{ a.assigned_on|date:"Y-m-d" }}{{ a.review_request.deadline|date:"Y-m-d" }}{{ a.completed_on|date:"Y-m-d" }}{{ a.reviewer.person }}{{ a.state }}{% if a.result %}{{ a.result }}{% endif %}
+ + {% endif %} + + {% if not closed_review_requests and not closed_review_assignments %} +
None found
{% endif %} {% endblock %} diff --git a/ietf/templates/iesg/agenda_doc.html b/ietf/templates/iesg/agenda_doc.html index 4095122c5..3a018097e 100644 --- a/ietf/templates/iesg/agenda_doc.html +++ b/ietf/templates/iesg/agenda_doc.html @@ -47,11 +47,11 @@
Consensus
{{ doc.consensus }}
{% endif %} - {% if doc.review_requests %} + {% if doc.review_assignments %}
Reviews
- {% for review_request in doc.review_requests %} - {% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} + {% for review_assignment in doc.review_assignments %} + {% include "doc/review_assignment_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} {% endfor %}
{% endif %} diff --git a/ietf/templates/iesg/agenda_doc.txt b/ietf/templates/iesg/agenda_doc.txt index 49ed89043..c5700d795 100644 --- a/ietf/templates/iesg/agenda_doc.txt +++ b/ietf/templates/iesg/agenda_doc.txt @@ -5,7 +5,7 @@ {% endif %} Token: {{ doc.ad }}{% if doc.iana_review_state %} IANA Review: {{ doc.iana_review_state }}{% endif %}{% if doc.consensus %} Consensus: {{ doc.consensus }}{% endif %}{% if doc.lastcall_expires %} - Last call expires: {{ doc.lastcall_expires|date:"Y-m-d" }}{% endif %}{% if doc.review_requests %} - Reviews: {% for review_request in doc.review_requests %}{% with current_doc_name=doc.name current_rev=doc.rev %}{% if not forloop.first %} {% endif %}{{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review{% if review_request.state_id == "completed" or review_request.state_id == "part-completed" %}{% if review_request.reviewed_rev and review_request.reviewed_rev != current_rev or review_request.doc_id != current_doc_name %} (of {% if review_request.doc_id != current_doc_name %}{{ review_request.doc_id }}{% endif %}-{{ review_request.reviewed_rev }}){% endif %}{% if review_request.result %}: {{ review_request.result.name }}{% endif %} {% if review_request.state_id == "part-completed" %}(partially completed){% endif %}{% else %} - due: {{ review_request.deadline|date:"Y-m-d" }}{% endif %}{% endwith %} + Last call expires: {{ doc.lastcall_expires|date:"Y-m-d" }}{% endif %}{% if doc.review_assignments %} + Reviews: {% for assignment in doc.review_assignments %}{% with current_doc_name=doc.name current_rev=doc.rev %}{% if not forloop.first %} {% endif %}{{ assignment.review_request.team.acronym|upper }} {{ assignment.review_request.type.name }} Review{% if assignment.state_id == "completed" or assignment.state_id == "part-completed" %}{% if assignment.reviewed_rev and assignment.reviewed_rev != current_rev or assignment.review_request.doc_id != current_doc_name %} (of {% if assignment.review_request.doc_id != current_doc_name %}{{ assignment.review_request.doc_id }}{% endif %}-{{ assignment.reviewed_rev }}){% endif %}{% if assignment.result %}: {{ assignment.result.name }}{% endif %} {% if assignment.state_id == "part-completed" %}(partially completed){% endif %}{% else %} - due: {{ assignment.review_request.deadline|date:"Y-m-d" }}{% endif %}{% endwith %} {% endfor %}{% endif %} {% with doc.active_defer_event as defer %}{% if defer %} Was deferred by {{defer.by}} on {{defer.time|date:"Y-m-d"}}{% endif %}{% endwith %} diff --git a/ietf/templates/ietfauth/review_overview.html b/ietf/templates/ietfauth/review_overview.html index 1451f5458..e79259a5b 100644 --- a/ietf/templates/ietfauth/review_overview.html +++ b/ietf/templates/ietfauth/review_overview.html @@ -17,7 +17,7 @@

Assigned reviews

- {% if open_review_requests %} + {% if open_review_assignments %} @@ -30,15 +30,15 @@ - {% for r in open_review_requests %} + {% for r in open_review_assignments %} - - - - - + + + + + @@ -50,9 +50,9 @@ {% endif %} -

Latest closed review requests

+

Latest closed review assignments

- {% if closed_review_requests %} + {% if closed_review_assignments %}
{{ r.doc.name }}{% if r.requested_rev %}{{ r.requested_rev }}{% else %}Current{% endif %}{{r.doc.rev}}{{ r.team.acronym }}{{ r.type.name }}{{ r.review_request.doc.name }}{% if r.review_request.requested_rev %}{{ r.review_request.requested_rev }}{% else %}Current{% endif %}{{r.review_request.doc.rev}}{{ r.review_request.team.acronym }}{{ r.review_request.type.name }} - {{ r.deadline|date:"Y-m-d" }} + {{ r.review_request.deadline|date:"Y-m-d" }} {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %}
@@ -66,14 +66,14 @@ - {% for r in closed_review_requests %} + {% for r in closed_review_assignments %} - - - - + + + + diff --git a/ietf/templates/review/completed_review.txt b/ietf/templates/review/completed_review.txt index ca4eb28fd..bdbe321ca 100644 --- a/ietf/templates/review/completed_review.txt +++ b/ietf/templates/review/completed_review.txt @@ -1,9 +1,8 @@ -{% load ietf_filters %}{% autoescape off %}{% filter maybewordwrap:80 %}{% if review_req.state_id == "part-completed" %} -Review is partially done. Another review request has been registered for -completing it. +{% load ietf_filters %}{% autoescape off %}{% filter maybewordwrap:80 %}{% if assignment.state_id == "part-completed" %} +Review is partially done. Another assignment may be needed to complete it. -{% endif %}Reviewer: {{ review_req.reviewer.person }} -Review result: {{ review_req.result.name }} +{% endif %}Reviewer: {{ assignment.reviewer.person }} +Review result: {{ assignment.result.name }} {{ content }} {% endfilter %}{% endautoescape %} diff --git a/ietf/templates/review/notify_ad.txt b/ietf/templates/review/notify_ad.txt index 9e5196a9a..7dc585b2d 100644 --- a/ietf/templates/review/notify_ad.txt +++ b/ietf/templates/review/notify_ad.txt @@ -1,13 +1,13 @@ {% load ietf_filters %}{% autoescape off %}From: {{settings.DEFAULT_FROM_EMAIL}} To: {{to}}{% if cc %} Cc: {{cc}}{% endif %} -Subject: "{{review_req.result}}" review submitted for {{review_req.doc}}{% if review_req.reviewed_rev %}-{{review_req.reviewed_rev}}{% endif %} +Subject: "{{assignment.result}}" review submitted for {{assignment.review_request.doc}}{% if assignment.reviewed_rev %}-{{assignment.reviewed_rev}}{% endif %} -{{review_req.reviewer.person.name}} has submitted a "{{review_req.result}}" review result for {{review_req.doc}}{% if review_req.reviewed_rev %}-{{review_req.reviewed_rev}}{% endif %}. +{{assignment.reviewer.person}} has submitted a "{{assignment.result}}" review result for {{assignment.review_request.doc}}{% if assignment.reviewed_rev %}-{{assignment.reviewed_rev}}{% endif %}. -The review is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.review.name %} +The review is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=assignment.review.name %} -The document is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.doc.name %} +The document is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=assignment.review_request.doc.name %} This message was sent because {% if explicit_request %}the reviewer indicated it should be on the review completion form{% else %}the review team settings indicated it should be{% endif %}. diff --git a/ietf/templates/review/partially_completed_review.txt b/ietf/templates/review/partially_completed_review.txt index 6d518357c..8316ee7fd 100644 --- a/ietf/templates/review/partially_completed_review.txt +++ b/ietf/templates/review/partially_completed_review.txt @@ -1,10 +1,7 @@ {% autoescape off %}Review was partially completed by {{ by }}. -{% if new_review_req_url %} -A new review request has been added for completing the review: - -{{ new_review_req_url }} -{% else %} -Found {{ existing_open_reqs|length }} open review request{{ existing_open_reqs|pluralize }} on the document so a new -review request has not been added. +A new reviewer may need to be assigned to get a complete review. +{% if existing_assignments %} +The following are already assigned to review this document:{% for assignment in existing_assignments %} +{{ assignment.reviewer.name }} : {{ assignment.state }}{% endfor %} {% endif %}{% endautoescape %} diff --git a/ietf/templates/submit/tool_instructions.html b/ietf/templates/submit/tool_instructions.html index df4c1a977..0b31b9331 100644 --- a/ietf/templates/submit/tool_instructions.html +++ b/ietf/templates/submit/tool_instructions.html @@ -12,7 +12,28 @@

This page will explain the purpose and content of each screen in the I-D Submission Tool, and the actions that result by clicking the form buttons on each screen. - The specification for this tool can be found in RFC 4228. +

+ +

+ Internet-Drafts are working documents of the Internet Engineering + Task Force (IETF), its areas, and its working groups. Note that other + groups may also distribute working documents as Internet-Drafts. +

+ +

+ Internet-Drafts are draft documents, and are valid for a maximum of + six months. They may be updated, replaced, or obsoleted by other + documents at any time. +

+ +

+ The list of current Internet-Drafts can be accessed at + https://www.ietf.org/ietf/1id-abstracts.txt. +

+ +

+ An API for automated draft submission is available as an alternative to this webpage at + https://datatracker.ietf.org/api/submit/.

Upload screen

@@ -20,6 +41,17 @@ The Upload screen is the first screen that a user will see when he or she starts the I-D submission process. A user can submit four different formats of an I-D, plain text, XML, PDF, and postscript, at the same time. Failure to submit at least one of a plain-text or xml version will cause an error, and an error screen will be displayed.

+

+ By submitting your I-D, you are granting some rights to the IETF Trust. Before you submit your I-D, + review the information on the NOTE WELL tab and BCP 78, + "Rights Contributors Provide to the IETF Trust". +

+ +

+ Before you submit your I-D, it is recommended that you check it for nits + using the idnits tool. +

+

Form buttons and resulting actions:

@@ -133,4 +165,9 @@

Please send problem reports to ietf-action@ietf.org.

+ +

+ The specification for this tool can be found in RFC 4228. +

+ {% endblock %} diff --git a/ietf/templates/submit/upload_submission.html b/ietf/templates/submit/upload_submission.html index 62b3fc734..5fca19e10 100644 --- a/ietf/templates/submit/upload_submission.html +++ b/ietf/templates/submit/upload_submission.html @@ -12,27 +12,14 @@

WARNING: currently in draft submission blackout period

{% endif %} +

This page is used to submit Internet-Drafts to the Internet-Draft repository.

+ {% if form.cutoff_warning %}
{{ form.cutoff_warning|safe }}
{% endif %} -

This page is used to submit Internet-Drafts to the - Internet-Draft repository. The list of current Internet-Drafts can be - accessed at https://www.ietf.org/ietf/1id-abstracts.txt. - An API for automated draft submission is available at - https://datatracker.ietf.org/api/submit/. -

- -

Internet-Drafts are working documents of the Internet Engineering - Task Force (IETF), its areas, and its working groups. Note that other - groups may also distribute working documents as Internet-Drafts.

- -

Internet-Drafts are draft documents, and are valid for a maximum of - six months. They may be updated, replaced, or obsoleted by other - documents at any time.

- {% if not form.shutdown or user|has_role:"Secretariat" %}

If you run into problems when submitting an Internet-Draft using this and the following pages, you may alternatively submit @@ -42,7 +29,13 @@

- Before you submit your draft, it is recommended that you check it for nits + By submitting your I-D, you are granting some rights to the IETF Trust. Before you submit your I-D, + review the information on the NOTE WELL tab and BCP 78, + "Rights Contributors Provide to the IETF Trust". +

+ +

+ Before you submit your I-D, it is recommended that you check it for nits using the idnits tool.

{{ r.doc.name }}{{r.reviewed_rev|default:"See review"}}{% if r.requested_rev %}{% if r.requested_rev != r.reviewed_rev %}({{ r.requested_rev }} requested){% endif %}{% endif %}{{ r.team.acronym }}{{ r.type.name }}{{ r.review_request.doc.name }}{{r.reviewed_rev|default:"See review"}}{% if r.review_request.requested_rev %}{% if r.review_request.requested_rev != r.reviewed_rev %}({{ r.review_request.requested_rev }} requested){% endif %}{% endif %}{{ r.review_request.team.acronym }}{{ r.review_request.type.name }} - {{ r.deadline|date:"Y-m-d" }} + {{ r.review_request.deadline|date:"Y-m-d" }} {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %} {{ r.state.name }}