From 51d0dfb504a3e14904c1e89e0eb0bc5e7dd42100 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 20 Sep 2024 15:28:18 -0700 Subject: [PATCH] docs: Tweak some documentation around send_event. --- docs/subsystems/events-system.md | 5 +++++ docs/tutorials/new-feature-tutorial.md | 12 +++++++----- docs/tutorials/writing-views.md | 6 +++--- zerver/lib/test_classes.py | 17 ++++++++++++----- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/docs/subsystems/events-system.md b/docs/subsystems/events-system.md index 623587980c..76207f0164 100644 --- a/docs/subsystems/events-system.md +++ b/docs/subsystems/events-system.md @@ -77,6 +77,11 @@ example, an event containing direct message content is sent to the entire organization. However, if an event isn't sent to enough clients, there will likely be user-visible real-time sync bugs. +As indicated in the name, `send_event_on_commit` is intended to be +called inside the database transaction that is changing the state +itself; this design ensures that the event is only sent if the +transaction successfully commits. + Most of the hard work in event generation is about defining consistent event dictionaries that are clear, readable, will be useful to the wide range of possible clients, and make it easy for developers. diff --git a/docs/tutorials/new-feature-tutorial.md b/docs/tutorials/new-feature-tutorial.md index be8fda0b0e..032a21afbe 100644 --- a/docs/tutorials/new-feature-tutorial.md +++ b/docs/tutorials/new-feature-tutorial.md @@ -277,11 +277,13 @@ first contacts the server, the server sends the client its initial state. Subsequently, clients subscribe to "events," which can (among other things) indicate that settings have changed. -For the backend piece, we will need our action to make a call to `send_event_on_commit` -to send the event to clients that are active. We will also need to -modify `fetch_initial_state_data` so that the new field is passed to -clients. See [our event system docs](../subsystems/events-system.md) for all the -gory details. +For the backend piece, we will need our action to make a call to +`send_event_on_commit` to send the event to clients that are active +(The event is only sent after the current database transaction +commits, hence the name). We will also need to modify +`fetch_initial_state_data` so that the new field is passed to +clients. See [our event system docs](../subsystems/events-system.md) +for all the gory details. Anyway, getting back to implementation details... diff --git a/docs/tutorials/writing-views.md b/docs/tutorials/writing-views.md index 1cbbe1b67a..72cc82d846 100644 --- a/docs/tutorials/writing-views.md +++ b/docs/tutorials/writing-views.md @@ -332,9 +332,9 @@ def update_realm( ``` `realm.save()` actually saves the changes to the realm to the -database, and `send_event_on_commit` sends the event to active clients belonging -to the provided list of users (in this case, all active users in the -Zulip realm). +database, and `send_event_on_commit` sends the event to active clients +belonging to the provided list of users (in this case, all active +users in the Zulip realm), once the current transaction completes. ### Calling from the web application diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index ae77de0643..a50788babe 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -2052,11 +2052,18 @@ class ZulipTestCase(ZulipTestCaseMixin, TestCase): # So explicitly change parameter name to 'notice' to work around this problem with ( mock.patch("zerver.tornado.event_queue.process_notification", lst.append), - # Some `send_event_rollback_unsafe` calls need to be executed only after the - # current transaction commits (using `on_commit` hooks). Because the transaction - # in Django tests never commits (rather, gets rolled back after the test completes), - # such events would never be sent in tests, and we would be unable to verify them. - # Hence, we use this helper to make sure the `send_event_rollback_unsafe` calls actually run. + # Some `send_event_rollback_unsafe` calls need to be + # executed only after the current transaction commits + # (mainly those using the `send_event_on_commit` wrapper, which + # sends the actual event inside an `on_commit` hook). + # + # Because the outer transaction in Django tests never + # commits (it gets rolled back when the test completes + # to restore the database to the desired state for the + # next test), such events would never be sent in + # tests, and we would be unable to verify them. + # Hence, we use this helper to make sure the + # `send_event_rollback_unsafe` calls actually run. self.captureOnCommitCallbacks(execute=True), ): yield lst