diff --git a/ietf/community/models.py b/ietf/community/models.py index b1295461d6..4d820eb0f1 100644 --- a/ietf/community/models.py +++ b/ietf/community/models.py @@ -104,12 +104,15 @@ def notify_events(sender, instance, **kwargs): if not isinstance(instance, DocEvent): return + if not kwargs.get("created", False): + return # only notify on creation + if instance.doc.type_id != 'draft': return if getattr(instance, "skip_community_list_notification", False): return - + # kludge alert: queuing a celery task in response to a signal can cause unexpected attempts to # start a Celery task during tests. To prevent this, don't queue a celery task if we're running # tests. diff --git a/ietf/community/tests.py b/ietf/community/tests.py index 84e4370771..181e9e0fa6 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.py @@ -17,6 +17,7 @@ import ietf.community.views from ietf.group.models import Group from ietf.group.utils import setup_default_community_list_for_group +from ietf.doc.factories import DocumentFactory from ietf.doc.models import State from ietf.doc.utils import add_state_change_event from ietf.person.models import Person, Email, Alias @@ -439,39 +440,45 @@ def test_notification_signal_receiver(self, mock_notify_task): This implicitly tests that notify_events is hooked up to the post_save signal. """ # Arbitrary model that's not a DocEvent - p = PersonFactory() + person = PersonFactory() mock_notify_task.reset_mock() # clear any calls that resulted from the factories # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): - p.save() + person.save() self.assertFalse(mock_notify_task.delay.called) - d = DocEventFactory() - mock_notify_task.reset_mock() # clear any calls that resulted from the factories + # build a DocEvent that is not yet persisted + doc = DocumentFactory() + d = DocEventFactory.build(by=person, doc=doc) + # mock_notify_task.reset_mock() # clear any calls that resulted from the factories # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): d.save() - self.assertEqual(mock_notify_task.delay.call_count, 1) + self.assertEqual(mock_notify_task.delay.call_count, 1, "notify_task should be run on creation of DocEvent") self.assertEqual(mock_notify_task.delay.call_args, mock.call(event_id = d.pk)) mock_notify_task.reset_mock() + with override_settings(SERVER_MODE="not-test"): + d.save() + self.assertFalse(mock_notify_task.delay.called, "notify_task should not be run save of on existing DocEvent") + + mock_notify_task.reset_mock() + d = DocEventFactory.build(by=person, doc=doc) d.skip_community_list_notification = True # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): d.save() - self.assertFalse(mock_notify_task.delay.called) + self.assertFalse(mock_notify_task.delay.called, "notify_task should not run when skip_community_list_notification is set") - del(d.skip_community_list_notification) - d.doc.type_id="rfc" # not "draft" - d.doc.save() + d = DocEventFactory.build(by=person, doc=DocumentFactory(type_id="rfc")) # be careful overriding SERVER_MODE - we do it here because the method # under test does not make this call when in "test" mode with override_settings(SERVER_MODE="not-test"): d.save() - self.assertFalse(mock_notify_task.delay.called) + self.assertFalse(mock_notify_task.delay.called, "notify_task should not run on a document with type 'rfc'") @mock.patch("ietf.utils.mail.send_mail_text") def test_notify_event_to_subscribers(self, mock_send_mail_text): diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 74000e598b..97243a20d6 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -398,8 +398,12 @@ def get_unicode_document_content(key, filename, codec='utf-8', errors='ignore'): def tags_suffix(tags): return ("::" + "::".join(t.name for t in tags)) if tags else "" -def add_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_tags=None, timestamp=None): - """Add doc event to explain that state change just happened.""" + +def new_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_tags=None, timestamp=None): + """Create unsaved doc event to explain that state change just happened + + Returns None if no state change occurred. + """ if prev_state and new_state: assert prev_state.type_id == new_state.type_id @@ -419,7 +423,22 @@ def add_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_t e.desc += " from %s" % (prev_state.name + tags_suffix(prev_tags)) if timestamp: e.time = timestamp - e.save() + return e # not saved! + + +def add_state_change_event(doc, by, prev_state, new_state, prev_tags=None, new_tags=None, timestamp=None): + """Add doc event to explain that state change just happened. + + Returns None if no state change occurred. + + Note: Creating a state change DocEvent will trigger notifications to be sent to people subscribed + to the doc via a CommunityList on its first save(). If you need to adjust the event (say, changing + its desc) before that notification is sent, use new_state_change_event() instead and save the + event after making your changes. + """ + e = new_state_change_event(doc, by, prev_state, new_state, prev_tags, new_tags, timestamp) + if e is not None: + e.save() return e diff --git a/ietf/sync/rfceditor.py b/ietf/sync/rfceditor.py index d3371ea36c..6b3482f10d 100644 --- a/ietf/sync/rfceditor.py +++ b/ietf/sync/rfceditor.py @@ -21,7 +21,7 @@ from ietf.doc.models import ( Document, State, StateType, DocEvent, DocRelationshipName, DocTagName, RelatedDocument, RelatedDocHistory ) from ietf.doc.expire import move_draft_files_to_archive -from ietf.doc.utils import add_state_change_event, prettify_std_name, update_action_holders +from ietf.doc.utils import add_state_change_event, new_state_change_event, prettify_std_name, update_action_holders from ietf.group.models import Group from ietf.ipr.models import IprDocRel from ietf.name.models import StdLevelName, StreamName @@ -202,11 +202,14 @@ def update_drafts_from_queue(drafts): if prev_state != next_state: d.set_state(next_state) - e = add_state_change_event(d, system, prev_state, next_state) + e = new_state_change_event(d, system, prev_state, next_state) # unsaved + if e: + if auth48: + e.desc = re.sub(r"(.*)", "\\1" % auth48, e.desc) + e.save() + events.append(e) if auth48: - e.desc = re.sub(r"(.*)", "\\1" % auth48, e.desc) - e.save() # Create or update the auth48 URL whether or not this is a state expected to have one. d.documenturl_set.update_or_create( tag_id='auth48', # look up existing based on this field @@ -215,8 +218,6 @@ def update_drafts_from_queue(drafts): else: # Remove any existing auth48 URL when an update does not have one. d.documenturl_set.filter(tag_id='auth48').delete() - if e: - events.append(e) changed.add(name)