diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 3e59d12594..7660225735 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -294,16 +294,17 @@ def destroy( def partial_update( self, request: Request, *args: tuple[Any], **kwargs: dict[str, Any] ) -> Response: - # Store current shared users before update (for email notifications) adapter = self.get_object() - current_shared_users = set(adapter.shared_users.all()) + shared_users_updated = AdapterKeys.SHARED_USERS in request.data + current_shared_users = set() - if AdapterKeys.SHARED_USERS in request.data: + if shared_users_updated: + current_shared_users = set(adapter.shared_users.all()) # find the deleted users shared_users = { int(user_id) for user_id in request.data.get("shared_users", {}) } - current_users = {user.id for user in adapter.shared_users.all()} + current_users = {user.id for user in current_shared_users} removed_users = current_users.difference(shared_users) # if removed user use this adapter as default @@ -345,7 +346,11 @@ def partial_update( response = super().partial_update(request, *args, **kwargs) # Send email notifications to newly shared users - if response.status_code == 200 and AdapterKeys.SHARED_USERS in request.data: + if ( + response.status_code == 200 + and shared_users_updated + and bool(notification_plugin) + ): try: adapter.refresh_from_db() new_shared_users = set(adapter.shared_users.all()) diff --git a/backend/api_v2/api_deployment_views.py b/backend/api_v2/api_deployment_views.py index e636ca01ec..8ab725f381 100644 --- a/backend/api_v2/api_deployment_views.py +++ b/backend/api_v2/api_deployment_views.py @@ -373,17 +373,16 @@ def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Respons """Override partial_update to handle sharing notifications.""" # Get current instance and shared users instance = self.get_object() - current_shared_users = set(instance.shared_users.all()) + shared_users_updated = "shared_users" in request.data + current_shared_users = set() + if shared_users_updated: + current_shared_users = set(instance.shared_users.all()) # Perform the update response = super().partial_update(request, *args, **kwargs) # If successful and shared_users changed, send notifications - if ( - response.status_code == 200 - and "shared_users" in request.data - and notification_plugin - ): + if response.status_code == 200 and shared_users_updated and notification_plugin: try: instance.refresh_from_db() new_shared_users = set(instance.shared_users.all()) diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index 37525e4d5a..8bcd406f72 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -204,13 +204,16 @@ def perform_destroy(self, instance: ConnectorInstance) -> None: def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Response: """Override to handle sharing notifications.""" instance = self.get_object() - current_shared_users = set(instance.shared_users.all()) + shared_users_updated = "shared_users" in request.data + current_shared_users = set() + if shared_users_updated: + current_shared_users = set(instance.shared_users.all()) response = super().partial_update(request, *args, **kwargs) if ( response.status_code == 200 - and "shared_users" in request.data + and shared_users_updated and bool(notification_plugin) ): try: diff --git a/backend/pipeline_v2/views.py b/backend/pipeline_v2/views.py index 0d902a792c..d047ab3a7e 100644 --- a/backend/pipeline_v2/views.py +++ b/backend/pipeline_v2/views.py @@ -143,15 +143,14 @@ def list_of_shared_users(self, request: Request, pk: str | None = None) -> Respo def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Response: """Override to handle sharing notifications.""" instance = self.get_object() - current_shared_users = set(instance.shared_users.all()) + shared_users_updated = "shared_users" in request.data + current_shared_users = set() + if shared_users_updated: + current_shared_users = set(instance.shared_users.all()) response = super().partial_update(request, *args, **kwargs) - if ( - response.status_code == 200 - and "shared_users" in request.data - and notification_plugin - ): + if response.status_code == 200 and shared_users_updated and notification_plugin: try: instance.refresh_from_db() new_shared_users = set(instance.shared_users.all()) diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index 99edefd0b4..0458283aad 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -266,15 +266,17 @@ def destroy( def partial_update( self, request: Request, *args: tuple[Any], **kwargs: dict[str, Any] ) -> Response: - # Store current shared users before update for email notifications custom_tool = self.get_object() - current_shared_users = set(custom_tool.shared_users.all()) + shared_users_updated = "shared_users" in request.data + current_shared_users = set() + if shared_users_updated: + current_shared_users = set(custom_tool.shared_users.all()) # Perform the update response = super().partial_update(request, *args, **kwargs) # Send email notifications to newly shared users - if response.status_code == 200 and "shared_users" in request.data: + if response.status_code == 200 and shared_users_updated: from plugins import get_plugin notification_plugin = get_plugin("notification") diff --git a/backend/utils/tests/test_shared_user_query_optimizations.py b/backend/utils/tests/test_shared_user_query_optimizations.py new file mode 100644 index 0000000000..fd970bd6dc --- /dev/null +++ b/backend/utils/tests/test_shared_user_query_optimizations.py @@ -0,0 +1,62 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from rest_framework import status, viewsets +from rest_framework.response import Response + +from adapter_processor_v2.views import AdapterInstanceViewSet +from workflow_manager.workflow_v2.views import WorkflowViewSet + + +def test_workflow_partial_update_skips_shared_user_lookup_without_share_changes() -> None: + view = WorkflowViewSet() + workflow = MagicMock() + workflow.shared_users.all.side_effect = AssertionError( + "shared_users should not be queried for non-sharing updates" + ) + request = SimpleNamespace(data={"workflow_name": "renamed"}, user=MagicMock()) + + with ( + patch.object(WorkflowViewSet, "get_object", return_value=workflow), + patch.object( + viewsets.ModelViewSet, + "partial_update", + autospec=True, + return_value=Response(status=status.HTTP_200_OK), + ), + ): + response = view.partial_update(request) + + assert response.status_code == status.HTTP_200_OK + workflow.shared_users.all.assert_not_called() + + +def test_adapter_partial_update_queries_shared_users_once_without_notification_plugin() -> None: + # When notification_plugin is absent (the common test-env case), the post-update + # comparison block is skipped entirely. Only one pre-update snapshot query should + # be made — the old code made two (snapshot + duplicate ID extraction). + view = AdapterInstanceViewSet() + shared_user_1 = SimpleNamespace(id=1) + shared_user_2 = SimpleNamespace(id=2) + adapter = MagicMock(adapter_type="LLM", adapter_name="Adapter", id=1) + adapter.shared_users.all.return_value = [shared_user_1, shared_user_2] + request = SimpleNamespace( + data={"shared_users": ["1", "2"]}, + user=MagicMock(), + ) + + with ( + patch.object(AdapterInstanceViewSet, "get_object", return_value=adapter), + patch.object( + viewsets.ModelViewSet, + "partial_update", + autospec=True, + return_value=Response(status=status.HTTP_200_OK), + ), + patch("adapter_processor_v2.views.notification_plugin", None), + ): + response = view.partial_update(request) + + assert response.status_code == status.HTTP_200_OK + # Pre-update snapshot only — the duplicate ID-extraction query is gone. + assert adapter.shared_users.all.call_count == 1 diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index 629b52bb22..8371426147 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -140,9 +140,12 @@ def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Respons """Override partial_update to handle sharing notifications.""" # Get the workflow instance before update workflow = self.get_object() + shared_users_updated = "shared_users" in request.data - # Store current shared users for comparison - current_shared_users = set(workflow.shared_users.all()) + # Store current shared users for comparison only when sharing changes. + current_shared_users = set() + if shared_users_updated: + current_shared_users = set(workflow.shared_users.all()) # Perform the standard partial update response = super().partial_update(request, *args, **kwargs) @@ -150,7 +153,7 @@ def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Respons # If update was successful and shared_users field was modified if ( response.status_code == 200 - and "shared_users" in request.data + and shared_users_updated and bool(notification_plugin) ): try: