diff --git a/ietf/bin/send-review-reminders b/ietf/bin/send-review-reminders index eb21325fa..36eb5a235 100755 --- a/ietf/bin/send-review-reminders +++ b/ietf/bin/send-review-reminders @@ -19,19 +19,17 @@ django.setup() import datetime from ietf.review.utils import ( - review_assignments_needing_reviewer_reminder, email_reviewer_reminder, - review_assignments_needing_secretary_reminder, email_secretary_reminder, + review_requests_needing_reviewer_reminder, email_reviewer_reminder, + review_requests_needing_secretary_reminder, email_secretary_reminder, ) today = datetime.date.today() -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 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, 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)) +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)) diff --git a/ietf/dbtemplate/migrations/0003_adjust_review_templates.py b/ietf/dbtemplate/migrations/0003_adjust_review_templates.py deleted file mode 100644 index 5a5ffc48b..000000000 --- a/ietf/dbtemplate/migrations/0003_adjust_review_templates.py +++ /dev/null @@ -1,118 +0,0 @@ -# -*- 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 deleted file mode 100644 index 0c4463a74..000000000 --- a/ietf/dbtemplate/migrations/0004_adjust_assignment_email_summary_templates.py +++ /dev/null @@ -1,289 +0,0 @@ -# -*- 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 d61ccb191..55285b49d 100644 --- a/ietf/doc/admin.py +++ b/ietf/doc/admin.py @@ -5,8 +5,7 @@ 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, - ReviewAssignmentDocEvent ) + AddedMessageEvent, SubmissionDocEvent, DeletedEvent, EditedAuthorsDocEvent, DocumentURL) class StateTypeAdmin(admin.ModelAdmin): @@ -144,7 +143,6 @@ 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 f5f669181..10d866f7b 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -209,12 +209,6 @@ 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 deleted file mode 100644 index 63df9a962..000000000 --- a/ietf/doc/migrations/0011_reviewassignmentdocevent.py +++ /dev/null @@ -1,28 +0,0 @@ -# -*- 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 484122922..47d0fa81f 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, ReviewAssignmentStateName, FormalLanguageName, + DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName, FormalLanguageName, DocUrlTagName) from ietf.person.models import Email, Person from ietf.person.utils import get_active_ads @@ -1149,10 +1149,6 @@ 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 64fd71a15..62f928ced 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, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL) + ReviewRequestDocEvent, EditedAuthorsDocEvent, DocumentURL) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -652,32 +652,3 @@ 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 34de0d551..24b229243 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, ReviewFactory -from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewAssignmentDocEvent +from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory +from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewRequestDocEvent 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, ReviewAssignmentStateName +from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName from ietf.person.models import Email, Person -from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory +from ietf.review.factories import ReviewRequestFactory 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,8 +60,7 @@ 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') - 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') + 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)) url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) login_testing_unauthorized(self, "ad", url) @@ -136,8 +135,7 @@ 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='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') + 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)) # move the review request to a doubly-replaced document to # check we can fish it out @@ -152,15 +150,13 @@ 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='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') + 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)) url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -182,8 +178,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='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()) + 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)) + close_url = urlreverse('ietf.doc.views_review.close_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -291,8 +287,7 @@ class ReviewTests(TestCase): settings, _ = ReviewerSettings.objects.get_or_create(team=team, person=reviewers[2]) settings.min_interval = 30 settings.save() - 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) + 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()) 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) @@ -319,25 +314,17 @@ 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 - req = ReviewRequestFactory( + ReviewRequest.objects.create( time=datetime.datetime.now() - datetime.timedelta(days=100), requested_by=Person.objects.get(name="(System)"), doc=doc, - type_id='early', + type=ReviewTypeName.objects.get(slug="early"), team=review_req.team, - state_id='assigned', - requested_rev="01", - deadline=datetime.date.today() - datetime.timedelta(days=80), - ) - ReviewAssignmentFactory( - review_request = req, - state_id='completed', + state=ReviewRequestStateName.objects.get(slug="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), + deadline=datetime.date.today() - datetime.timedelta(days=80), + reviewer=reviewer_email, ) reviewer_settings = ReviewerSettings.objects.get(person__email=reviewer_email, team=review_req.team) @@ -400,25 +387,37 @@ class ReviewTests(TestCase): self.assertEqual(r.status_code, 302) review_req = reload_db_objects(review_req) - 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(review_req.state_id, "requested") + self.assertEqual(review_req.reviewer, reviewer) 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='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()) + 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)) url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) - username = assignment.reviewer.person.user.username + username = review_req.reviewer.person.user.username self.client.login(username=username, password=username + "+password") r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -429,22 +428,21 @@ class ReviewTests(TestCase): r = self.client.post(url, { "action": "accept" }) self.assertEqual(r.status_code, 302) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "accepted") + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.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='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') + 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)) - reject_url = urlreverse('ietf.doc.views_review.reject_reviewer_assignment', kwargs={ "name": doc.name, "assignment_id": assignment.pk }) + reject_url = urlreverse('ietf.doc.views_review.reject_reviewer_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk }) # follow link - req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": assignment.review_request.pk }) + req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) self.client.login(username="reviewsecretary", password="reviewsecretary+password") r = self.client.get(req_url) self.assertEqual(r.status_code, 200) @@ -455,18 +453,19 @@ 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(assignment.reviewer.person) in unicontent(r)) + self.assertTrue(unicode(review_req.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) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "rejected") + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.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")) @@ -510,8 +509,7 @@ 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='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') + 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)) # test URL construction query_urls = ietf.review.mailarch.construct_query_urls(review_req) @@ -527,7 +525,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, "assignment_id": assignment.pk }) + url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "request_id": review_req.pk }) login_testing_unauthorized(self, "reviewsecretary", url) r = self.client.get(url) @@ -557,7 +555,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, "assignment_id": assignment.pk }) + url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "request_id": review_req.pk }) r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -574,19 +572,18 @@ 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='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()) + 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)) 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, "assignment_id": review_req.pk }) + url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk }) - return assignment, url + return review_req, url def test_complete_review_upload_content(self): - assignment, url = self.setup_complete_review_test() + review_req, url = self.setup_complete_review_test() - login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) # get r = self.client.get(url) @@ -614,9 +611,9 @@ class ReviewTests(TestCase): test_file.name = "unnamed" r = self.client.post(url, data={ - "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, + "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, "review_submission": "upload", "review_content": "", "review_url": "", @@ -624,25 +621,25 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - 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) + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + self.assertEqual(review_req.result_id, "ready") + self.assertEqual(review_req.reviewed_rev, review_req.doc.rev) + self.assertTrue(review_req.team.acronym.lower() in review_req.review.name) + self.assertTrue(review_req.doc.rev in review_req.review.name) - with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: self.assertEqual(f.read(), "This is a review\nwith two lines") self.assertEqual(len(outbox), 1) - 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.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(settings.MAILING_LIST_ARCHIVE_URL in assignment.review.external_url) + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url) # Check that the review has the reviewer as author - self.assertEqual(assignment.reviewer, assignment.review.documentauthor_set.first().email) + self.assertEqual(review_req.reviewer, review_req.review.documentauthor_set.first().email) # Check that we have a copy of the outgoing message msgid = outbox[0]["Message-ID"] @@ -653,25 +650,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": assignment.review.name }) + url = urlreverse('ietf.doc.views_doc.document_main', kwargs={ "name": review_req.review.name }) r = self.client.get(url) self.assertEqual(r.status_code, 200) content = unicontent(r) - self.assertTrue("{} Review".format(assignment.review_request.type.name) in content) + self.assertTrue("{} Review".format(review_req.type.name) in content) self.assertTrue("This is a review" in content) def test_complete_review_enter_content(self): - assignment, url = self.setup_complete_review_test() + review_req, url = self.setup_complete_review_test() - login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "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, + "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, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -679,33 +676,32 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "completed") - self.assertNotEqual(assignment.completed_on, None) + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") - with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: self.assertEqual(f.read(), "This is a review\nwith two lines") self.assertEqual(len(outbox), 1) - self.assertTrue(assignment.review_request.team.list_email in outbox[0]["To"]) + self.assertTrue(review_req.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 assignment.review.external_url) + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url) def test_complete_notify_ad_because_team_settings(self): - assignment, url = self.setup_complete_review_test() - assignment.review_request.team.reviewteamsettings.notify_ad_when.add(ReviewResultName.objects.get(slug='issues')) + review_req, url = self.setup_complete_review_test() + review_req.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 - 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) + 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) empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=assignment.review_request.team, slug="issues").pk, + "result": ReviewResultName.objects.get(reviewteamsettings_review_results_set__group=review_req.team, slug="issues").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, - "reviewed_rev": assignment.review_request.doc.rev, + "reviewed_rev": review_req.doc.rev, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -718,17 +714,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): - 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) + 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) empty_outbox() r = self.client.post(url, data={ - "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, + "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, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -750,16 +746,16 @@ class ReviewTests(TestCase): mock.return_value = response # Run the test - assignment, url = self.setup_complete_review_test() + review_req, url = self.setup_complete_review_test() - login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "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, + "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, "review_submission": "link", "review_content": response.content, "review_url": "http://example.com/testreview/", @@ -767,27 +763,27 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "completed") + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") - with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, review_req.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 assignment.review.external_url) + self.assertTrue("http://example.com" in review_req.review.external_url) def test_partially_complete_review(self): - assignment, url = self.setup_complete_review_test() + review_req, url = self.setup_complete_review_test() - login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) # partially complete empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(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, + "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, "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", }) @@ -795,15 +791,16 @@ class ReviewTests(TestCase): - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "part-completed") - self.assertTrue(assignment.review_request.doc.rev in assignment.review.name) + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "part-completed") + self.assertTrue(review_req.doc.rev in review_req.review.name) self.assertEqual(len(outbox), 2) self.assertTrue("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(assignment.review_request.team.list_email in outbox[1]["To"]) + self.assertTrue(review_req.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) @@ -813,26 +810,30 @@ 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 = assignment.review + first_review = review_req.review + first_reviewer = review_req.reviewer # complete - assignment = assignment.review_request.reviewassignment_set.create(state_id="assigned", reviewer=assignment.reviewer) + review_req = ReviewRequest.objects.get(state="requested", doc=review_req.doc, team=review_req.team) + self.assertEqual(review_req.reviewer, None) + review_req.reviewer = first_reviewer # same reviewer, so we can test uniquification + review_req.save() - url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": assignment.review_request.doc.name, "assignment_id": assignment.pk }) + url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk }) r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(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, + "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, "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) - 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 + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + self.assertTrue(review_req.doc.rev in review_req.review.name) + second_review = review_req.review self.assertTrue(first_review.name != second_review.name) self.assertTrue(second_review.name.endswith("-2")) # uniquified @@ -844,18 +845,18 @@ class ReviewTests(TestCase): def test_revise_review_enter_content(self): - assignment, url = self.setup_complete_review_test() - assignment.state = ReviewAssignmentStateName.objects.get(slug="no-response") - assignment.save() + review_req, url = self.setup_complete_review_test() + review_req.state = ReviewRequestStateName.objects.get(slug="no-response") + review_req.save() - login_testing_unauthorized(self, assignment.reviewer.person.user.username, url) + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) empty_outbox() r = self.client.post(url, data={ - "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, + "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, "review_submission": "enter", "review_content": "This is a review\nwith two lines", "review_url": "", @@ -865,12 +866,12 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.state_id, "completed") - event = ReviewAssignmentDocEvent.objects.get(type="closed_review_request", review_assignment=assignment) + 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) self.assertEqual(event.time, datetime.datetime(2012, 12, 24, 12, 13, 14)) - with open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f: + with open(os.path.join(self.review_subdir, review_req.review.name + ".txt")) as f: self.assertEqual(f.read(), "This is a review\nwith two lines") self.assertEqual(len(outbox), 0) @@ -878,9 +879,9 @@ class ReviewTests(TestCase): # revise again empty_outbox() r = self.client.post(url, data={ - "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, + "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, "review_submission": "enter", "review_content": "This is a revised review", "review_url": "", @@ -890,9 +891,9 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - assignment = reload_db_objects(assignment) - self.assertEqual(assignment.review.rev, "01") - event = ReviewAssignmentDocEvent.objects.get(type="closed_review_request", review_assignment=assignment) + 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) self.assertEqual(event.time, datetime.datetime(2013, 12, 24, 11, 11, 11)) self.assertEqual(len(outbox), 0) @@ -902,8 +903,7 @@ 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='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') + 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)) url = urlreverse('ietf.doc.views_review.edit_comment', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -924,8 +924,7 @@ 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,deadline=datetime.datetime.now()+datetime.timedelta(days=20)) - ReviewAssignmentFactory(review_request = review_req, reviewer = rev_role.person.email_set.first(), state_id='accepted') + 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)) url = urlreverse('ietf.doc.views_review.edit_deadline', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -943,34 +942,3 @@ 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 23b217c50..d3cba6ffa 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -7,11 +7,9 @@ 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]+)/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]+)/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]+)/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 31b365e27..6b61d73db 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=["assigned","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=["requested","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 e9597a506..e4166edd5 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 ReviewAssignment -from ietf.review.utils import can_request_review_of_doc, review_assignments_to_list_for_docs +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.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_assignments = review_assignments_to_list_for_docs([doc]).get(doc.pk, []) + review_requests = review_requests_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_assignments=review_assignments, + review_requests=review_requests, 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_assignment = ReviewAssignment.objects.filter(review=doc.name).first() + review_req = ReviewRequest.objects.filter(review=doc.name).first() other_reviews = [] - 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] + 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] 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_assignment.review_request, + review_req=review_req, other_reviews=other_reviews, )) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index fb5a55039..9103a65c1 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, ReviewAssignmentDocEvent, DocumentAuthor) -from ietf.name.models import ReviewRequestStateName, ReviewAssignmentStateName, ReviewResultName, DocTypeName -from ietf.review.models import ReviewRequest, ReviewAssignment + LastCallDocEvent, ReviewRequestDocEvent, DocumentAuthor) +from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName +from ietf.review.models import ReviewRequest 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_assignment_change, email_review_request_change, - close_review_request_states, - close_review_request, setup_reviewer_field) + email_review_request_change, make_new_review_request_from_existing, + 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,50 +184,47 @@ 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", "assigned"] + can_close_request = (review_req.state_id in ["requested", "accepted"] and (is_authorized_in_doc_stream(request.user, doc) or can_manage_request)) - can_assign_reviewer = (review_req.state_id in ["requested", "assigned"] + can_assign_reviewer = (review_req.state_id in ["requested", "accepted"] 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 - assignments = review_req.reviewassignment_set.all() - for assignment in assignments: - assignment.is_reviewer = user_is_person(request.user, assignment.reviewer.person) + 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() - 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, }) @@ -248,7 +245,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", "assigned"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) can_request = is_authorized_in_doc_stream(request.user, doc) can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) @@ -268,13 +265,12 @@ 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(label="Assign Additional Reviewer", empty_label="(None)", required=False) + reviewer = PersonEmailChoiceField(empty_label="(None)", required=False) add_skip = forms.BooleanField(label='Skip next time', required=False) def __init__(self, review_req, *args, **kwargs): @@ -285,7 +281,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", "assigned"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) if not can_manage_review_requests_for_team(request.user, review_req.team): return HttpResponseForbidden("You do not have permission to perform this action") @@ -304,7 +300,6 @@ 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, }) @@ -312,15 +307,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, assignment_id): +def reject_reviewer_assignment(request, name, request_id): doc = get_object_or_404(Document, name=name) - review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) - if not review_assignment.reviewer: - return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) + 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_assignment.reviewer.person) - can_manage_request = can_manage_review_requests_for_team(request.user, review_assignment.review_request.team) + 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) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") @@ -328,119 +323,47 @@ def reject_reviewer_assignment(request, name, assignment_id): if request.method == "POST" and request.POST.get("action") == "reject": form = RejectReviewerAssignmentForm(request.POST) if form.is_valid(): - # reject the assignment - review_assignment.state = ReviewAssignmentStateName.objects.get(slug="rejected") - review_assignment.save() + # reject the request + review_req.state = ReviewRequestStateName.objects.get(slug="rejected") + review_req.save() - ReviewAssignmentDocEvent.objects.create( + ReviewRequestDocEvent.objects.create( type="closed_review_request", - doc=review_assignment.review_request.doc, - rev=review_assignment.review_request.doc.rev, + doc=review_req.doc, + rev=review_req.doc.rev, by=request.user.person, desc="Assignment of request for {} review by {} to {} was rejected".format( - review_assignment.review_request.type.name, - review_assignment.review_request.team.acronym.upper(), - review_assignment.reviewer.person, + review_req.type.name, + review_req.team.acronym.upper(), + review_req.reviewer.person, ), - review_assignment=review_assignment, - state=review_assignment.state, + review_request=review_req, + state=review_req.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_assignment_change(request, review_assignment, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) + 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) - return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) + return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) else: form = RejectReviewerAssignmentForm() return render(request, 'doc/review/reject_reviewer_assignment.html', { 'doc': doc, - 'review_req': review_assignment.review_request, - 'assignments': review_assignment.review_request.reviewassignment_set.all(), + 'review_req': review_req, '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=ReviewAssignmentStateName.objects.filter(slug__in=("completed", "part-completed")).order_by("-order"), widget=forms.RadioSelect, initial="completed") + state = forms.ModelChoiceField(queryset=ReviewRequestStateName.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 = [ @@ -458,16 +381,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, assignment, is_reviewer, *args, **kwargs): - self.assignment = assignment + def __init__(self, review_req, is_reviewer, *args, **kwargs): + self.review_req = review_req super(CompleteReviewForm, self).__init__(*args, **kwargs) - doc = self.assignment.review_request.doc + doc = self.review_req.doc known_revisions = NewRevisionDocEvent.objects.filter(doc=doc).order_by("time", "id").values_list("rev", "time", flat=False) - revising_review = assignment.state_id not in ["assigned", "accepted"] + revising_review = review_req.state_id not in ["requested", "accepted"] if not revising_review: self.fields["state"].choices = [ @@ -479,7 +402,7 @@ class CompleteReviewForm(forms.Form): reviewed_rev_class = [] for r in known_revisions: last_version = r[0] - if r[1] < assignment.review_request.time: + if r[1] < review_req.time: kwargs["initial"]["reviewed_rev"] = r[0] reviewed_rev_class.append('reviewer-doc-past') else: @@ -503,13 +426,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=assignment.review_request.team) + self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings_review_results_set__group=review_req.team) def format_submission_choice(label): if revising_review: label = label.replace(" (automatically posts to {mailing_list})", "") - return label.format(mailing_list=assignment.review_request.team.list_email or "[error: team has no mailing list set]") + return label.format(mailing_list=review_req.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] @@ -520,7 +443,7 @@ class CompleteReviewForm(forms.Form): del self.fields["completion_time"] def clean_reviewed_rev(self): - return clean_doc_revision(self.assignment.review_request.doc, self.cleaned_data.get("reviewed_rev")) + return clean_doc_revision(self.review_req.doc, self.cleaned_data.get("reviewed_rev")) def clean_review_content(self): return self.cleaned_data["review_content"].replace("\r", "") @@ -538,7 +461,7 @@ class CompleteReviewForm(forms.Form): return url def clean(self): - if "@" in self.assignment.reviewer.person.ascii: + if "@" in self.review_req.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): @@ -554,37 +477,40 @@ class CompleteReviewForm(forms.Form): require_field("review_url") @login_required -def complete_review(request, name, assignment_id): +def complete_review(request, name, request_id): doc = get_object_or_404(Document, name=name) - assignment = get_object_or_404(ReviewAssignment, pk=assignment_id) + review_req = get_object_or_404(ReviewRequest, pk=request_id) - revising_review = assignment.state_id not in ["assigned", "accepted"] + revising_review = review_req.state_id not in ["requested", "accepted"] - 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 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) 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 = assignment.review_request) + (to, cc) = gather_address_lists('review_completed',review_req = review_req) if request.method == "POST": - form = CompleteReviewForm(assignment, is_reviewer, + form = CompleteReviewForm(review_req, is_reviewer, request.POST, request.FILES) if form.is_valid(): review_submission = form.cleaned_data['review_submission'] - review = assignment.review + review = review_req.review if not review: # create review doc for i in range(1, 100): name_components = [ "review", - strip_prefix(assignment.review_request.doc.name, "draft-"), + strip_prefix(review_req.doc.name, "draft-"), form.cleaned_data["reviewed_rev"], - assignment.review_request.team.acronym, - assignment.review_request.type.slug, - xslugify(assignment.reviewer.person.ascii_parts()[3]), + review_req.team.acronym, + review_req.type.slug, + xslugify(review_req.reviewer.person.ascii_parts()[3]), datetime.date.today().isoformat(), ] if i > 1: @@ -597,10 +523,10 @@ def complete_review(request, name, assignment_id): break review.type = DocTypeName.objects.get(slug="review") - review.group = assignment.review_request.team + review.group = review_req.team review.rev = "00" if not review.rev else "{:02}".format(int(review.rev) + 1) - review.title = "{} Review of {}-{}".format(assignment.review_request.type.name, assignment.review_request.doc.name, form.cleaned_data["reviewed_rev"]) + review.title = "{} Review of {}-{}".format(review_req.type.name, review_req.doc.name, form.cleaned_data["reviewed_rev"]) review.time = datetime.datetime.now() if review_submission == "link": review.external_url = form.cleaned_data['review_url'] @@ -628,55 +554,63 @@ def complete_review(request, name, assignment_id): with open(filename, 'wb') as destination: destination.write(encoded_content) - 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 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() - # 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() - - need_to_email_review = review_submission != "link" and assignment.review_request.team.list_email and not revising_review + need_to_email_review = review_submission != "link" and review_req.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, + 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." - 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) + 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.doc = assignment.review_request.doc - close_event.rev = assignment.review_request.doc.rev + 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) + + close_event.doc = review_req.doc + close_event.rev = review_req.doc.rev close_event.by = request.user.person close_event.desc = desc - close_event.state = assignment.state + close_event.state = review_req.state close_event.time = completion_datetime close_event.save() - 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")) + 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")) - subject = "Review of {}-{} completed partially".format(assignment.review_request.doc.name, assignment.reviewed_rev) + 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) msg = render_to_string("review/partially_completed_review.txt", { - "existing_assignments": existing_assignments, + "new_review_req_url": new_review_req_url, + "existing_open_reqs": existing_open_reqs, "by": request.user.person, }) - email_review_assignment_change(request, assignment, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False, notify_requested_by=False) + email_review_request_change(request, review_req, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False, notify_requested_by=False) - role = request.user.person.role_set.filter(group=assignment.review_request.team,name='reviewer').first() + role = request.user.person.role_set.filter(group=review_req.team,name='reviewer').first() if role and role.email.active: author_email = role.email frm = role.formatted_email() @@ -687,10 +621,10 @@ def complete_review(request, name, assignment_id): if need_to_email_review: # email the review - 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) + 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) msg = Message.objects.create( by=request.user.person, subject=subject, @@ -698,26 +632,26 @@ def complete_review(request, name, assignment_id): to=", ".join(to), cc=form.cleaned_data["cc"], body = render_to_string("review/completed_review.txt", { - "assignment": assignment, + "review_req": review_req, "content": encoded_content.decode("utf-8"), }), ) msg.related_groups.add(*related_groups) - msg.related_docs.add(assignment.review_request.doc) + msg.related_docs.add(review_req.doc) msg = send_mail_message(request, msg) - list_name = mailarch.list_name_from_email(assignment.review_request.team.list_email) + list_name = mailarch.list_name_from_email(review_req.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 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() + 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() msg_txt = render_to_string("review/notify_ad.txt", { "to": to, "cc": cc, - "assignment": assignment, + "review_req": review_req, "settings": settings, "explicit_request": form.cleaned_data['email_ad'], }) @@ -726,41 +660,42 @@ def complete_review(request, name, assignment_id): msg.save() send_mail_message(request, msg) - return redirect("ietf.doc.views_doc.document_main", name=assignment.review.name) + return redirect("ietf.doc.views_doc.document_main", name=review_req.review.name) else: initial={ - "reviewed_rev": assignment.reviewed_rev, - "result": assignment.result_id, + "reviewed_rev": review_req.reviewed_rev, + "result": review_req.result_id, "cc": ", ".join(cc), } try: - 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()}) + 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()}) except TemplateDoesNotExist: pass - form = CompleteReviewForm(assignment, is_reviewer, initial=initial) + form = CompleteReviewForm(review_req, is_reviewer, initial=initial) - mail_archive_query_urls = mailarch.construct_query_urls(assignment.review_request) + mail_archive_query_urls = mailarch.construct_query_urls(review_req) return render(request, 'doc/review/complete_review.html', { 'doc': doc, - 'assignment': assignment, + 'review_req': review_req, 'form': form, 'mail_archive_query_urls': mail_archive_query_urls, 'revising_review': revising_review, }) -def search_mail_archive(request, name, assignment_id): - assignment = get_object_or_404(ReviewAssignment, pk=assignment_id) +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) - 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) + 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) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") - res = mailarch.construct_query_urls(assignment.review_request, query=request.GET.get("query")) + res = mailarch.construct_query_urls(review_req, query=request.GET.get("query")) if not res: return JsonResponse({ "error": "Couldn't do lookup in mail archive - don't know where to look"}) @@ -825,7 +760,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.requested_rev) + subject = "Deadline changed: {} {} review of {}-{}".format(review_req.team.acronym.capitalize(),review_req.type.name.lower(), review_req.doc.name, review_req.reviewed_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 635312263..1cbc56ee8 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -239,6 +239,8 @@ 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 021172a98..92bed8aab 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -9,27 +9,26 @@ 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 Person -from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings +from ietf.person.models import Email, Person +from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings from ietf.review.utils import ( suggested_review_requests_for_team, - review_assignments_needing_reviewer_reminder, email_reviewer_reminder, - review_assignments_needing_secretary_reminder, email_secretary_reminder, + review_requests_needing_reviewer_reminder, email_reviewer_reminder, + review_requests_needing_secretary_reminder, email_secretary_reminder, reviewer_rotation_list, ) -from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName +from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName 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, ReviewAssignmentFactory +from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory class ReviewTests(TestCase): def test_review_requests(self): - review_req = ReviewRequestFactory(state_id='assigned') - assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned', reviewer=EmailFactory(), assigned_on = review_req.time) + review_req = ReviewRequestFactory(reviewer=EmailFactory()) group = review_req.team for url in [urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }), @@ -37,7 +36,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(assignment.reviewer.person.__unicode__(), unicontent(r)) + self.assertIn(unicode(review_req.reviewer.person), unicontent(r)) url = urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }) @@ -51,8 +50,7 @@ class ReviewTests(TestCase): self.assertTrue(review_req.doc.name in unicontent(r)) def test_suggested_review_requests(self): - review_req = ReviewRequestFactory(state_id='assigned') - assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned') + review_req = ReviewRequestFactory() doc = review_req.doc team = review_req.team @@ -91,23 +89,22 @@ class ReviewTests(TestCase): self.assertEqual(list(suggested_review_requests_for_team(team)), []) # blocked by versioned refusal - review_req.state = ReviewRequestStateName.objects.get(slug="no-review-version") + review_req.reviewed_rev = doc.rev + review_req.state = ReviewRequestStateName.objects.get(slug="no-review-document") review_req.save() self.assertEqual(list(suggested_review_requests_for_team(team)), []) # blocked by completion - review_req.state = ReviewRequestStateName.objects.get(slug="assigned") + review_req.state = ReviewRequestStateName.objects.get(slug="completed") 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 - assignment.reviewed_rev = prev_rev - assignment.save() + review_req.reviewed_rev = prev_rev + review_req.state = ReviewRequestStateName.objects.get(slug="completed") + review_req.save() self.assertEqual(len(suggested_review_requests_for_team(team)), 1) @@ -115,19 +112,17 @@ 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) - ReviewAssignmentFactory(review_request = review_req1, reviewer=reviewer.email()) + review_req1 = ReviewRequestFactory(state_id='completed',team=team,reviewer=reviewer.email()) PersonFactory(user__username='plain') - 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(), + 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"), ) UnavailablePeriod.objects.create( @@ -174,70 +169,169 @@ class ReviewTests(TestCase): def test_manage_review_requests(self): group = ReviewTeamFactory() - RoleFactory(name_id='reviewer',group=group,person__user__username='reviewer').person + reviewer = 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(),team=group) - review_req2 = ReviewRequestFactory(team=group) - review_req3 = ReviewRequestFactory(team=group) + review_req1 = ReviewRequestFactory(doc__pages=2,doc__shepherd=marsperson.email(),reviewer=reviewer.email(),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" }) - login_testing_unauthorized(self, "secretary", unassigned_url) + + 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, + ) # 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(unassigned_url) + r = self.client.get(assigned_url) self.assertEqual(r.status_code, 200) self.assertTrue(review_req1.doc.name in unicontent(r)) - # Test that conflicts are detected - r = self.client.post(unassigned_url, { - "reviewrequest": [str(review_req3.pk)], + # 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)], - "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", + # 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("2 requests opened" in content) + 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(review_req1.pk),str(review_req2.pk),str(review_req3.pk)], + "reviewrequest": [str(123456)], + "action": "save-continue", + }) + self.assertEqual(r.status_code, 200) + content = unicontent(r).lower() + self.assertTrue("1 request opened" in content) - "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, + # 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 + 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): "", "action": "save", }) self.assertEqual(r.status_code, 302) - review_req3 = reload_db_objects(review_req3) - settings = ReviewerSettings.objects.filter(team=review_req3.team, person=another_reviewer).first() + 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() self.assertEqual(settings.skip_next,1) - self.assertEqual(review_req3.state_id, "assigned") + 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) def test_email_open_review_assignments(self): - review_req1 = ReviewRequestFactory() - ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman')) + review_req1 = ReviewRequestFactory(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_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 %} + {% 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 %} {% endfor %} {% if rotation_list %}Next in the reviewer rotation: @@ -282,14 +376,15 @@ class ReviewTests(TestCase): def test_change_reviewer_settings(self): reviewer = ReviewerSettingsFactory(person__user__username='reviewer',expertise='Some expertise').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) + review_req = ReviewRequestFactory(reviewer=reviewer.email()) + RoleFactory(name_id='reviewer',group=review_req.team,person=review_req.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": assignment.reviewer_id, + "reviewer_email": review_req.reviewer_id, }) login_testing_unauthorized(self, reviewer.user.username, url) @@ -297,7 +392,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": assignment.reviewer_id, + "reviewer_email": review_req.reviewer_id, }) # get @@ -356,7 +451,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() @@ -431,9 +526,8 @@ class ReviewTests(TestCase): self.assertEqual(settings.remind_days_before_deadline, 6) def test_review_reminders(self): - review_req = ReviewRequestFactory(state_id='assigned') + review_req = ReviewRequestFactory() 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) @@ -457,23 +551,23 @@ class ReviewTests(TestCase): review_req.save() # reviewer - needing_reminders = review_assignments_needing_reviewer_reminder(today - datetime.timedelta(days=1)) + needing_reminders = review_requests_needing_reviewer_reminder(today - datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) - needing_reminders = review_assignments_needing_reviewer_reminder(today) - self.assertEqual(list(needing_reminders), [assignment]) + needing_reminders = review_requests_needing_reviewer_reminder(today) + self.assertEqual(list(needing_reminders), [review_req]) - needing_reminders = review_assignments_needing_reviewer_reminder(today + datetime.timedelta(days=1)) + needing_reminders = review_requests_needing_reviewer_reminder(today + datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) # secretary - needing_reminders = review_assignments_needing_secretary_reminder(today - datetime.timedelta(days=1)) + needing_reminders = review_requests_needing_secretary_reminder(today - datetime.timedelta(days=1)) self.assertEqual(list(needing_reminders), []) - needing_reminders = review_assignments_needing_secretary_reminder(today) - self.assertEqual(list(needing_reminders), [(assignment, secretary_role)]) + 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 + datetime.timedelta(days=1)) + needing_reminders = review_requests_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 3a9a70235..6f02b8b27 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/(?Punassigned)/$', views.manage_review_requests), + url(r'^reviews/manage/(?Passigned|unassigned)/$', 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 2fb088b51..8ae517dd2 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 a60ff5d90..c6a405439 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, ReviewAssignment, ReviewerSettings, ReviewSecretarySettings +from ietf.review.models import ReviewRequest, 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_assignments_for_reviewers, + latest_review_requests_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 ReviewAssignmentStateName +from ietf.name.models import ReviewRequestStateName from ietf.utils.mail import send_mail_text, parse_preformatted from ietf.ietfauth.utils import user_is_person @@ -1263,26 +1263,27 @@ 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).filter( - Q(state_id='requested') | Q(state_id='assigned',reviewassignment__state__in=('assigned','accepted')) + open_review_requests = ReviewRequest.objects.filter( + team=team, + state__in=("requested", "accepted") ).prefetch_related( - "type", "state", "doc", "doc__states", + "reviewer__person", "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(state_id='requested')) + open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(reviewer=None)) elif assignment_status == "assigned": - open_review_requests = list(open_review_requests.filter(state_id='assigned')) + open_review_requests = list(open_review_requests.exclude(reviewer=None)) 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 @@ -1291,19 +1292,25 @@ def review_requests(request, acronym, group_type=None): if not group.features.has_reviews: raise Http404 - unassigned_review_requests = [r for r in get_open_review_requests_for_team(group) if not r.state_id=='assigned'] + assigned_review_requests = [] + unassigned_review_requests = [] - 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) + for r in get_open_review_requests_for_team(group): + if r.reviewer: + assigned_review_requests.append(r) + else: + unassigned_review_requests.append(r) - closed_review_assignments = ReviewAssignment.objects.filter(review_request__team=group).exclude(state_id__in=('assigned','accepted')).prefetch_related("state","result").order_by('-assigned_on') + open_review_requests = [ + ("Unassigned", unassigned_review_requests), + ("Assigned", assigned_review_requests), + ] - closed_review_requests = ReviewRequest.objects.filter(team=group).exclude(state__in=("requested", "assigned")).prefetch_related("type", "state", "doc").order_by("-time", "-id") + 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") since_choices = [ (None, "1 month"), @@ -1331,14 +1338,10 @@ 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, { - "unassigned_review_requests": unassigned_review_requests, - "open_review_assignments": open_review_assignments, + "open_review_requests": open_review_requests, "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), @@ -1362,8 +1365,8 @@ def reviewer_overview(request, acronym, group_type=None): today = datetime.date.today() - req_data_for_reviewers = latest_review_assignments_for_reviewers(group) - assignment_state_by_slug = { n.slug: n for n in ReviewAssignmentStateName.objects.all() } + req_data_for_reviewers = latest_review_requests_for_reviewers(group) + review_state_by_slug = { n.slug: n for n in ReviewRequestStateName.objects.all() } days_needed = days_needed_to_fulfill_min_interval_for_reviewers(group) @@ -1384,16 +1387,15 @@ 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 ["assigned", "accepted"]) + open_reqs = sum(1 for d in req_data if d.state in ["requested", "accepted"]) latest_reqs = [] for d in req_data: - 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), + 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), 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: @@ -1472,6 +1474,23 @@ 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()) @@ -1546,35 +1565,37 @@ 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_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 = 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.sort(key=lambda r:r.reviewer.person.last_name()+r.reviewer.person.first_name()) + review_requests.sort(key=lambda r:r.reviewer.person.last_name()+r.reviewer.person.first_name()) - for r in review_assignments: - if r.review_request.doc.telechat_date(): - r.section = 'For telechat %s' % r.review_request.doc.telechat_date().isoformat() + for r in review_requests: + if r.doc.telechat_date(): + r.section = 'For telechat %s' % r.doc.telechat_date().isoformat() r.section_order='0'+r.section - elif r.review_request.type_id == 'early': + elif r.type_id == 'early': r.section = 'Early review requests:' r.section_order='2' else: r.section = 'Last calls:' r.section_order='1' - e = r.review_request.doc.latest_event(LastCallDocEvent, type="sent_last_call") + e = r.doc.latest_event(LastCallDocEvent, type="sent_last_call") r.lastcall_ends = e and e.expires.date().isoformat() - r.earlier_review = ReviewAssignment.objects.filter(review_request__doc=r.review_request.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed") + r.earlier_review = ReviewRequest.objects.filter(doc=r.doc,reviewer__in=r.reviewer.person.email_set.all(),state="completed") if r.earlier_review: - req_rev = r.review_request.requested_rev or r.review_request.doc.rev + req_rev = r.requested_rev or r.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_assignments.sort(key=lambda r: r.section_order) + review_requests.sort(key=lambda r: r.section_order) back_url = request.GET.get("next") if not back_url: @@ -1609,7 +1630,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_assignments": review_assignments, + "review_requests": review_requests, "rotation_list": reviewer_rotation_list(group)[:10], "group" : group, }) @@ -1630,7 +1651,7 @@ def email_open_review_assignments(request, acronym, group_type=None): return render(request, 'group/email_open_review_assignments.html', { 'group': group, - 'review_assignments': review_assignments, + 'review_requests': review_requests, 'form': form, 'back_url': back_url, }) @@ -1716,14 +1737,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_assignments = ReviewAssignment.objects.filter(state__in=["assigned", "accepted"], reviewer=reviewer_role.email, review_request__team=group) + review_reqs = ReviewRequest.objects.filter(state__in=["requested", "accepted"], reviewer=reviewer_role.email, team=group) msg += "\n\n" - if review_assignments: + if review_reqs: msg += "{} is currently assigned to review:".format(reviewer_role.person) - for r in review_assignments: + for r in review_reqs: msg += "\n\n" - msg += "{} (deadline: {})".format(r.review_request.doc_id, r.review_request.deadline.isoformat()) + msg += "{} (deadline: {})".format(r.doc_id, r.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 46a22adf3..a221c1d59 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_assignments_to_list_for_docs +from ietf.review.utils import review_requests_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_assignments_for_docs = review_assignments_to_list_for_docs(docs) + review_requests_for_docs = review_requests_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_assignments = review_assignments_for_docs.get(doc.pk, []) + doc.review_requests = review_requests_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 e1ff2ed50..fbc5831ee 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, ReviewAssignmentFactory +from ietf.review.factories import ReviewRequestFactory from ietf.review.models import ReviewWish, UnavailablePeriod from ietf.utils.decorators import skip_coverage @@ -358,12 +358,11 @@ class IetfAuthTests(TestCase): self.assertTrue(self.username_in_htpasswd_file(user.username)) def test_review_overview(self): - 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) + review_req = ReviewRequestFactory(reviewer=EmailFactory(person__user__username='reviewer')) + RoleFactory(name_id='reviewer',group=review_req.team,person=review_req.reviewer.person) doc = review_req.doc - reviewer = assignment.reviewer.person + reviewer = review_req.reviewer.person UnavailablePeriod.objects.create( team=review_req.team, diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 4fe2e1349..dd1dbda97 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 ReviewerSettings, ReviewWish, ReviewAssignment +from ietf.review.models import ReviewRequest, ReviewerSettings, ReviewWish 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_assignments = ReviewAssignment.objects.filter( + open_review_requests = ReviewRequest.objects.filter( reviewer__person__user=request.user, - state__in=["assigned", "accepted"], + state__in=["requested", "accepted"], ) today = Date.today() - for r in open_review_assignments: - r.due = max(0, (today - r.review_request.deadline).days) + for r in open_review_requests: + r.due = max(0, (today - r.deadline).days) - closed_review_assignments = ReviewAssignment.objects.filter( + closed_review_requests = ReviewRequest.objects.filter( reviewer__person__user=request.user, state__in=["no-response", "part-completed", "completed"], - ).order_by("-review_request__time")[:20] + ).order_by("-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_assignments': open_review_assignments, - 'closed_review_assignments': closed_review_assignments, + 'open_review_requests': open_review_requests, + 'closed_review_requests': closed_review_requests, '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 3924b7910..9eb0f5cc5 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, ReviewAssignmentStateName) + DocUrlTagName) from ietf.stats.models import CountryAlias @@ -68,7 +68,6 @@ 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 deleted file mode 100644 index dbf253e51..000000000 --- a/ietf/name/migrations/0005_reviewassignmentstatename.py +++ /dev/null @@ -1,29 +0,0 @@ -# -*- 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 deleted file mode 100644 index 18c464a8c..000000000 --- a/ietf/name/migrations/0006_adjust_statenames.py +++ /dev/null @@ -1,93 +0,0 @@ -# -*- 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 5a257c22f..ac96ab9bc 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -95,9 +95,8 @@ class LiaisonStatementEventTypeName(NameModel): class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" class ReviewRequestStateName(NameModel): - """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""" + """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, + No Response, No Review of Version, No Review of Document, 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 83f8d0b6f..76dca2a15 100644 --- a/ietf/name/resources.py +++ b/ietf/name/resources.py @@ -14,9 +14,8 @@ from ietf.name.models import ( AgendaTypeName, BallotPositionName, ConstraintNam ImportantDateName, IntendedStdLevelName, IprDisclosureStateName, IprEventTypeName, IprLicenseTypeName, LiaisonStatementEventTypeName, LiaisonStatementPurposeName, LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName, - ReviewAssignmentStateName, ReviewRequestStateName, ReviewResultName, ReviewTypeName, - RoleName, RoomResourceName, SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, - TopicAudienceName, ) + ReviewRequestStateName, ReviewResultName, ReviewTypeName, RoleName, RoomResourceName, + SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, TopicAudienceName, ) class TimeSlotTypeNameResource(ModelResource): class Meta: @@ -429,20 +428,6 @@ 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() @@ -582,5 +567,3 @@ class AgendaTypeNameResource(ModelResource): "order": ALL, } api.name.register(AgendaTypeNameResource()) - - diff --git a/ietf/review/admin.py b/ietf/review/admin.py index 4fbe41059..691cee738 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, ReviewAssignment, ReviewTeamSettings ) + ReviewWish, NextReviewerInTeam, ReviewRequest, ReviewTeamSettings ) class ReviewerSettingsAdmin(simple_history.admin.SimpleHistoryAdmin): def acronym(self, obj): @@ -53,23 +53,14 @@ 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"] + list_filter = ["team", "type", "state", "result"] ordering = ["-id"] - raw_id_fields = ["doc", "team", "requested_by"] + raw_id_fields = ["doc", "team", "requested_by", "reviewer", "review"] date_hierarchy = "time" - search_fields = ["doc__name"] + search_fields = ["doc__name", "reviewer__person__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 7e7a2480f..80353b4be 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, ReviewAssignment, ReviewerSettings +from ietf.review.models import ReviewTeamSettings, ReviewRequest, ReviewerSettings from ietf.name.models import ReviewTypeName, ReviewResultName class ReviewTeamSettingsFactory(factory.DjangoModelFactory): @@ -39,15 +39,6 @@ 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 deleted file mode 100644 index 766dc209e..000000000 --- a/ietf/review/migrations/0009_refactor_review_request.py +++ /dev/null @@ -1,67 +0,0 @@ -# -*- 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 deleted file mode 100644 index f18b63d61..000000000 --- a/ietf/review/migrations/0010_populate_review_assignments.py +++ /dev/null @@ -1,121 +0,0 @@ -# -*- 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 dab758d50..d4a89619d 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, ReviewAssignmentStateName +from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName from ietf.utils.validators import validate_regular_expression_string from ietf.utils.models import ForeignKey, OneToOneField @@ -105,7 +105,9 @@ 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.""" + """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.""" state = ForeignKey(ReviewRequestStateName) # Fields filled in on the initial record creation - these @@ -119,38 +121,34 @@ 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='') - # 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) + # 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) - 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) + 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) def __unicode__(self): return u"%s review on %s by %s %s" % (self.type, self.doc, self.team, self.state) - def all_completed_assignments_for_doc(self): - return ReviewAssignment.objects.filter(review_request__doc=self.doc, state__in=['completed','part-completed']) + def other_requests(self): + return self.doc.reviewrequest_set.exclude(id=self.id) - 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 other_completed_requests(self): + return self.other_requests().filter(state_id__in=['completed','part-completed']) + 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 abbf10202..059b801c5 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, ReviewAssignment, +from ietf.review.models import (ReviewerSettings, ReviewRequest, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewSecretarySettings, ReviewTeamSettings, HistoricalReviewerSettings ) @@ -45,6 +45,9 @@ 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() @@ -56,38 +59,17 @@ 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(ReviewAssignmentResource()) +api.review.register(ReviewRequestResource()) 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 aa9fc888d..32994b329 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, ReviewAssignmentDocEvent, State, +from ietf.doc.models import (Document, ReviewRequestDocEvent, 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, ReviewAssignment, ReviewRequestStateName, ReviewTypeName, +from ietf.review.models import (ReviewRequest, 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", "assigned"]) + return ReviewRequestStateName.objects.filter(used=True).exclude(slug__in=["requested", "accepted", "rejected", "part-completed", "completed"]) 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_assignments_to_list_for_docs(docs): - assignment_qs = ReviewAssignment.objects.filter( - state__in=["assigned", "accepted", "part-completed", "completed"], +def review_requests_to_list_for_docs(docs): + request_qs = ReviewRequest.objects.filter( + state__in=["requested", "accepted", "part-completed", "completed"], ).prefetch_related("result") doc_names = [d.name for d in docs] - return extract_revision_ordered_review_assignments_for_documents_and_replaced(assignment_qs, doc_names) + return extract_revision_ordered_review_requests_for_documents_and_replaced(request_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__requested_rev=rev, + reviewrequest__reviewed_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(ReviewAssignment.objects.filter( - review_request__team=team, - ).values_list("reviewer__person").annotate(Max("assigned_on"))) + latest_assignments = dict(ReviewRequest.objects.filter( + team=team, + ).values_list("reviewer__person").annotate(Max("time"))) min_intervals = dict(ReviewerSettings.objects.filter(team=team).values_list("person_id", "min_interval")) @@ -166,39 +166,40 @@ def days_needed_to_fulfill_min_interval_for_reviewers(team): return res -ReviewAssignmentData = namedtuple("ReviewAssignmentData", [ - "assignment_pk", "doc", "doc_pages", "req_time", "state", "assigned_time", "deadline", "reviewed_rev", "result", "team", "reviewer", +ReviewRequestData = namedtuple("ReviewRequestData", [ + "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"]) -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) +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) for easy use with itertools.groupby. Valid entries in *ordering are "team" and "reviewer".""" filters = Q() if teams: - filters &= Q(review_request__team__in=teams) + filters &= Q(team__in=teams) if reviewers: filters &= Q(reviewer__person__in=reviewers) if time_from: - filters &= Q(review_request__time__gte=time_from) + filters &= Q(time__gte=time_from) if time_to: - filters &= Q(review_request__time__lte=time_to) + filters &= Q(time__lte=time_to) - # 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) + # we may be dealing with a big bunch of data, so treat it carefully + event_qs = ReviewRequest.objects.filter(filters) + # left outer join with RequestRequestDocEvent for request/assign/close time event_qs = event_qs.values_list( - "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" + "pk", "doc", "doc__pages", "time", "state", "deadline", "reviewed_rev", "result", "team", + "reviewer__person", "reviewrequestdocevent__time", "reviewrequestdocevent__type" ) - 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"]) + event_qs = event_qs.order_by(*[o.replace("reviewer", "reviewer__person") for o in ordering] + ["time", "pk", "-reviewrequestdocevent__time"]) def positive_days(time_from, time_to): if time_from is None or time_to is None: @@ -211,31 +212,33 @@ def extract_review_assignment_data(teams=None, reviewers=None, time_from=None, t else: return 0.0 - requested_time = assigned_time = closed_time = None + for _, events in itertools.groupby(event_qs.iterator(), lambda t: t[0]): + requested_time = assigned_time = closed_time = None - for assignment in event_qs: + for e in events: + req_pk, doc, doc_pages, req_time, state, deadline, reviewed_rev, result, team, reviewer, event_time, event_type = e - 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 + 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 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 = ReviewAssignmentData(assignment_pk, doc, doc_pages, req_time, state, assigned_time, deadline, reviewed_rev, result, team, reviewer, + d = ReviewRequestData(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) yield d - -def aggregate_raw_period_review_assignment_stats(review_assignment_data, count=None): +def aggregate_raw_period_review_request_stats(review_request_data, count=None): """Take a sequence of review request data from - extract_review_assignment_data and aggregate them.""" + extract_review_request_data and aggregate them.""" state_dict = defaultdict(int) late_state_dict = defaultdict(int) @@ -243,8 +246,8 @@ def aggregate_raw_period_review_assignment_stats(review_assignment_data, count=N assignment_to_closure_days_list = [] assignment_to_closure_days_count = 0 - 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: + 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: if count == "pages": c = doc_pages else: @@ -263,7 +266,7 @@ def aggregate_raw_period_review_assignment_stats(review_assignment_data, count=N return state_dict, late_state_dict, result_dict, assignment_to_closure_days_list, assignment_to_closure_days_count -def sum_period_review_assignment_stats(raw_aggregation): +def sum_period_review_request_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 @@ -271,11 +274,11 @@ def sum_period_review_assignment_stats(raw_aggregation): res["state"] = state_dict res["result"] = result_dict - res["open"] = sum(state_dict.get(s, 0) for s in ("assigned", "accepted")) + res["open"] = sum(state_dict.get(s, 0) for s in ("requested", "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 ("assigned", "accepted")) + res["open_late"] = sum(late_state_dict.get(s, 0) for s in ("requested", "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"] @@ -284,7 +287,7 @@ def sum_period_review_assignment_stats(raw_aggregation): return res -def sum_raw_review_assignment_aggregations(raw_aggregations): +def sum_raw_review_request_aggregations(raw_aggregations): """Collapse a sequence of aggregations into one aggregation.""" state_dict = defaultdict(int) late_state_dict = defaultdict(int) @@ -306,61 +309,34 @@ def sum_raw_review_assignment_aggregations(raw_aggregations): return state_dict, late_state_dict, result_dict, assignment_to_closure_days_list, assignment_to_closure_days_count -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.""" +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.""" - extracted_data = extract_review_assignment_data( + extracted_data = extract_review_request_data( teams=[team], time_from=datetime.date.today() - datetime.timedelta(days=days_back), ordering=["reviewer"], ) - assignment_data_for_reviewers = { + req_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 assignment_data_for_reviewers + return req_data_for_reviewers -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 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_request_change(request, review_req, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by): @@ -386,8 +362,7 @@ 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: - for assignment in review_req.reviewassignment_set.all(): - extract_email_addresses([assignment.reviewer]) + extract_email_addresses([review_req.reviewer]) if notify_requested_by: extract_email_addresses([review_req.requested_by.email()]) @@ -448,21 +423,24 @@ 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", "assigned") + assert review_req.state_id in ("requested", "accepted") - if review_req.reviewassignment_set.filter(reviewer=reviewer).exists(): + if reviewer == review_req.reviewer: return - # Note that assigning a review no longer unassigns other reviews + 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) - 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()) + review_req.state = ReviewRequestStateName.objects.get(slug="requested") + review_req.reviewer = reviewer + review_req.save() - if reviewer: - possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip) + if review_req.reviewer: + possibly_advance_next_reviewer_for_team(review_req.team, review_req.reviewer.person_id, add_skip) ReviewRequestDocEvent.objects.create( type="assigned_review_request", @@ -472,40 +450,26 @@ 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(), - reviewer.person if reviewer else "(None)", + review_req.reviewer.person if review_req.reviewer else "(None)", ), review_request=review_req, - 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', + state=None, ) msg = "%s has assigned you as a reviewer for this document." % request.user.person.ascii - prev_team_reviews = ReviewAssignment.objects.filter( - review_request__doc=review_req.doc, + prev_team_reviews = ReviewRequest.objects.filter( + doc=review_req.doc, state="completed", - review_request__team=review_req.team, + team=review_req.team, ) if prev_team_reviews.exists(): msg = msg + '\n\nThis team has completed other reviews of this document:\n' - for assignment in prev_team_reviews: + for req in prev_team_reviews: msg += u'%s %s -%s %s\n'% ( - assignment.completed_on.strftime('%d %b %Y'), - assignment.reviewer.person.ascii, - assignment.reviewed_rev or assignment.review_request.requested_rev, - assignment.result.name, + req.review_done_time().strftime('%d %b %Y'), + req.reviewer.person.ascii, + req.reviewed_rev or req.requested_rev, + req.result.name, ) email_review_request_change( @@ -562,9 +526,8 @@ def close_review_request(request, review_req, close_state): suggested_req = review_req.pk is None review_req.state = close_state -# 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 + 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: @@ -579,19 +542,6 @@ 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), @@ -686,7 +636,7 @@ def suggested_review_requests_for_team(team): seen_deadlines[doc_pk] = deadline - # filter those with existing explicit requests + # filter those with existing requests existing_requests = defaultdict(list) for r in ReviewRequest.objects.filter(doc__in=requests.iterkeys(), team=team): existing_requests[r.doc_id].append(r) @@ -696,67 +646,18 @@ def suggested_review_requests_for_team(team): return False no_review_document = existing.state_id == "no-review-document" - 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() + pending = (existing.state_id in ("requested", "accepted") and (not existing.requested_rev or existing.requested_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() + completed_or_closed = (existing.state_id not in ("part-completed", "rejected", "overtaken", "no-response") + and existing.reviewed_rev == request.doc.rev) - return any([no_review_document, no_review_rev, pending, request_closed, some_assignment_completed]) + return no_review_document or pending or completed_or_closed 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.""" @@ -765,7 +666,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("-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("-reviewed_rev", "-time", "-id").iterator(): requests_for_each_doc[r.doc_id].append(r) # now collect in breadth-first order to keep the revision order intact @@ -803,12 +704,10 @@ 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) - one_assignment = review_req.reviewassignment_set.first() - if one_assignment: - field.initial = one_assignment.reviewer_id + if review_req.reviewer: + field.initial = review_req.reviewer_id choices = make_assignment_choices(field.queryset, review_req) if not field.required: @@ -853,15 +752,15 @@ def make_assignment_choices(email_queryset, review_req): # previous review of document has_reviewed_previous = ReviewRequest.objects.filter( doc=doc, - reviewassignment__reviewer__person__in=possible_person_ids, - reviewassignment__state="completed", + reviewer__person__in=possible_person_ids, + 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("reviewassignment__reviewer__person", flat=True)) + has_reviewed_previous = set(has_reviewed_previous.values_list("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)) @@ -881,7 +780,7 @@ def make_assignment_choices(email_queryset, review_req): unavailable_periods = current_unavailable_periods_for_reviewers(team) # reviewers statistics - assignment_data_for_reviewers = latest_review_assignments_for_reviewers(team) + req_data_for_reviewers = latest_review_requests_for_reviewers(team) ranking = [] for e in possible_emails: @@ -936,21 +835,21 @@ def make_assignment_choices(email_queryset, review_req): # stats stats = [] - assignment_data = assignment_data_for_reviewers.get(e.person_id, []) + req_data = req_data_for_reviewers.get(e.person_id, []) - 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"]) + 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"]) if currently_open > 0: stats.append("currently {count} open, {pages} pages".format(count=currently_open, pages=pages)) - could_have_completed = [d for d in assignment_data if d.state in ["part-completed", "completed", "no-response"]] + could_have_completed = [d for d in req_data if d.state in ["part-completed", "completed", "no-response"]] if could_have_completed: - no_response = len([d for d in assignment_data if d.state == 'no-response']) + no_response = len([d for d in req_data if d.state == 'no-response']) if no_response: stats.append("%s no response" % no_response) - part_completed = len([d for d in assignment_data if d.state == 'part-completed']) + part_completed = len([d for d in req_data if d.state == 'part-completed']) if part_completed: stats.append("%s partially complete" % part_completed) - completed = len([d for d in assignment_data if d.state == 'completed']) + completed = len([d for d in req_data if d.state == 'completed']) if completed: stats.append("%s fully completed" % completed) @@ -971,19 +870,21 @@ def make_assignment_choices(email_queryset, review_req): return [(r["email"].pk, r["label"]) for r in ranking] -def review_assignments_needing_reviewer_reminder(remind_date): - assignment_qs = ReviewAssignment.objects.filter( - state__in=("assigned", "accepted"), +def review_requests_needing_reviewer_reminder(remind_date): + reqs_qs = ReviewRequest.objects.filter( + state__in=("requested", "accepted"), reviewer__person__reviewersettings__remind_days_before_deadline__isnull=False, - reviewer__person__reviewersettings__team=F("review_request__team"), - ).values_list("pk", "review_request__deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct() + reviewer__person__reviewersettings__team=F("team"), + ).exclude( + reviewer=None + ).values_list("pk", "deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct() - assignment_pks = [] - for a_pk, deadline, remind_days in assignment_qs: + req_pks = [] + for r_pk, deadline, remind_days in reqs_qs: if (deadline - remind_date).days == remind_days: - assignment_pks.append(a_pk) + req_pks.append(r_pk) - return ReviewAssignment.objects.filter(pk__in=assignment_pks).select_related("reviewer", "reviewer__person", "state", "review_request__team") + return ReviewRequest.objects.filter(pk__in=req_pks).select_related("reviewer", "reviewer__person", "state", "team") def email_reviewer_reminder(review_request): team = review_request.team @@ -1010,24 +911,24 @@ def email_reviewer_reminder(review_request): "remind_days": remind_days, }) -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"), +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"), ).exclude( reviewer=None - ).values_list("pk", "review_request__deadline", "review_request__team__role", "review_request__team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct() + ).values_list("pk", "deadline", "team__role", "team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct() - assignment_pks = {} - for a_pk, deadline, secretary_role_pk, remind_days in assignment_qs: + req_pks = {} + for r_pk, deadline, secretary_role_pk, remind_days in reqs_qs: if (deadline - remind_date).days == remind_days: - assignment_pks[a_pk] = secretary_role_pk + req_pks[r_pk] = secretary_role_pk - 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") } + 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") } - return [ (review_assignments[a_pk], secretary_roles[secretary_role_pk]) for a_pk, secretary_role_pk in assignment_pks.iteritems() ] + return [ (review_reqs[req_pk], secretary_roles[secretary_role_pk]) for req_pk, secretary_role_pk in req_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 19485abcf..ad2a0c9aa 100644 --- a/ietf/static/ietf/css/ietf.css +++ b/ietf/static/ietf/css/ietf.css @@ -451,11 +451,7 @@ 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 13656a723..ee7571f4b 100644 --- a/ietf/stats/tests.py +++ b/ietf/stats/tests.py @@ -4,8 +4,6 @@ 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 @@ -20,7 +18,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, ReviewAssignmentFactory +from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory from ietf.stats.models import MeetingRegistration, CountryAlias from ietf.stats.utils import get_meeting_registration_data @@ -159,8 +157,7 @@ class StatisticsTests(TestCase): def test_review_stats(self): reviewer = PersonFactory() - review_req = ReviewRequestFactory(state_id='assigned') - ReviewAssignmentFactory(review_request=review_req, state_id='assigned', reviewer=reviewer.email_set.first()) + review_req = ReviewRequestFactory(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 4eeac6506..f2848b36f 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_assignment_data, - aggregate_raw_period_review_assignment_stats, - ReviewAssignmentData, - sum_period_review_assignment_stats, - sum_raw_review_assignment_aggregations) +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.submit.models import Submission from ietf.group.models import Role, Group from ietf.person.models import Person -from ietf.name.models import ReviewResultName, CountryName, DocRelationshipName, ReviewAssignmentStateName +from ietf.name.models import ReviewRequestStateName, ReviewResultName, CountryName, DocRelationshipName 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", "Assignment states"), + ("states", "Request 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 = ReviewAssignmentData._fields.index("team") + group_by_index = ReviewRequestData._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__reviewassignment__review_request__time__gte=from_time, - email__reviewassignment__review_request__time__lte=to_time, - email__reviewassignment__review_request__team=reviewers_for_team, + email__reviewrequest__time__gte=from_time, + email__reviewrequest__time__lte=to_time, + email__reviewrequest__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 = ReviewAssignmentData._fields.index("reviewer") + group_by_index = ReviewRequestData._fields.index("reviewer") # now filter and aggregate the data possible_teams = possible_completion_types = possible_results = possible_states = None @@ -1139,23 +1139,22 @@ 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_assignment_data(query_teams, query_reviewers, from_time, to_time) + extracted_data = extract_review_request_data(query_teams, query_reviewers, from_time, to_time) - req_time_index = ReviewAssignmentData._fields.index("req_time") + req_time_index = ReviewRequestData._fields.index("req_time") def time_key_fn(t): d = t[req_time_index].date() #d -= datetime.timedelta(days=d.weekday()) # weekly - # 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 + d -= datetime.timedelta(days=d.day) # 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_assignment_stats(request_data_items, count=count) - aggr = sum_period_review_assignment_stats(raw_aggr) + raw_aggr = aggregate_raw_period_review_request_stats(request_data_items, count=count) + aggr = sum_period_review_request_stats(raw_aggr) aggrs.append((d, aggr)) @@ -1165,7 +1164,7 @@ def review_stats(request, stats_type=None, acronym=None): found_states.add(slug) results = ReviewResultName.objects.filter(slug__in=found_results) - states = ReviewAssignmentStateName.objects.filter(slug__in=found_states) + states = ReviewRequestStateName.objects.filter(slug__in=found_states) # choice @@ -1214,7 +1213,7 @@ def review_stats(request, stats_type=None, acronym=None): }]) else: # tabular data - extracted_data = extract_review_assignment_data(query_teams, query_reviewers, from_time, to_time, ordering=[level]) + extracted_data = extract_review_request_data(query_teams, query_reviewers, from_time, to_time, ordering=[level]) data = [] @@ -1222,10 +1221,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_assignment_stats(request_data_items, count=count) + raw_aggr = aggregate_raw_period_review_request_stats(request_data_items, count=count) raw_aggrs.append(raw_aggr) - aggr = sum_period_review_assignment_stats(raw_aggr) + aggr = sum_period_review_request_stats(raw_aggr) # skip zero-valued rows if aggr["open"] == 0 and aggr["completed"] == 0 and aggr["not_completed"] == 0: @@ -1242,12 +1241,12 @@ def review_stats(request, stats_type=None, acronym=None): # add totals row if len(raw_aggrs) > 1: - totals = sum_period_review_assignment_stats(sum_raw_review_assignment_aggregations(raw_aggrs)) + totals = sum_period_review_request_stats(sum_raw_review_request_aggregations(raw_aggrs)) totals["obj"] = "Totals" data.append(totals) results = ReviewResultName.objects.filter(slug__in=found_results) - states = ReviewAssignmentStateName.objects.filter(slug__in=found_states) + states = ReviewRequestStateName.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 884b2db37..d8c4ad95c 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -204,14 +204,14 @@ {% endif %} {% endfor %} - {% if review_assignments or can_request_review %} + {% if review_requests or can_request_review %} Reviews - {% for review_assignment in review_assignments %} - {% include "doc/review_assignment_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} + {% for review_request in review_requests %} + {% include "doc/review_request_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 efa13552f..263ee5cfd 100644 --- a/ietf/templates/doc/document_review.html +++ b/ietf/templates/doc/document_review.html @@ -43,8 +43,8 @@ Other reviews - {% 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 %} + {% 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 %} {% endfor %} diff --git a/ietf/templates/doc/review/complete_review.html b/ietf/templates/doc/review/complete_review.html index cfc586529..12dda2b60 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
- {{ assignment.review_request.doc.name }} + {{ review_req.doc.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 }}
+
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 }}

