Skip to content

Commit aec1ced

Browse files
silvestridCopilot
andauthored
fix(webhook): 'rows enter view' webhook shows stale rows when webhook is deleted and recreated (baserow#4438)
* Fix bug * Update enterprise/backend/src/baserow_enterprise/webhook_event_types.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address feedback --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent d1ff46a commit aec1ced

File tree

7 files changed

+93
-54
lines changed

7 files changed

+93
-54
lines changed

backend/src/baserow/contrib/database/views/handler.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3711,6 +3711,50 @@ def _get_prepared_values_for_data(
37113711

37123712

37133713
class ViewSubscriptionHandler:
3714+
@classmethod
3715+
def get_subscribed_views(cls, subscriber: django_models.Model) -> QuerySet[View]:
3716+
"""
3717+
Returns all views a subscriber is subscribed to.
3718+
3719+
:param subscriber: The subscriber to get the views for.
3720+
:return: A list of views the subscriber is subscribed to.
3721+
"""
3722+
3723+
return View.objects.filter(
3724+
id__in=ViewSubscription.objects.filter(
3725+
subscriber_content_type=ContentType.objects.get_for_model(subscriber),
3726+
subscriber_id=subscriber.pk,
3727+
).values("view_id")
3728+
)
3729+
3730+
@classmethod
3731+
def sync_view_rows(cls, views: list[View], model=None) -> list[ViewRows]:
3732+
"""
3733+
Updates or creates the ViewRows objects for the given views by executing
3734+
the view queries and storing the resulting row IDs.
3735+
3736+
:param views: The views for which to create ViewRows objects.
3737+
:param model: The table model to use for the views. If not provided, the model
3738+
will be generated automatically.
3739+
:return: A list of created or updated ViewRows objects.
3740+
"""
3741+
3742+
view_rows = []
3743+
for view in views:
3744+
row_ids = (
3745+
ViewHandler()
3746+
.get_queryset(None, view, model=model, apply_sorts=False)
3747+
.values_list("id", flat=True)
3748+
)
3749+
view_rows.append(ViewRows(view=view, row_ids=list(row_ids)))
3750+
3751+
return ViewRows.objects.bulk_create(
3752+
view_rows,
3753+
update_conflicts=True,
3754+
update_fields=["row_ids"],
3755+
unique_fields=["view_id"],
3756+
)
3757+
37143758
@classmethod
37153759
def subscribe_to_views(cls, subscriber: django_models.Model, views: list[View]):
37163760
"""
@@ -3723,7 +3767,7 @@ def subscribe_to_views(cls, subscriber: django_models.Model, views: list[View]):
37233767
"""
37243768

37253769
cls.notify_table_views_updates(views)
3726-
ViewRows.create_missing_for_views(views)
3770+
cls.sync_view_rows(views)
37273771

37283772
new_subscriptions = []
37293773
for view in views:

backend/src/baserow/contrib/database/views/models.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -977,34 +977,6 @@ class ViewRows(CreatedAndUpdatedOnMixin, models.Model):
977977
"to determine which rows have been changed since the last check.",
978978
)
979979

980-
@classmethod
981-
def create_missing_for_views(cls, views: list[View], model=None):
982-
"""
983-
Creates ViewRows objects for the given views if they don't already exist.
984-
985-
:param views: The views for which to create ViewRows objects.
986-
"""
987-
988-
from baserow.contrib.database.views.handler import ViewHandler
989-
990-
existing_view_ids = ViewRows.objects.filter(view__in=views).values_list(
991-
"view_id", flat=True
992-
)
993-
view_map = {view.id: view for view in views}
994-
missing_view_ids = list(set(view_map.keys()) - set(existing_view_ids))
995-
996-
view_rows = []
997-
for view_id in missing_view_ids:
998-
view = view_map[view_id]
999-
row_ids = (
1000-
ViewHandler()
1001-
.get_queryset(None, view, model=model, apply_sorts=False)
1002-
.values_list("id", flat=True)
1003-
)
1004-
view_rows.append(ViewRows(view=view, row_ids=list(row_ids)))
1005-
1006-
return ViewRows.objects.bulk_create(view_rows, ignore_conflicts=True)
1007-
1008980
def get_diff(self, model=None):
1009981
"""
1010982
Executes the view query and returns the current row IDs in the view,

backend/src/baserow/contrib/database/webhooks/handler.py

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,6 @@ def create_table_webhook(
255255
if event_config is not None and not values.get("include_all_events"):
256256
self._update_webhook_event_config(webhook, event_config, webhook_events)
257257

258-
for webhook_event in webhook_events:
259-
webhook_event_type = webhook_event.get_type()
260-
webhook_event_type.after_create(webhook_event)
261-
262258
if headers is not None:
263259
header_objects = []
264260
for key, value in headers.items():
@@ -324,7 +320,6 @@ def update_table_webhook(
324320
kwargs.get("include_all_events", False) and not old_include_all_events
325321
)
326322

327-
created_events = []
328323
if not should_update_events:
329324
TableWebhookEvent.objects.filter(webhook=webhook).delete()
330325
elif events is not None:
@@ -349,15 +344,11 @@ def update_table_webhook(
349344
]
350345

351346
if len(events_to_create) > 0:
352-
created_events = TableWebhookEvent.objects.bulk_create(events_to_create)
347+
TableWebhookEvent.objects.bulk_create(events_to_create)
353348

354349
if event_config is not None and should_update_events:
355350
self._update_webhook_event_config(webhook, event_config)
356351

357-
for webhook_event in created_events:
358-
webhook_event_type = webhook_event.get_type()
359-
webhook_event_type.after_create(webhook_event)
360-
361352
if headers is not None:
362353
existing_headers = webhook.headers.all()
363354

backend/src/baserow/contrib/database/webhooks/registries.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,6 @@ def listener_after_commit(self, **kwargs):
214214
except SkipWebhookCall:
215215
pass
216216

217-
def after_create(self, webhook_event: TableWebhookEvent):
218-
"""
219-
This method is called after a webhook event has been created. By default it
220-
does nothing, but can be overwritten to add additional functionality.
221-
222-
:param webhook_event: The created webhook event.
223-
"""
224-
225217
def after_update(self, webhook_event: TableWebhookEvent):
226218
"""
227219
This method is called after a webhook event has been updated. By default it
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"type": "bug",
3+
"message": "Fix an issue where \"Rows enter view\" webhook shows stale rows when webhook is deleted and recreated",
4+
"issue_origin": "github",
5+
"issue_number": 4437,
6+
"domain": "database",
7+
"bullet_points": [],
8+
"created_at": "2025-12-12"
9+
}

