diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index d56ae56b5..2c33c936f 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -411,12 +411,8 @@ class BallotPositionDocEventFactory(DocEventFactory): model = BallotPositionDocEvent type = 'changed_ballot_position' - - # This isn't right - it needs to build a ballot for the same doc as this position - # For now, deal with this in test code by building BallotDocEvent and BallotPositionDocEvent - # separately and passing the same doc into thier factories. - ballot = factory.SubFactory(BallotDocEventFactory) - + ballot = factory.SubFactory(BallotDocEventFactory) + doc = factory.SelfAttribute('ballot.doc') # point to same doc as the ballot balloter = factory.SubFactory('ietf.person.factories.PersonFactory') pos_id = 'discuss' diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 8ddaa3174..0e2502974 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -1353,7 +1353,11 @@ class BallotPositionDocEvent(DocEvent): def any_email_sent(self): # When the send_email field is introduced, old positions will have it # set to None. We still essentially return True, False, or don't know: - sent_list = BallotPositionDocEvent.objects.filter(ballot=self.ballot, time__lte=self.time, ad=self.ad).values_list('send_email', flat=True) + sent_list = BallotPositionDocEvent.objects.filter( + ballot=self.ballot, + time__lte=self.time, + balloter=self.balloter, + ).values_list('send_email', flat=True) false = any( s==False for s in sent_list ) true = any( s==True for s in sent_list ) return True if true else False if false else None diff --git a/ietf/doc/tests_ballot.py b/ietf/doc/tests_ballot.py index 9db73becc..43cb2bd37 100644 --- a/ietf/doc/tests_ballot.py +++ b/ietf/doc/tests_ballot.py @@ -9,12 +9,16 @@ from pyquery import PyQuery import debug # pyflakes:ignore +from django.test import RequestFactory +from django.utils.text import slugify from django.urls import reverse as urlreverse -from ietf.doc.models import ( Document, State, DocEvent, - BallotPositionDocEvent, LastCallDocEvent, WriteupDocEvent, TelechatDocEvent ) -from ietf.doc.factories import DocumentFactory, IndividualDraftFactory, IndividualRfcFactory, WgDraftFactory +from ietf.doc.models import (Document, State, DocEvent, + BallotPositionDocEvent, LastCallDocEvent, WriteupDocEvent, TelechatDocEvent) +from ietf.doc.factories import (DocumentFactory, IndividualDraftFactory, IndividualRfcFactory, WgDraftFactory, + BallotPositionDocEventFactory, BallotDocEventFactory) from ietf.doc.utils import create_ballot_if_not_open +from ietf.doc.views_doc import document_ballot_content from ietf.group.models import Group, Role from ietf.group.factories import GroupFactory, RoleFactory, ReviewTeamFactory from ietf.ipr.factories import HolderIprDisclosureFactory @@ -22,6 +26,7 @@ from ietf.name.models import BallotPositionName from ietf.iesg.models import TelechatDate from ietf.person.models import Person, PersonalApiKey from ietf.person.factories import PersonFactory +from ietf.person.utils import get_active_ads from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.text import unwrap @@ -1100,7 +1105,7 @@ class RegenerateLastCallTestCase(TestCase): self.assertTrue("rfc6666" in lc_text) self.assertTrue("Independent Submission" in lc_text) - draft.relateddocument_set.create(target=rfc.docalias.get(name='rfc6666'),relationship_id='downref-approval') + draft.relateddocument_set.create(target=rfc.docalias.get(name='rfc6666'), relationship_id='downref-approval') r = self.client.post(url, dict(regenerate_last_call_text="1")) self.assertEqual(r.status_code, 200) @@ -1108,3 +1113,270 @@ class RegenerateLastCallTestCase(TestCase): lc_text = draft.latest_event(WriteupDocEvent, type="changed_last_call_text").text self.assertFalse("contains these normative down" in lc_text) self.assertFalse("rfc6666" in lc_text) + + +class BallotContentTests(TestCase): + def test_ballotpositiondocevent_any_email_sent(self): + now = datetime.datetime.now() # be sure event timestamps are at distinct times + bpde_with_null_send_email = BallotPositionDocEventFactory( + time=now - datetime.timedelta(minutes=30), + send_email=None, + ) + ballot = bpde_with_null_send_email.ballot + balloter = bpde_with_null_send_email.balloter + self.assertIsNone( + bpde_with_null_send_email.any_email_sent(), + 'Result is None when only send_email is None', + ) + + self.assertIsNone( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=29), + send_email=None, + ).any_email_sent(), + 'Result is None when all send_email values are None', + ) + + # test with assertIs instead of assertFalse to distinguish None from False + self.assertIs( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=28), + send_email=False, + ).any_email_sent(), + False, + 'Result is False when current send_email is False' + ) + + self.assertIs( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=27), + send_email=None, + ).any_email_sent(), + False, + 'Result is False when earlier send_email is False' + ) + + self.assertIs( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=26), + send_email=True, + ).any_email_sent(), + True, + 'Result is True when current send_email is True' + ) + + self.assertIs( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=25), + send_email=None, + ).any_email_sent(), + True, + 'Result is True when earlier send_email is True and current is None' + ) + + self.assertIs( + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloter, + time=now - datetime.timedelta(minutes=24), + send_email=False, + ).any_email_sent(), + True, + 'Result is True when earlier send_email is True and current is False' + ) + + def _assertBallotMessage(self, q, balloter, expected): + heading = q(f'h4[id$="_{slugify(balloter.plain_name())}"]') + self.assertEqual(len(heading), 1) + #
is followed by a panel with the message of interest, so use next() + self.assertEqual( + len(heading.next().find( + f'*[title="{expected}"]' + )), + 1, + ) + + def test_document_ballot_content_email_sent(self): + """Ballot content correctly describes whether email is requested for each position""" + ballot = BallotDocEventFactory() + balloters = get_active_ads() + self.assertGreaterEqual(len(balloters), 6, + 'Oops! Need to create additional active balloters for test') + + # send_email is True + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[0], + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=True, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[1], + pos_id='noobj', + comment='Commentary', + comment_time=datetime.datetime.now(), + send_email=True, + ) + + # send_email False + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[2], + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=False, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[3], + pos_id='noobj', + comment='Commentary', + comment_time=datetime.datetime.now(), + send_email=False, + ) + + # send_email False but earlier position had send_email True + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[4], + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now() - datetime.timedelta(days=1), + send_email=True, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[4], + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=False, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[5], + pos_id='noobj', + comment='Commentary', + comment_time=datetime.datetime.now() - datetime.timedelta(days=1), + send_email=True, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[5], + pos_id='noobj', + comment='Commentary', + comment_time=datetime.datetime.now(), + send_email=False, + ) + + # Create a few positions with non-active-ad people. These will be treated + # as "old" ballot positions because the people are not in the list returned + # by get_active_ads() + # + # Some faked non-ASCII names wind up with plain names that cannot be slugified. + # This causes test failure because that slug is used in an HTML element ID. + # Until that's fixed, set the plain names to something guaranteed unique so + # the test does not randomly fail. + no_email_balloter = BallotPositionDocEventFactory( + ballot=ballot, + balloter__plain='plain name1', + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=False, + ).balloter + send_email_balloter = BallotPositionDocEventFactory( + ballot=ballot, + balloter__plain='plain name2', + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=True, + ).balloter + prev_send_email_balloter = BallotPositionDocEventFactory( + ballot=ballot, + balloter__plain='plain name3', + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now() - datetime.timedelta(days=1), + send_email=True, + ).balloter + BallotPositionDocEventFactory( + ballot=ballot, + balloter=prev_send_email_balloter, + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=False, + ) + + content = document_ballot_content( + request=RequestFactory(), + doc=ballot.doc, + ballot_id=ballot.pk, + ) + q = PyQuery(content) + self._assertBallotMessage(q, balloters[0], 'Email requested to be sent for this discuss') + self._assertBallotMessage(q, balloters[1], 'Email requested to be sent for this comment') + self._assertBallotMessage(q, balloters[2], 'No email send requests for this discuss') + self._assertBallotMessage(q, balloters[3], 'No email send requests for this comment') + self._assertBallotMessage(q, balloters[4], 'Email requested to be sent for earlier discuss') + self._assertBallotMessage(q, balloters[5], 'Email requested to be sent for earlier comment') + self._assertBallotMessage(q, no_email_balloter, 'No email send requests for this ballot position') + self._assertBallotMessage(q, send_email_balloter, 'Email requested to be sent for this ballot position') + self._assertBallotMessage(q, prev_send_email_balloter, 'Email requested to be sent for earlier ballot position') + + def test_document_ballot_content_without_send_email_values(self): + """Ballot content correctly indicates lack of send_email field in records""" + ballot = BallotDocEventFactory() + balloters = get_active_ads() + self.assertGreaterEqual(len(balloters), 2, + 'Oops! Need to create additional active balloters for test') + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[0], + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=None, + ) + BallotPositionDocEventFactory( + ballot=ballot, + balloter=balloters[1], + pos_id='noobj', + comment='Commentary', + comment_time=datetime.datetime.now(), + send_email=None, + ) + old_balloter = BallotPositionDocEventFactory( + ballot=ballot, + balloter__plain='plain name', # ensure plain name is slugifiable + pos_id='discuss', + discuss='Discussion text', + discuss_time=datetime.datetime.now(), + send_email=None, + ).balloter + + content = document_ballot_content( + request=RequestFactory(), + doc=ballot.doc, + ballot_id=ballot.pk, + ) + q = PyQuery(content) + self._assertBallotMessage(q, balloters[0], 'No email send requests for this discuss') + self._assertBallotMessage(q, balloters[1], 'No ballot position send log available') + self._assertBallotMessage(q, old_balloter, 'No ballot position send log available') diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 5d4211de6..db3f381e1 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -1177,6 +1177,10 @@ def document_ballot_content(request, doc, ballot_id, editable=True): positions = ballot.all_positions() # put into position groups + # + # Each position group is a tuple (BallotPositionName, [BallotPositionDocEvent, ...]) + # THe list contains the latest entry for each AD, possibly with a fake 'no record' entry + # for any ADs without an event. Blocking positions are earlier in the list than non-blocking. position_groups = [] for n in BallotPositionName.objects.filter(slug__in=[p.pos_id for p in positions]).order_by('order'): g = (n, [p for p in positions if p.pos_id == n.slug])