diff --git a/backend/justfile b/backend/justfile index c86bbced1d..c3dfce6130 100644 --- a/backend/justfile +++ b/backend/justfile @@ -576,11 +576,11 @@ alias mk := makemigrations # Open Django shell with shell_plus and SQL logging. Pass --print-sql to enable SQL query logging. [group('2 - development')] -shell_plus: _check-dev +shell_plus *args: _check-dev #!/usr/bin/env bash set -euo pipefail {{ _load_env }} - {{ uv_run }} baserow shell_plus + {{ uv_run }} baserow shell_plus {{ args }} alias sp := shell_plus diff --git a/backend/src/baserow/core/notifications/handler.py b/backend/src/baserow/core/notifications/handler.py index 8e6cb76d94..01bc03984d 100644 --- a/backend/src/baserow/core/notifications/handler.py +++ b/backend/src/baserow/core/notifications/handler.py @@ -702,9 +702,9 @@ def filter_and_annotate_users_with_notifications_to_send_by_email( notifications_to_send_by_email_prefetch = Prefetch( "notifications", - queryset=Notification.objects.filter( - id__in=unsent_notification_subquery - ).distinct(), + queryset=Notification.objects.filter(id__in=unsent_notification_subquery) + .distinct() + .select_related("sender"), to_attr="unsent_email_notifications", ) diff --git a/backend/tests/baserow/core/notifications/test_notifications_handler.py b/backend/tests/baserow/core/notifications/test_notifications_handler.py index b807d16fea..5420b9c944 100644 --- a/backend/tests/baserow/core/notifications/test_notifications_handler.py +++ b/backend/tests/baserow/core/notifications/test_notifications_handler.py @@ -1,8 +1,10 @@ from unittest.mock import MagicMock, call, patch from django.conf import settings +from django.db import connection from django.db.models import Q from django.test import override_settings +from django.test.utils import CaptureQueriesContext import pytest @@ -14,6 +16,11 @@ UserNotificationsGrouper, ) from baserow.core.notifications.models import Notification, NotificationRecipient +from baserow.core.notifications.registries import ( + EmailNotificationTypeMixin, + NotificationType, + notification_type_registry, +) from baserow.core.user.handler import UserHandler from .utils import custom_notification_types_registered @@ -813,3 +820,87 @@ def test_email_notifications_are_not_sent_if_already_cleared_by_user( assert res.remaining_users_to_notify_count == 0 mock_get_mail_connection.assert_not_called() + + +@pytest.mark.django_db(transaction=True) +@patch("baserow.core.notifications.handler.get_mail_connection") +def test_email_notifications_queries_are_limited( + mock_get_mail_connection, + data_fixture, + mutable_notification_type_registry, +): + """ + The query count for sending notification emails must not scale with the + number of notifications or the number of users. In particular the + prefetch must include select_related("sender") so that accessing + notification.sender during email rendering doesn't cause one query per + notification. + """ + + mock_connection = MagicMock() + mock_get_mail_connection.return_value = mock_connection + + class SenderAccessingNotification(EmailNotificationTypeMixin, NotificationType): + type = "test_sender_accessing_notification" + + @classmethod + def get_notification_title_for_email(cls, notification, context): + return f"Notification from {notification.sender.first_name}" + + @classmethod + def get_notification_description_for_email(cls, notification, context): + return None + + notification_type_registry.register(SenderAccessingNotification()) + try: + sender = data_fixture.create_user(first_name="Sender") + + # --- Warm-up run to prime internal caches (license cache, etc.) --- + warmup_user = data_fixture.create_user() + data_fixture.create_notification_for_users( + recipients=[warmup_user], + sender=sender, + notification_type=SenderAccessingNotification.type, + ) + NotificationHandler.send_unread_notifications_by_email_to_users_matching_filters( + Q(pk=warmup_user.pk) + ) + mock_get_mail_connection.reset_mock() + mock_connection.reset_mock() + + # --- Run 1: 1 user, 1 notification --- + user_a = data_fixture.create_user() + data_fixture.create_notification_for_users( + recipients=[user_a], + sender=sender, + notification_type=SenderAccessingNotification.type, + ) + + with CaptureQueriesContext(connection) as ctx_small: + NotificationHandler.send_unread_notifications_by_email_to_users_matching_filters( + Q(pk=user_a.pk) + ) + assert len(ctx_small) # sanity + + # --- Run 2: 2 users, 5 notifications each --- + mock_get_mail_connection.reset_mock() + mock_connection.reset_mock() + + user_b = data_fixture.create_user() + user_c = data_fixture.create_user() + for _ in range(5): + data_fixture.create_notification_for_users( + recipients=[user_b, user_c], + sender=sender, + notification_type=SenderAccessingNotification.type, + ) + + with CaptureQueriesContext(connection) as ctx_large: + NotificationHandler.send_unread_notifications_by_email_to_users_matching_filters( + Q(pk__in=[user_b.pk, user_c.pk]) + ) + + # Query count must be identical: no N+1 on sender, users, or notifications. + assert len(ctx_large) == len(ctx_small) + finally: + notification_type_registry.unregister(SenderAccessingNotification.type) diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 8ce6b80b24..acecc950a2 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -65,10 +65,9 @@ services: # reducing the chance the containers screw up the bind mounted folders. UID: $UID GID: $GID - command: "nuxt-dev-with-storybook" + command: "nuxt-dev" ports: - "${HOST_PUBLISH_IP:-127.0.0.1}:3000:3000" - - "${HOST_PUBLISH_IP:-127.0.0.1}:6006:6006" volumes: - ./web-frontend:/baserow/web-frontend - ./premium/web-frontend/:/baserow/premium/web-frontend @@ -79,12 +78,45 @@ services: # overwriting existing node_modules. - node_modules:/baserow/web-frontend/node_modules - nuxt:/baserow/web-frontend/.nuxt/ - - storybook:/baserow/web-frontend/.nuxt-storybook/ - cache:/baserow/web-frontend/.cache/ # Open stdin and tty so when attaching key input works as expected. stdin_open: true tty: true + storybook: + image: baserow_web-frontend:dev + profiles: ["optional"] + build: + dockerfile: ./web-frontend/Dockerfile + context: . + target: dev + args: + # We allow configuring the UID/GID here so you can run as the dev's actual user + # reducing the chance the containers screw up the bind mounted folders. + UID: $UID + GID: $GID + command: "storybook-dev" + ports: + - "${HOST_PUBLISH_IP:-127.0.0.1}:6006:6006" + volumes: + - ./web-frontend:/baserow/web-frontend + - ./premium/web-frontend/:/baserow/premium/web-frontend + - ./enterprise/web-frontend/:/baserow/enterprise/web-frontend + - ./tests/:/baserow/tests + - ./deploy/plugins:/baserow/plugins + # dev volumes to avoid polluting your local filesystem with build files or + # overwriting existing node_modules. + - node_modules:/baserow/web-frontend/node_modules + # Open stdin and tty so when attaching key input works as expected. + stdin_open: true + tty: true + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost:6006/"] + interval: 60s + timeout: 5s + retries: 3 + start_period: 30s + celery: image: baserow_backend:dev user: "${UID}:${GID}" diff --git a/web-frontend/docker/docker-entrypoint.sh b/web-frontend/docker/docker-entrypoint.sh index 70f815da22..e30f819531 100755 --- a/web-frontend/docker/docker-entrypoint.sh +++ b/web-frontend/docker/docker-entrypoint.sh @@ -104,6 +104,12 @@ case "$1" in yarn storybook & attachable_exec_retry yarn dev ;; + storybook-dev) + startup_plugin_setup + setup_additional_modules + # Retry the command over and over to work around heap crash. + attachable_exec_retry yarn storybook + ;; lint) exec yarn lint ;; diff --git a/web-frontend/modules/automation/components/workflow/WorkflowNodeContent.vue b/web-frontend/modules/automation/components/workflow/WorkflowNodeContent.vue index b3d08d7e53..ad90fcb1d6 100644 --- a/web-frontend/modules/automation/components/workflow/WorkflowNodeContent.vue +++ b/web-frontend/modules/automation/components/workflow/WorkflowNodeContent.vue @@ -166,9 +166,10 @@ onMove(() => { const editNodeContext = ref(null) const editNodeContextToggle = ref(null) + const openEditContext = () => { if (editNodeContext.value && editNodeContextToggle.value) { - activeNodeContext.value = editNodeContext + activeNodeContext.value = editNodeContext.value editNodeContext.value.toggle( editNodeContextToggle.value, 'bottom', @@ -179,12 +180,13 @@ const openEditContext = () => { } const replaceNodeContext = ref(null) + const openReplaceContext = async () => { editNodeContext.value.hide() // As the target isn't the element that triggered the show of the context it is not // ignored by the click outside handler and it immediately closes the context await flushPromises() - activeNodeContext.value = replaceNodeContext + activeNodeContext.value = replaceNodeContext.value replaceNodeContext.value.toggle( editNodeContextToggle.value, 'bottom', diff --git a/web-frontend/modules/builder/components/PageEditorContent.vue b/web-frontend/modules/builder/components/PageEditorContent.vue new file mode 100644 index 0000000000..06fd40da95 --- /dev/null +++ b/web-frontend/modules/builder/components/PageEditorContent.vue @@ -0,0 +1,136 @@ + + + diff --git a/web-frontend/modules/builder/pages/pageEditor.vue b/web-frontend/modules/builder/pages/pageEditor.vue index 12f1a625c3..97535ce490 100644 --- a/web-frontend/modules/builder/pages/pageEditor.vue +++ b/web-frontend/modules/builder/pages/pageEditor.vue @@ -1,33 +1,21 @@