enterprise/backend/src/baserow_enterprise/webhook_event_types.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,18 @@ def get_payload(self, event_id, webhook, view, row_ids, **kwargs):
121121
def after_update(self, webhook_event):
122122
# This is called also during webhook creation, when setting the
123123
# webhook_event_config
124-
ViewSubscriptionHandler.unsubscribe_from_views(webhook_event)
125-
views = webhook_event.views.all()
126-
if views:
127-
ViewSubscriptionHandler.subscribe_to_views(webhook_event, views)
124+
subscribed_views = ViewSubscriptionHandler.get_subscribed_views(webhook_event)
125+
requested_views = webhook_event.views.all()
126+
127+
subscriptions_to_add = set(requested_views) - set(subscribed_views)
128+
if subscriptions_to_add:
129+
# Automatically initialize the current rows in the views subscriptions
130+
ViewSubscriptionHandler.subscribe_to_views(
131+
webhook_event, list(subscriptions_to_add)
132+
)
133+
134+
subscriptions_to_delete = set(subscribed_views) - set(requested_views)
135+
if subscriptions_to_delete:
136+
ViewSubscriptionHandler.unsubscribe_from_views(
137+
webhook_event, list(subscriptions_to_delete)
138+
)

enterprise/backend/tests/baserow_enterprise_tests/webhooks/test_webhook_event_types.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414

