diff --git a/ietf/doc/expire.py b/ietf/doc/expire.py index 681ca963a3..0581c9a03f 100644 --- a/ietf/doc/expire.py +++ b/ietf/doc/expire.py @@ -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 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 move_file_to(subdir): 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) diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py index f1de459dd8..b7f89e1f9b 100644 --- a/ietf/doc/tasks.py +++ b/ietf/doc/tasks.py @@ -18,6 +18,7 @@ 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): diff --git a/ietf/doc/tests_draft.py b/ietf/doc/tests_draft.py index 09a7b38999..84959625c9 100644 --- a/ietf/doc/tests_draft.py +++ b/ietf/doc/tests_draft.py @@ -19,10 +19,10 @@ 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.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 @@ def test_expire_drafts(self): 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 @@ def test_clean_up_draft_files(self): 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 I-D Exists 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): diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py index b75f58656b..135b52f604 100644 --- a/ietf/doc/tests_tasks.py +++ b/ietf/doc/tests_tasks.py @@ -1,4 +1,6 @@ # Copyright The IETF Trust 2024, All Rights Reserved + +import debug # pyflakes:ignore import datetime import mock @@ -19,6 +21,7 @@ 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 @@ def test_expire_last_calls_task(self, mock_get_expired, mock_expire): 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'] diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index b2bc0997b1..ff19dfbde6 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -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 @@ -511,41 +512,50 @@ 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() - - # 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', - ) + 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) + + # 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): diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index d709aedd42..34104b2005 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -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