From 66687a5a3715adf31e78af0efb76ee4ffee13949 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@painless-security.com>
Date: Wed, 12 Jan 2022 20:21:41 +0000
Subject: [PATCH] Update any_email_sent() to use balloters instead of old ad
field. Add tests to catch the otherwise quiet failure. Fixes #3438. Commit
ready for merge. - Legacy-Id: 19837
---
ietf/doc/factories.py | 8 +-
ietf/doc/models.py | 6 +-
ietf/doc/tests_ballot.py | 280 ++++++++++++++++++++++++++++++++++++++-
ietf/doc/views_doc.py | 4 +
4 files changed, 287 insertions(+), 11 deletions(-)
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)
+ # <h4/> 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])