Repository: saleor/saleor
Pull request: Use default db for synchronous tax webhooks
https://github.com/saleor/saleor/issues/11906
I want to merge this change because when tax webhooks are triggered during mutation, subscription payload is generated based on replica db, so changes done by the mutation are not visible in the payload.


<!-- Please mention all relevant issue numbers. -->

# Impact

* [ ] New migrations
* [ ] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations
* [ ] Documentation needs to be updated

# Pull Request Checklist

<!-- Please keep this section. It will make the maintainer's life easier. -->

* [ ] Privileged queries and mutations are guarded by proper permission checks
* [ ] Database queries are optimized and the number of queries is constant
* [ ] Database migration files are up to date
* [ ] The changes are tested
* [ ] GraphQL schema and type definitions are up to date
* [ ] Changes are mentioned in the changelog


Problem statement:
Bug: Two request context in single Django request.
### What are you trying to achieve?

When CalculateTaxes event is triggered during `odrerDiscountAdd` mutation (within transaction), subscription payload is prepared base on replica db. This leads to uncommitted changes not being visible in the payload.

### Steps to reproduce the problem

Steps to reproduce:
1. Create new saleor.cloud instance ~~(locally (docker+gunicorn) it is not reproducible for me - which can indicate some setting impacts this)~~.
2. Configure webhook with subscription query provided below and syncEvent `CalculateTaxes`
3. Through API create Draft Order (`draftOrderCreate`)
4. Add any non-free lines (`orderLinesCreate`)
5. Add discount (`orderDiscountAdd`)
6. Tax webhook will be sent without discount in payload.

```grapql
subscription {
  event {
    ... on CalculateTaxes {
      taxBase {
        sourceObject {
          ... on Order {
            discounts {
              amount {
                amount
              }
              reason
            }
          }
        }
      }
    }
  }
}
```

update:
To reproduce locally change settings.py:
```
DATABASE_CONNECTION_DEFAULT_NAME = "default"
DATABASE_CONNECTION_REPLICA_NAME = "replica"

DATABASES = {
    DATABASE_CONNECTION_DEFAULT_NAME: dj_database_url.config(
        default="postgres://saleor:saleor@localhost:5432/saleor",
        conn_max_age=DB_CONN_MAX_AGE,
    ),
    DATABASE_CONNECTION_REPLICA_NAME: dj_database_url.config(
        default="postgres://saleor_read_only:saleor@localhost:5432/saleor",
        conn_max_age=DB_CONN_MAX_AGE,
    ),
}
```

### What did you expect to happen?

Discount visible in webhook payload

### Logs

_No response_

### Environment

Saleor version: 3.7+
Only saleor.cloud instances

Hints from issue discussion:

Commit message: Rename flag to allow_replica, add test.

Patch to review:
diff --git a/saleor/graphql/context.py b/saleor/graphql/context.py
index 06142aaa9ab..2440bca9ae6 100644
--- a/saleor/graphql/context.py
+++ b/saleor/graphql/context.py
@@ -17,7 +17,7 @@
 def get_context_value(request: HttpRequest) -> SaleorContext:
     request = cast(SaleorContext, request)
     request.dataloaders = {}
-    request.is_mutation = False
+    request.is_mutation = getattr(request, "is_mutation", False)
     request.request_time = timezone.now()
     set_app_on_context(request)
     set_auth_on_context(request)
diff --git a/saleor/graphql/plugins/dataloaders.py b/saleor/graphql/plugins/dataloaders.py
index 964c9980203..41b6a7ef51f 100644
--- a/saleor/graphql/plugins/dataloaders.py
+++ b/saleor/graphql/plugins/dataloaders.py
@@ -31,14 +31,16 @@ class PluginManagerByRequestorDataloader(DataLoader):
     context_key = "plugin_manager_by_requestor"
 
     def batch_load(self, keys):
-        return [get_plugins_manager(lambda: key) for key in keys]
+        is_mutation = getattr(self.context, "is_mutation", False)
+        return [get_plugins_manager(lambda: key, not is_mutation) for key in keys]
 
 
 class AnonymousPluginManagerLoader(DataLoader):
     context_key = "anonymous_plugin_manager"
 
     def batch_load(self, keys):