1515
@pytest.mark.django_db()
1616
def test_rows_enter_view_event_type(enterprise_data_fixture):
17+
"""
18+
Test the payload structure for the view.rows_entered webhook event type.
19+
"""
20+
1721
user = enterprise_data_fixture.create_user()
1822
table = enterprise_data_fixture.create_database_table(user=user)
1923
view = enterprise_data_fixture.create_grid_view(table=table)
@@ -44,7 +48,6 @@ def test_rows_enter_view_event_type(enterprise_data_fixture):
4448
"name": table.name,
4549
"database_id": table.database_id,
4650
},
47-
"type": "grid",
4851
"filters_disabled": False,
4952
"show_logo": True,
5053
"allow_public_export": False,
@@ -127,6 +130,10 @@ def test_rows_enter_view_event_type(enterprise_data_fixture):
127130
def test_rows_enter_view_event_type_require_enterprise_license(
128131
mock_call_webhook, enterprise_data_fixture, synced_roles
129132
):
133+
"""
134+
Test that the view.rows_entered event only triggers with an enterprise license.
135+
"""
136+
130137
user = enterprise_data_fixture.create_user()
131138
table = enterprise_data_fixture.create_database_table(user=user)
132139
view = enterprise_data_fixture.create_grid_view(table=table)
@@ -166,6 +173,8 @@ def test_rows_enter_view_event_type_require_enterprise_license(
166173
def test_rows_enter_view_event_type_not_triggerd_with_include_all_events(
167174
mock_call_webhook, enterprise_data_fixture, enable_enterprise, synced_roles
168175
):
176+
"""Test that view.rows_entered is not triggered when include_all_events is True."""
177+
169178
user = enterprise_data_fixture.create_user()
170179
table = enterprise_data_fixture.create_database_table(user=user)
171180
view = enterprise_data_fixture.create_grid_view(table=table)
@@ -183,6 +192,8 @@ def test_rows_enter_view_event_type_not_triggerd_with_include_all_events(
183192

184193
@pytest.mark.django_db()
185194
def test_rows_enter_view_event_event_type_test_payload(enterprise_data_fixture):
195+
"""Test the test/mock payload generation for the view.rows_entered event type."""
196+
186197
user = enterprise_data_fixture.create_user()
187198
table = enterprise_data_fixture.create_database_table(user=user)
188199
field = enterprise_data_fixture.create_text_field(
@@ -210,7 +221,6 @@ def test_rows_enter_view_event_event_type_test_payload(enterprise_data_fixture):
210221
"name": table.name,
211222
"database_id": table.database_id,
212223
},
213-
"type": "grid",
214224
"filters_disabled": False,
215225
"show_logo": True,
216226
"allow_public_export": False,
@@ -238,6 +248,8 @@ def test_rows_enter_view_event_event_type_test_payload(enterprise_data_fixture):
238248
def test_rows_enter_view_event_type_not_called_without_view(
239249
mock_call_webhook, enterprise_data_fixture, enable_enterprise, synced_roles
240250
):
251+
"""Test that the webhook is not called when no views are configured."""
252+
241253
user = enterprise_data_fixture.create_user()
242254
table = enterprise_data_fixture.create_database_table(user=user)
243255
view = enterprise_data_fixture.create_grid_view(table=table)
@@ -277,6 +289,8 @@ def test_rows_enter_view_event_type_not_called_without_view(
277289
def test_rows_enter_view_event_type_called_once_per_view(
278290
mock_call_webhook, enterprise_data_fixture, enable_enterprise, synced_roles
279291
):
292+
"""Test that the webhook is called once for each configured view."""
293+
280294
user = enterprise_data_fixture.create_user()
281295
table = enterprise_data_fixture.create_database_table(user=user)
282296
view_a = enterprise_data_fixture.create_grid_view(table=table)
@@ -306,6 +320,8 @@ def test_rows_enter_view_event_type_called_once_per_view(
306320
def test_rows_enter_view_event_type_only_right_webhook_is_called(
307321
mock_call_webhook, enterprise_data_fixture, enable_enterprise, synced_roles
308322
):
323+
"""Test that only the webhook for the correct table/view is triggered."""
324+
309325
user = enterprise_data_fixture.create_user()
310326
table_a = enterprise_data_fixture.create_database_table(user=user)
311327
view_a = enterprise_data_fixture.create_grid_view(table=table_a)
@@ -350,6 +366,8 @@ def test_rows_enter_view_event_type_only_right_webhook_is_called(
350366
def test_rows_enter_view_event_type_paginate_data(
351367
mock_make_request, enterprise_data_fixture, enable_enterprise, synced_roles
352368
):
369+
"""Test that large row batches are paginated into multiple webhook calls."""
370+
353371
user = enterprise_data_fixture.create_user()
354372
table = enterprise_data_fixture.create_database_table(user=user)
355373
text_field = enterprise_data_fixture.create_text_field(table=table, name="text")
@@ -376,7 +394,6 @@ def test_rows_enter_view_event_type_paginate_data(
376394
"name": table.name,
377395
"database_id": table.database_id,
378396
},
379-
"type": "grid",
380397
"filters_disabled": False,
381398
"show_logo": True,
382399
"allow_public_export": False,
@@ -513,6 +530,7 @@ def test_rows_enter_view_webhook_does_not_trigger_for_events_before_creation(
513530
table=table,
514531
rows_values=[
515532
{"id": rows[0].id, f"field_{text_field.id}": "Banana"},
533+
{"id": rows[3].id, f"field_{text_field.id}": "Banana no more"},
516534
],
517535
)
518536
with transaction.atomic():
@@ -540,3 +558,5 @@ def test_rows_enter_view_webhook_does_not_trigger_for_events_before_creation(
540558

541559
assert mock_call_webhook.delay.call_count == 1
542560
assert mock_call_webhook.delay.call_args[1]["event_type"] == "view.rows_entered"
561+
# Only row[2] entered the view since we recreated the webhook
562+
assert mock_call_webhook.delay.call_args[1]["payload"]["row_ids"] == [rows[2].id]

0 commit comments

Comments
 (0)