altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 4 Packages Projects Releases Wiki Activity

fix: keep draft-iesg state on expiration. Update action holders. (#8321)

Browse source 
* fix: keep draft-iesg state on expiration. Update action holders

* feat: task to repair docs in dead because expiry

* fix: restore all to-date flows through update_action_holders

* fix: Fetch the System user following more regular conventions

* fix: better signal test
This commit is contained in:
Robert Sparks 2024-12-13 11:48:19 -06:00 committed by GitHub
parent 70ab711216
commit c747e97201
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 184 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
ietf/doc/expire.py
View file

@ -3,6 +3,8 @@
# expiry of Internet-Drafts
import debug # pyflakes:ignore
from django.conf import settings
from django.utils import timezone
@ -11,12 +13,12 @@ from pathlib import Path
from typing import List, Optional # pyflakes:ignore
from ietf.doc.utils import new_state_change_event, update_action_holders
from ietf.utils import log
from ietf.utils.mail import send_mail
from ietf.doc.models import Document, DocEvent, State, IESG_SUBSTATE_TAGS
from ietf.doc.models import Document, DocEvent, State, StateDocEvent
from ietf.person.models import Person
from ietf.meeting.models import Meeting
from ietf.doc.utils import add_state_change_event, update_action_holders
from ietf.mailtrigger.utils import gather_address_lists
from ietf.utils.timezone import date_today, datetime_today, DEADLINE_TZINFO
@ -161,24 +163,11 @@ def expire_draft(doc):
events = []
# change the state
if doc.latest_event(type='started_iesg_process'):
new_state = State.objects.get(used=True, type="draft-iesg", slug="dead")
prev_state = doc.get_state(new_state.type_id)
prev_tags = doc.tags.filter(slug__in=IESG_SUBSTATE_TAGS)
if new_state != prev_state:
doc.set_state(new_state)
doc.tags.remove(*prev_tags)
e = add_state_change_event(doc, system, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
if e:
events.append(e)
e = update_action_holders(doc, prev_state, new_state, prev_tags=prev_tags, new_tags=[])
if e:
events.append(e)
events.append(DocEvent.objects.create(doc=doc, rev=doc.rev, by=system, type="expired_document", desc="Document has expired"))
prev_draft_state=doc.get_state("draft")
doc.set_state(State.objects.get(used=True, type="draft", slug="expired"))
events.append(update_action_holders(doc, prev_draft_state, doc.get_state("draft"),[],[]))
doc.save_with_history(events)
def clean_up_draft_files():
@ -238,3 +227,42 @@ def clean_up_draft_files():
except Document.DoesNotExist:
# All uses of this past 2014 seem related to major system failures.
move_file_to("unknown_ids")
def repair_dead_on_expire():
by = Person.objects.get(name="(System)")
id_exists = State.objects.get(type="draft-iesg", slug="idexists")
dead = State.objects.get(type="draft-iesg", slug="dead")
dead_drafts = Document.objects.filter(
states__type="draft-iesg", states__slug="dead", type_id="draft"
)
for d in dead_drafts:
dead_event = d.latest_event(
StateDocEvent, state_type="draft-iesg", state__slug="dead"
)
if dead_event is not None:
if d.docevent_set.filter(type="expired_document").exists():
closest_expiry = min(
[
abs(e.time - dead_event.time)
for e in d.docevent_set.filter(type="expired_document")
]
)
if closest_expiry.total_seconds() < 60:
d.set_state(id_exists)
events = []
e = DocEvent(
doc=d,
rev=d.rev,
type="added_comment",
by=by,
desc="IESG Dead state was set due only to document expiry - changing IESG state to ID-Exists",
)
e.skip_community_list_notification = True
e.save()
events.append(e)
e = new_state_change_event(d, by, dead, id_exists)
e.skip_community_list_notification = True
e.save()
events.append(e)
d.save_with_history(events)

6
ietf/doc/tasks.py
View file

@ -18,6 +18,7 @@ from .expire import (
in_draft_expire_freeze,
get_expired_drafts,
expirable_drafts,
repair_dead_on_expire,
send_expire_notice_for_draft,
expire_draft,
clean_up_draft_files,
@ -61,6 +62,11 @@ def expire_ids_task():
raise
@shared_task
def repair_dead_on_expire_task():
repair_dead_on_expire()
@shared_task
def notify_expirations_task(notify_days=14):
for doc in get_soon_to_expire_drafts(notify_days):

86
ietf/doc/tests_draft.py
View file

@ -19,10 +19,10 @@ from django.utils.html import escape
import debug # pyflakes:ignore
from ietf.doc.expire import get_expired_drafts, send_expire_notice_for_draft, expire_draft
from ietf.doc.factories import EditorialDraftFactory, IndividualDraftFactory, WgDraftFactory, RgDraftFactory, DocEventFactory
from ietf.doc.expire import expirable_drafts, get_expired_drafts, repair_dead_on_expire, send_expire_notice_for_draft, expire_draft
from ietf.doc.factories import EditorialDraftFactory, IndividualDraftFactory, StateDocEventFactory, WgDraftFactory, RgDraftFactory, DocEventFactory
from ietf.doc.models import ( Document, DocReminder, DocEvent,
ConsensusDocEvent, LastCallDocEvent, RelatedDocument, State, TelechatDocEvent,
ConsensusDocEvent, LastCallDocEvent, RelatedDocument, State, StateDocEvent, TelechatDocEvent,
WriteupDocEvent, DocRelationshipName, IanaExpertDocEvent )
from ietf.doc.utils import get_tags_for_stream_id, create_ballot_if_not_open
from ietf.doc.views_draft import AdoptDraftForm
@ -36,7 +36,7 @@ from ietf.iesg.models import TelechatDate
from ietf.utils.test_utils import login_testing_unauthorized
from ietf.utils.mail import outbox, empty_outbox, get_payload_text
from ietf.utils.test_utils import TestCase
from ietf.utils.timezone import date_today, datetime_from_date, DEADLINE_TZINFO
from ietf.utils.timezone import date_today, datetime_today, datetime_from_date, DEADLINE_TZINFO
class ChangeStateTests(TestCase):
@ -763,13 +763,16 @@ class ExpireIDsTests(DraftFileMixin, TestCase):
txt = "%s-%s.txt" % (draft.name, draft.rev)
self.write_draft_file(txt, 5000)
self.assertFalse(expirable_drafts(Document.objects.filter(pk=draft.pk)).exists())
draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="idexists"))
self.assertTrue(expirable_drafts(Document.objects.filter(pk=draft.pk)).exists())
expire_draft(draft)
draft = Document.objects.get(name=draft.name)
self.assertEqual(draft.get_state_slug(), "expired")
self.assertEqual(draft.get_state_slug("draft-iesg"), "dead")
self.assertEqual(draft.get_state_slug("draft-iesg"), "idexists")
self.assertTrue(draft.latest_event(type="expired_document"))
self.assertCountEqual(draft.action_holders.all(), [])
self.assertEqual(draft.action_holders.count(), 0)
self.assertIn('Removed all action holders', draft.latest_event(type='changed_action_holders').desc)
self.assertTrue(not os.path.exists(os.path.join(settings.INTERNET_DRAFT_PATH, txt)))
self.assertTrue(os.path.exists(os.path.join(settings.INTERNET_DRAFT_ARCHIVE_DIR, txt)))
@ -842,6 +845,77 @@ class ExpireIDsTests(DraftFileMixin, TestCase):
self.assertTrue(not os.path.exists(os.path.join(settings.INTERNET_DRAFT_PATH, txt)))
self.assertTrue(os.path.exists(os.path.join(settings.INTERNET_DRAFT_ARCHIVE_DIR, txt)))
@mock.patch("ietf.community.signals.notify_of_event")
def test_repair_dead_on_expire(self, mock_notify):
# Create a draft in iesg idexists - ensure it doesn't get new docevents.
# Create a draft in iesg dead with no expires within the window - ensure it doesn't get new docevents and its state doesn't change.
# Create a draft in iesg dead with an expiry in the window - ensure it gets the right doc events, iesg state changes, draft state doesn't change.
last_year = datetime_today() - datetime.timedelta(days=365)
not_dead = WgDraftFactory(name="draft-not-dead")
not_dead_event_count = not_dead.docevent_set.count()
dead_not_from_expires = WgDraftFactory(name="draft-dead-not-from-expiring")
dead_not_from_expires.set_state(
State.objects.get(type="draft-iesg", slug="dead")
)
StateDocEventFactory(
doc=dead_not_from_expires, state=("draft-iesg", "dead"), time=last_year
)
DocEventFactory(
doc=dead_not_from_expires,
type="expired_document",
time=last_year + datetime.timedelta(days=1),
)
dead_not_from_expires_event_count = dead_not_from_expires.docevent_set.count()
dead_from_expires = []
dead_from_expires_event_count = dict()
for delta in [-5, 5]:
d = WgDraftFactory(
name=f"draft-dead-from-expiring-just-{'before' if delta<0 else 'after'}"
)
d.set_state(State.objects.get(type="draft-iesg", slug="dead"))
StateDocEventFactory(doc=d, state=("draft-iesg", "dead"), time=last_year)
DocEventFactory(
doc=d,
type="expired_document",
time=last_year + datetime.timedelta(seconds=delta),
)
dead_from_expires.append(d)
dead_from_expires_event_count[d] = d.docevent_set.count()
notified_during_factory_work = mock_notify.call_count
for call_args in mock_notify.call_args_list:
e = call_args.args[0]
self.assertTrue(isinstance(e,DocEvent))
self.assertFalse(hasattr(e,"skip_community_list_notification"))
repair_dead_on_expire()
self.assertEqual(not_dead.docevent_set.count(), not_dead_event_count)
self.assertEqual(
dead_not_from_expires.docevent_set.count(),
dead_not_from_expires_event_count,
)
for d in dead_from_expires:
self.assertEqual(
d.docevent_set.count(), dead_from_expires_event_count[d] + 2
)
self.assertIn(
"due only to document expiry", d.latest_event(type="added_comment").desc
)
self.assertEqual(
d.latest_event(StateDocEvent).desc,
"IESG state changed to <b>I-D Exists</b> from Dead",
)
self.assertEqual(mock_notify.call_count, 4+notified_during_factory_work)
for call_args in mock_notify.call_args_list[-4:]:
e = call_args.args[0]
self.assertTrue(isinstance(e,DocEvent))
self.assertTrue(hasattr(e,"skip_community_list_notification"))
self.assertTrue(e.skip_community_list_notification)
class ExpireLastCallTests(TestCase):
def test_expire_last_call(self):

7
ietf/doc/tests_tasks.py
View file

@ -1,4 +1,6 @@
# Copyright The IETF Trust 2024, All Rights Reserved
import debug # pyflakes:ignore
import datetime
import mock
@ -19,6 +21,7 @@ from .tasks import (
generate_idnits2_rfcs_obsoleted_task,
generate_idnits2_rfc_status_task,
notify_expirations_task,
repair_dead_on_expire_task,
)
class TaskTests(TestCase):
@ -96,6 +99,10 @@ class TaskTests(TestCase):
self.assertEqual(mock_expire.call_args_list[1], mock.call(docs[1]))
self.assertEqual(mock_expire.call_args_list[2], mock.call(docs[2]))
@mock.patch("ietf.doc.tasks.repair_dead_on_expire")
def test_repair_dead_on_expire_task(self, mock_repair):
repair_dead_on_expire_task()
self.assertEqual(mock_repair.call_count, 1)
class Idnits2SupportTests(TestCase):
settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['DERIVED_DIR']

76
ietf/doc/utils.py
View file

@ -491,8 +491,9 @@ def update_action_holders(doc, prev_state=None, new_state=None, prev_tags=None,
Returns an event describing the change which should be passed to doc.save_with_history()
Only cares about draft-iesg state changes. Places where other state types are updated
may not call this method. If you add rules for updating action holders on other state
Only cares about draft-iesg state changes and draft expiration.
Places where other state types are updated may not call this method.
If you add rules for updating action holders on other state
types, be sure this is called in the places that change that state.
"""
# Should not call this with different state types
@ -512,40 +513,49 @@ def update_action_holders(doc, prev_state=None, new_state=None, prev_tags=None,
# Remember original list of action holders to later check if it changed
prev_set = list(doc.action_holders.all())
# Update the action holders. To get this right for people with more
# than one relationship to the document, do removals first, then adds.
# Remove outdated action holders
iesg_state_changed = (prev_state != new_state) and (getattr(new_state, "type_id", None) == "draft-iesg")
if iesg_state_changed:
# Clear the action_holders list on a state change. This will reset the age of any that get added back.
if new_state and new_state.type_id=="draft" and new_state.slug=="expired":
doc.action_holders.clear()
if tags.removed("need-rev"):
# Removed the 'need-rev' tag - drop authors from the action holders list
DocumentActionHolder.objects.filter(document=doc, person__in=doc.authors()).delete()
elif tags.added("need-rev"):
# Remove the AD if we're asking for a new revision
DocumentActionHolder.objects.filter(document=doc, person=doc.ad).delete()
return add_action_holder_change_event(
doc,
Person.objects.get(name='(System)'),
prev_set,
reason='draft expired',
)
else:
# Update the action holders. To get this right for people with more
# than one relationship to the document, do removals first, then adds.
# Remove outdated action holders
iesg_state_changed = (prev_state != new_state) and (getattr(new_state, "type_id", None) == "draft-iesg")
if iesg_state_changed:
# Clear the action_holders list on a state change. This will reset the age of any that get added back.
doc.action_holders.clear()
if tags.removed("need-rev"):
# Removed the 'need-rev' tag - drop authors from the action holders list
DocumentActionHolder.objects.filter(document=doc, person__in=doc.authors()).delete()
elif tags.added("need-rev"):
# Remove the AD if we're asking for a new revision
DocumentActionHolder.objects.filter(document=doc, person=doc.ad).delete()
# Add new action holders
if doc.ad:
# AD is an action holder unless specified otherwise for the new state
if iesg_state_changed and new_state.slug not in DocumentActionHolder.CLEAR_ACTION_HOLDERS_STATES:
doc.action_holders.add(doc.ad)
# If AD follow-up is needed, make sure they are an action holder
if tags.added("ad-f-up"):
doc.action_holders.add(doc.ad)
# Authors get the action if a revision is needed
if tags.added("need-rev"):
for auth in doc.authors():
doc.action_holders.add(auth)
# Add new action holders
if doc.ad:
# AD is an action holder unless specified otherwise for the new state
if iesg_state_changed and new_state.slug not in DocumentActionHolder.CLEAR_ACTION_HOLDERS_STATES:
doc.action_holders.add(doc.ad)
# If AD follow-up is needed, make sure they are an action holder
if tags.added("ad-f-up"):
doc.action_holders.add(doc.ad)
# Authors get the action if a revision is needed
if tags.added("need-rev"):
for auth in doc.authors():
doc.action_holders.add(auth)
# Now create an event if we changed the set
return add_action_holder_change_event(
doc,
Person.objects.get(name='(System)'),
prev_set,
reason='IESG state changed',
)
# Now create an event if we changed the set
return add_action_holder_change_event(
doc,
Person.objects.get(name='(System)'),
prev_set,
reason='IESG state changed',
)
def update_documentauthors(doc, new_docauthors, by=None, basis=None):

3
ietf/doc/views_draft.py
View file

@ -95,7 +95,8 @@ def change_state(request, name):
and logging the change as a comment."""
doc = get_object_or_404(Document, name=name)
if (not doc.latest_event(type="started_iesg_process")) or doc.get_state_slug() == "expired":
# Steer ADs towards "Begin IESG Processing"
if doc.get_state_slug("draft-iesg")=="idexists" and not has_role(request.user,"Secretariat"):
raise Http404
login = request.user.person