-        return [get_plugins_manager() for key in keys]
+        is_mutation = getattr(self.context, "is_mutation", False)
+        return [get_plugins_manager(None, not is_mutation) for key in keys]
 
 
 def plugin_manager_promise(context: SaleorContext, app) -> Promise[PluginsManager]:
diff --git a/saleor/graphql/webhook/subscription_payload.py b/saleor/graphql/webhook/subscription_payload.py
index 65b6b1467b9..6ec071d826d 100644
--- a/saleor/graphql/webhook/subscription_payload.py
+++ b/saleor/graphql/webhook/subscription_payload.py
@@ -18,7 +18,9 @@
 logger = get_task_logger(__name__)
 
 
-def initialize_request(requestor=None, sync_event=False) -> SaleorContext:
+def initialize_request(
+    requestor=None, sync_event=False, is_mutation=False
+) -> SaleorContext:
     """Prepare a request object for webhook subscription.
 
     It creates a dummy request object.
@@ -30,7 +32,6 @@ def _get_plugins(requestor_getter):
         return PluginsManager(settings.PLUGINS, requestor_getter)
 
     request_time = timezone.now()
-
     request = SaleorContext()
     request.path = "/graphql/"
     request.path_info = "/graphql/"
@@ -43,6 +44,7 @@ def _get_plugins(requestor_getter):
     setattr(request, "sync_event", sync_event)
     request.requestor = requestor
     request.request_time = request_time
+    request.is_mutation = is_mutation
 
     return request
 
@@ -88,7 +90,6 @@ def generate_payload_from_subscription(
         ast,
     )
     app_id = app.pk if app else None
-
     request.app = app
 
     results = document.execute(
diff --git a/saleor/plugins/base_plugin.py b/saleor/plugins/base_plugin.py
index 114d6d91335..0a3b0f8d196 100644
--- a/saleor/plugins/base_plugin.py
+++ b/saleor/plugins/base_plugin.py
@@ -132,7 +132,8 @@ def __init__(
         active: bool,
         channel: Optional["Channel"] = None,
         requestor_getter: Optional[Callable[[], "Requestor"]] = None,
-        db_config: Optional["PluginConfiguration"] = None
+        db_config: Optional["PluginConfiguration"] = None,
+        allow_replica: bool = True,
     ):
         self.configuration = self.get_plugin_configuration(configuration)
         self.active = active
@@ -141,6 +142,7 @@ def __init__(
             SimpleLazyObject(requestor_getter) if requestor_getter else requestor_getter
         )
         self.db_config = db_config
+        self.allow_replica = allow_replica
 
     def __str__(self):
         return self.PLUGIN_NAME
diff --git a/saleor/plugins/manager.py b/saleor/plugins/manager.py
index 4b69ecbf65d..d9031f9895e 100644
--- a/saleor/plugins/manager.py
+++ b/saleor/plugins/manager.py
@@ -90,6 +90,7 @@ def _load_plugin(
         db_configs_map: dict,
         channel: Optional["Channel"] = None,
         requestor_getter=None,
+        allow_replica=True,
     ) -> "BasePlugin":
         db_config = None
         if PluginClass.PLUGIN_ID in db_configs_map:
@@ -107,9 +108,10 @@ def _load_plugin(
             channel=channel,
             requestor_getter=requestor_getter,
             db_config=db_config,
+            allow_replica=allow_replica,
         )
 
-    def __init__(self, plugins: List[str], requestor_getter=None):
+    def __init__(self, plugins: List[str], requestor_getter=None, allow_replica=True):
         with opentracing.global_tracer().start_active_span("PluginsManager.__init__"):
             self.all_plugins = []
             self.global_plugins = []
@@ -126,6 +128,7 @@ def __init__(self, plugins: List[str], requestor_getter=None):
                             PluginClass,
                             global_db_configs,
                             requestor_getter=requestor_getter,
+                            allow_replica=allow_replica,
                         )
                         self.global_plugins.append(plugin)
                         self.all_plugins.append(plugin)
@@ -133,7 +136,11 @@ def __init__(self, plugins: List[str], requestor_getter=None):
                         for channel in channels:
                             channel_configs = channel_db_configs.get(channel, {})
                             plugin = self._load_plugin(
-                                PluginClass, channel_configs, channel, requestor_getter
+                                PluginClass,
+                                channel_configs,
+                                channel,
+                                requestor_getter,
+                                allow_replica,
                             )
                             self.plugins_per_channel[channel.slug].append(plugin)
                             self.all_plugins.append(plugin)
@@ -1654,7 +1661,8 @@ def is_event_active_for_any_plugin(
 
 
 def get_plugins_manager(
-    requestor_getter: Optional[Callable[[], "Requestor"]] = None
+    requestor_getter: Optional[Callable[[], "Requestor"]] = None,
+    allow_replica=True,
 ) -> PluginsManager:
     with opentracing.global_tracer().start_active_span("get_plugins_manager"):
-        return PluginsManager(settings.PLUGINS, requestor_getter)
+        return PluginsManager(settings.PLUGINS, requestor_getter, allow_replica)
diff --git a/saleor/plugins/webhook/plugin.py b/saleor/plugins/webhook/plugin.py
index 825acc7157c..da93c7e3883 100644
--- a/saleor/plugins/webhook/plugin.py
+++ b/saleor/plugins/webhook/plugin.py
@@ -1303,7 +1303,6 @@ def __run_payment_webhook(
         transaction_kind: str,
         payment_information: "PaymentData",
         previous_value,
-        **kwargs
     ) -> "GatewayResponse":
         """Trigger payment webhook event.
 