{% 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 deleted file mode 100644 index aa23b3c6a..000000000 --- a/ietf/templates/doc/review/mark_reviewer_assignment_no_response.html +++ /dev/null @@ -1,22 +0,0 @@ -{% 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 d70ce0770..93380a405 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" %} -

Do you want to reject this assignment?

+

{{ review_req.reviewer.person }} is currently assigned to do the review. 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 78205bd5c..a59c607c0 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 bootstrap3 %}{% origin %} +{# Copyright The IETF Trust 2017, All Rights Reserved #}{% load origin %}{% origin %} @@ -68,20 +68,14 @@ {% endif %} - {% if doc.time %} - - - - {% endif %} - - - - + @@ -98,107 +92,115 @@ {% 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 %} +
Draft last updated{{ doc.time|date:"Y-m-d" }}
Completed reviewsOther Reviews - {% 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 %}
+ {% 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 %} {% 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 deleted file mode 100644 index 1c2496ff2..000000000 --- a/ietf/templates/doc/review/withdraw_reviewer_assignment.html +++ /dev/null @@ -1,22 +0,0 @@ -{% 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 deleted file mode 100644 index e6e9bcca6..000000000 --- a/ietf/templates/doc/review_assignment_summary.html +++ /dev/null @@ -1,12 +0,0 @@ -
- {% 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/group/email_open_review_assignments.html b/ietf/templates/group/email_open_review_assignments.html index 4ac55fc46..74dcc5b03 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_assignments %} + {% if review_requests %} {% if closed_review_requests %} -

Closed review requests

- +
@@ -134,7 +91,9 @@ + + @@ -144,49 +103,27 @@ - + + + {% 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.request_closed_time|date:"Y-m-d" }}{{ r.review_done_time|date:"Y-m-d" }} + {% if r.reviewer %} + {{ r.reviewer.person }} + {% else %} + not yet assigned + {% endif %} + {{ r.state.name }} + {% if r.result %} + {{ r.result.name }} + {% endif %} +
- {% 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
+ + {% else %} +

No closed requests found.

{% endif %} {% endblock %} diff --git a/ietf/templates/iesg/agenda_doc.html b/ietf/templates/iesg/agenda_doc.html index 3a018097e..4095122c5 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_assignments %} + {% if doc.review_requests %}
Reviews
- {% for review_assignment in doc.review_assignments %} - {% include "doc/review_assignment_summary.html" with current_doc_name=doc.name current_rev=doc.rev %} + {% for review_request in doc.review_requests %} + {% include "doc/review_request_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 c5700d795..49ed89043 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_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 %} + 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 %} {% 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 e79259a5b..1451f5458 100644 --- a/ietf/templates/ietfauth/review_overview.html +++ b/ietf/templates/ietfauth/review_overview.html @@ -17,7 +17,7 @@

Assigned reviews

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

Latest closed review assignments

+

Latest closed review requests

- {% if closed_review_assignments %} + {% if closed_review_requests %}
{{ 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.doc.name }}{% if r.requested_rev %}{{ r.requested_rev }}{% else %}Current{% endif %}{{r.doc.rev}}{{ r.team.acronym }}{{ r.type.name }} - {{ r.review_request.deadline|date:"Y-m-d" }} + {{ r.deadline|date:"Y-m-d" }} {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %}
@@ -66,14 +66,14 @@ - {% for r in closed_review_assignments %} + {% for r in closed_review_requests %} - - - - + + + + diff --git a/ietf/templates/review/completed_review.txt b/ietf/templates/review/completed_review.txt index bdbe321ca..ca4eb28fd 100644 --- a/ietf/templates/review/completed_review.txt +++ b/ietf/templates/review/completed_review.txt @@ -1,8 +1,9 @@ -{% 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. +{% 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. -{% endif %}Reviewer: {{ assignment.reviewer.person }} -Review result: {{ assignment.result.name }} +{% endif %}Reviewer: {{ review_req.reviewer.person }} +Review result: {{ review_req.result.name }} {{ content }} {% endfilter %}{% endautoescape %} diff --git a/ietf/templates/review/notify_ad.txt b/ietf/templates/review/notify_ad.txt index 7dc585b2d..9e5196a9a 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: "{{assignment.result}}" review submitted for {{assignment.review_request.doc}}{% if assignment.reviewed_rev %}-{{assignment.reviewed_rev}}{% endif %} +Subject: "{{review_req.result}}" review submitted 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 %}. +{{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 %}. -The review is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=assignment.review.name %} +The review is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.review.name %} -The document is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=assignment.review_request.doc.name %} +The document is available at {{settings.IDTRACKER_BASE_URL}}{% url 'ietf.doc.views_doc.document_main' name=review_req.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 8316ee7fd..6d518357c 100644 --- a/ietf/templates/review/partially_completed_review.txt +++ b/ietf/templates/review/partially_completed_review.txt @@ -1,7 +1,10 @@ {% autoescape off %}Review was partially completed by {{ by }}. -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 %} +{% 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. {% endif %}{% endautoescape %} diff --git a/ietf/templates/submit/tool_instructions.html b/ietf/templates/submit/tool_instructions.html index 0b31b9331..df4c1a977 100644 --- a/ietf/templates/submit/tool_instructions.html +++ b/ietf/templates/submit/tool_instructions.html @@ -12,28 +12,7 @@

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. -

- -

- 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/. + The specification for this tool can be found in RFC 4228.

Upload screen

@@ -41,17 +20,6 @@ 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:

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

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 5fca19e10..62b3fc734 100644 --- a/ietf/templates/submit/upload_submission.html +++ b/ietf/templates/submit/upload_submission.html @@ -12,14 +12,27 @@

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 @@ -29,13 +42,7 @@

- 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 + Before you submit your draft, it is recommended that you check it for nits using the idnits tool.

{{ 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.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.deadline|date:"Y-m-d" }} + {{ r.deadline|date:"Y-m-d" }} {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %} {{ r.state.name }}