@@ -1479,6 +1478,7 @@ def get_taxes_for_checkout(
             parse_tax_data,
             checkout_info.checkout,
             self.requestor,
+            self.allow_replica,
         )
 
     def get_taxes_for_order(
@@ -1490,6 +1490,7 @@ def get_taxes_for_order(
             parse_tax_data,
             order,
             self.requestor,
+            self.allow_replica,
         )
 
     def get_shipping_methods_for_checkout(
diff --git a/saleor/plugins/webhook/tasks.py b/saleor/plugins/webhook/tasks.py
index fe4ea801d98..dd802adb09d 100644
--- a/saleor/plugins/webhook/tasks.py
+++ b/saleor/plugins/webhook/tasks.py
@@ -254,6 +254,7 @@ def trigger_all_webhooks_sync(
     parse_response: Callable[[Any], Optional[R]],
     subscribable_object=None,
     requestor=None,
+    allow_replica=True,
 ) -> Optional[R]:
     """Send all synchronous webhook request for given event type.
 
@@ -270,8 +271,9 @@ def trigger_all_webhooks_sync(
         if webhook.subscription_query:
             if request_context is None:
                 request_context = initialize_request(
-                    requestor, event_type in WebhookEventSyncType.ALL
+                    requestor, event_type in WebhookEventSyncType.ALL, not allow_replica
                 )
+
             delivery = create_delivery_for_subscription_sync_event(
                 event_type=event_type,
                 subscribable_object=subscribable_object,
diff --git a/saleor/plugins/webhook/tests/subscription_webhooks/fixtures.py b/saleor/plugins/webhook/tests/subscription_webhooks/fixtures.py
index 377f1f3acce..1750e00af0d 100644
--- a/saleor/plugins/webhook/tests/subscription_webhooks/fixtures.py
+++ b/saleor/plugins/webhook/tests/subscription_webhooks/fixtures.py
@@ -924,3 +924,13 @@ def subscription_order_filter_shipping_methods_webhook_with_available_ship_metho
         queries.ORDER_FILTER_SHIPPING_METHODS_AVAILABLE_SHIPPING_METHODS,
         WebhookEventSyncType.ORDER_FILTER_SHIPPING_METHODS,
     )
+
+
+@pytest.fixture
+def subscription_calculate_taxes_for_order(
+    subscription_webhook,
+):
+    return subscription_webhook(
+        queries.ORDER_CALCULATE_TAXES,
+        WebhookEventSyncType.ORDER_CALCULATE_TAXES,
+    )
diff --git a/saleor/plugins/webhook/tests/subscription_webhooks/subscription_queries.py b/saleor/plugins/webhook/tests/subscription_webhooks/subscription_queries.py
index 16f55c0de9f..6056245f789 100644
--- a/saleor/plugins/webhook/tests/subscription_webhooks/subscription_queries.py
+++ b/saleor/plugins/webhook/tests/subscription_webhooks/subscription_queries.py
@@ -2080,3 +2080,24 @@
     }
     """
 )
+
+
+ORDER_CALCULATE_TAXES = """
+    subscription {
+      event {
+        ... on CalculateTaxes {
+          taxBase {
+            sourceObject {
+              ...on Order{
+                discounts {
+                  amount {
+                    amount
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+"""
diff --git a/saleor/plugins/webhook/tests/subscription_webhooks/test_webhook.py b/saleor/plugins/webhook/tests/subscription_webhooks/test_webhook.py
index 516c1098ee8..89597542846 100644
--- a/saleor/plugins/webhook/tests/subscription_webhooks/test_webhook.py
+++ b/saleor/plugins/webhook/tests/subscription_webhooks/test_webhook.py
@@ -1,8 +1,14 @@
 import dataclasses
 import json
+from decimal import Decimal
 from unittest import mock
 
+import graphene
+
 from .....core.models import EventDelivery
+from .....graphql.discount.enums import DiscountValueTypeEnum
+from .....graphql.order.tests.mutations.test_order_discount import ORDER_DISCOUNT_ADD
+from .....graphql.webhook.subscription_payload import initialize_request
 from .....webhook.event_types import WebhookEventAsyncType, WebhookEventSyncType
 from .....webhook.models import Webhook
 from ...tasks import trigger_webhook_sync, trigger_webhooks_async
@@ -71,3 +77,46 @@ def test_trigger_webhook_sync_with_subscription(
     # then
     assert json.loads(event_delivery.payload.payload) == expected_payment_payload
     mock_request.assert_called_once_with(payment_app.name, event_delivery)
+
+
+@mock.patch("saleor.plugins.webhook.tasks.get_webhooks_for_event")
+@mock.patch(
+    "saleor.graphql.webhook.subscription_payload.generate_payload_from_subscription"
+)
+def test_trigger_sync_webhook_with_subscription_from_default_database(
+    mocked_generate_payload,
+    mocked_get_webhooks_for_event,
+    draft_order,
+    app_api_client,
+    permission_manage_orders,
+    settings,
+    subscription_calculate_taxes_for_order,
+):
+    # given
+    webhook = subscription_calculate_taxes_for_order
+    settings.PLUGINS = ["saleor.plugins.webhook.plugin.WebhookPlugin"]
+    mocked_get_webhooks_for_event.return_value = [webhook]
+    variables = {
+        "orderId": graphene.Node.to_global_id("Order", draft_order.pk),
+        "input": {
+            "valueType": DiscountValueTypeEnum.PERCENTAGE.name,
+            "value": Decimal("50"),
+        },
+    }
+    app_api_client.app.permissions.add(permission_manage_orders)
+    allow_replica = False
+    request = initialize_request(
+        None, WebhookEventSyncType.ORDER_CALCULATE_TAXES, is_mutation=not allow_replica
+    )
+
+    # when
+    app_api_client.post_graphql(ORDER_DISCOUNT_ADD, variables)
+
+    # then
+    mocked_generate_payload.assert_called_once_with(
+        WebhookEventSyncType.ORDER_CALCULATE_TAXES,
+        draft_order,
+        webhook.subscription_query,
+        request,
+        webhook.app,
+    )


Code review comments:
diff_hunk: @@ -18,7 +18,9 @@
 logger = get_task_logger(__name__)
 
 
-def initialize_request(requestor=None, sync_event=False) -> SaleorContext:
+def initialize_request(
+    requestor=None, sync_event=False, is_mutation=False
path: saleor/graphql/webhook/subscription_payload.py
text: @user1:
I would change `is_mutation=False` to `allow_replica=True` everywhere to keep it consistent.

@author:
You are right, this function is only used by subscription stuff.

diff_hunk: @@ -270,8 +271,9 @@ def trigger_all_webhooks_sync(
         if webhook.subscription_query:
             if request_context is None:
                 request_context = initialize_request(
-                    requestor, event_type in WebhookEventSyncType.ALL
+                    requestor, event_type in WebhookEventSyncType.ALL, not allow_replica
path: saleor/plugins/webhook/tasks.py
text: @user1:
When we change is_mutation to allow_replica this should become: 
```suggestion
                    requestor, event_type in WebhookEventSyncType.ALL, allow_replica
```