Fix cache for shipping methods (#15626)

* Fix cache for shipping methods (#15583)

* Remove meta from payload for shipping methods

* Increase cache time for shipping methods to 12h

* Add test to ensure that getting external shipping method will be called once
 Conflicts:
	saleor/checkout/tests/test_calculations.py

* Add test for shipping list methods for checkout cache builder function

saleor/checkout/tests/test_calculations.py

@@ -6,9 +6,10 @@
 from django.test import override_settings
 from django.utils import timezone
 from freezegun import freeze_time
+from graphene import Node
 from prices import Money, TaxedMoney
 
-from ...checkout.utils import add_promo_code_to_checkout
+from ...checkout.utils import add_promo_code_to_checkout, set_external_shipping_id
 from ...core.prices import quantize_price
 from ...core.taxes import TaxData, TaxLineData, zero_taxed_money
 from ...plugins import PLUGIN_IDENTIFIER_PREFIX
@@ -22,6 +23,7 @@
 )
 from ..calculations import (
     _apply_tax_data,
+    _calculate_and_add_tax,
     _get_checkout_base_prices,
     fetch_checkout_data,
 )
@@ -633,3 +635,69 @@ def test_fetch_checkout_data_calls_inactive_plugin(
 
     # then
     assert checkout_with_items.tax_error == "Empty tax data."
+
+
+@patch("saleor.webhook.transport.synchronous.transport.send_webhook_request_sync")
+def test_external_shipping_method_called_only_once_during_tax_calculations(
+    mock_send_webhook_request_sync,
+    checkout_with_single_item,
+    settings,
+    tax_app_with_subscription_webhooks,
+    shipping_app_with_subscription,
+    address,
+):
+    # given
+    external_method_id = "method-1-from-shipping-app"
+    mock_send_webhook_request_sync.side_effect = (
+        [
+            {
+                "amount": 1337.0,
+                "currency": "USD",
+                "id": external_method_id,
+                "name": "Shipping app method 1",
+            }
+        ],
+        {
+            "lines": [
+                {"tax_rate": 0, "total_gross_amount": 21.6, "total_net_amount": 20}
+            ],
+            "shipping_price_gross_amount": 1443.96,
+            "shipping_price_net_amount": 1337,
+            "shipping_tax_rate": 0,
+        },
+    )
+    external_shipping_method_id = Node.to_global_id(
+        "app", f"{shipping_app_with_subscription.id}:{external_method_id}"
+    )
+
+    settings.PLUGINS = ["saleor.plugins.webhook.plugin.WebhookPlugin"]
+    manager = get_plugins_manager(allow_replica=False)
+
+    checkout_with_single_item.shipping_address = address
+    set_external_shipping_id(checkout_with_single_item, external_shipping_method_id)
+    checkout_with_single_item.save()
+    checkout_with_single_item.metadata_storage.save()
+    checkout_lines, _ = fetch_checkout_lines(checkout_with_single_item)
+    checkout_info = fetch_checkout_info(
+        checkout_with_single_item, checkout_lines, manager
+    )
+    assert checkout_with_single_item.shipping_price == TaxedMoney(
+        net=Money("0", "USD"), gross=Money("0", "USD")
+    )
+
+    # when
+    _calculate_and_add_tax(
+        TaxCalculationStrategy.TAX_APP,
+        None,
+        checkout_with_single_item,
+        manager,
+        checkout_info,
+        checkout_lines,
+        prices_entered_with_tax=False,
+    )
+
+    # then
+    assert mock_send_webhook_request_sync.call_count == 2
+    assert checkout_with_single_item.shipping_price == TaxedMoney(
+        net=Money("1337.00", "USD"), gross=Money("1443.96", "USD")
+    )

saleor/plugins/webhook/plugin.py

@@ -152,7 +152,10 @@
     from ...webhook.models import Webhook
 
 
-CACHE_TIME_SHIPPING_LIST_METHODS_FOR_CHECKOUT: Final[int] = 5 * 60  # 5 minutes
+# Set the timeout for the shipping methods cache to 12 hours as it was the lowest
+# time labels were valid for when checking documentation for the carriers
+# (FedEx, UPS, TNT, DHL).
+CACHE_TIME_SHIPPING_LIST_METHODS_FOR_CHECKOUT: Final[int] = 3600 * 12
 
 
 logger = logging.getLogger(__name__)

saleor/tests/fixtures.py

@@ -158,6 +158,125 @@
 from ..webhook.transport.utils import WebhookResponse, to_payment_app_id
 from .utils import dummy_editorjs
 
+CALCULATE_TAXES_SUBSCRIPTION_QUERY = """
+subscription CalculateTaxes {
+  event {
+    ...CalculateTaxesEvent
+  }
+}
+
+fragment CalculateTaxesEvent on Event {
+  __typename
+  ... on CalculateTaxes {
+    taxBase {
+      ...TaxBase
+    }
+    recipient {
+      privateMetadata {
+        key
+        value
+      }
+    }
+  }
+}
+
+fragment TaxBase on TaxableObject {
+  pricesEnteredWithTax
+  currency
+  channel {
+    slug
+  }
+  discounts {
+    ...TaxDiscount
+  }
+  address {
+    ...Address
+  }
+  shippingPrice {
+    amount
+  }
+  lines {
+    ...TaxBaseLine
+  }
+  sourceObject {
+    __typename
+    ... on Checkout {
+      avataxEntityCode: metafield(key: "avataxEntityCode")
+      user {
+        ...User
+      }
+    }
+    ... on Order {
+      avataxEntityCode: metafield(key: "avataxEntityCode")
+      user {
+        ...User
+      }
+    }
+  }
+}
+
+fragment TaxDiscount on TaxableObjectDiscount {
+  name
+  amount {
+    amount
+  }
+}
+
+fragment Address on Address {
+  streetAddress1
+  streetAddress2
+  city
+  countryArea
+  postalCode
+  country {
+    code
+  }
+}
+
+fragment TaxBaseLine on TaxableObjectLine {
+  sourceLine {
+    __typename
+    ... on CheckoutLine {
+      id
+      checkoutProductVariant: variant {
+        id
+        product {
+          taxClass {
+            id
+            name
+          }
+        }
+      }
+    }
+    ... on OrderLine {
+      id
+      orderProductVariant: variant {
+        id
+        product {
+          taxClass {
+            id
+            name
+          }
+        }
+      }
+    }
+  }
+  quantity
+  unitPrice {
+    amount
+  }
+  totalPrice {
+    amount
+  }
+}
+
+fragment User on User {
+  id
+  email
+  avataxCustomerCode: metafield(key: "avataxCustomerCode")
+}
+"""
+
 
 class CaptureQueriesContext(BaseCaptureQueriesContext):
     IGNORED_QUERIES = settings.PATTERNS_IGNORED_IN_QUERY_CAPTURES  # type: ignore
@@ -7234,6 +7353,39 @@ def shipping_app(db, permission_manage_shipping):
     return app
 
 
+@pytest.fixture
+def shipping_app_with_subscription(db, permission_manage_shipping):
+    app = App.objects.create(name="Shipping App with subscription", is_active=True)
+    app.tokens.create(name="Default")
+    app.permissions.add(permission_manage_shipping)
+
+    webhook = Webhook.objects.create(
+        name="shipping-webhook-1",
+        app=app,
+        target_url="https://shipping-app.com/api/",
+        subscription_query="""
+        subscription {
+  event {
+    ... on ShippingListMethodsForCheckout {
+      __typename
+    }
+  }
+}
+
+        """,
+    )
+    webhook.events.bulk_create(
+        [
+            WebhookEvent(event_type=event_type, webhook=webhook)
+            for event_type in [
+                WebhookEventSyncType.SHIPPING_LIST_METHODS_FOR_CHECKOUT,
+                WebhookEventAsyncType.FULFILLMENT_CREATED,
+            ]
+        ]
+    )
+    return app
+
+
 @pytest.fixture
 def list_stored_payment_methods_app(db, permission_manage_payments):
     app = App.objects.create(
@@ -7348,6 +7500,30 @@ def tax_app(db, permission_handle_taxes):
         name="tax-webhook-1",
         app=app,
         target_url="https://tax-app.com/api/",
+        subscription_query=CALCULATE_TAXES_SUBSCRIPTION_QUERY,
+    )
+    webhook.events.bulk_create(
+        [
+            WebhookEvent(event_type=event_type, webhook=webhook)
+            for event_type in [
+                WebhookEventSyncType.ORDER_CALCULATE_TAXES,
+                WebhookEventSyncType.CHECKOUT_CALCULATE_TAXES,
+            ]
+        ]
+    )
+    return app
+
+
+@pytest.fixture
+def tax_app_with_subscription_webhooks(db, permission_handle_taxes):
+    app = App.objects.create(name="Tax App with subscription", is_active=True)
+    app.permissions.add(permission_handle_taxes)
+
+    webhook = Webhook.objects.create(
+        name="tax-subscription-webhook-1",
+        app=app,
+        target_url="https://tax-app.com/api/",
+        subscription_query=CALCULATE_TAXES_SUBSCRIPTION_QUERY,
     )
     webhook.events.bulk_create(
         [

saleor/webhook/transport/shipping.py

@@ -222,5 +222,5 @@ def get_cache_data_for_shipping_list_methods_for_checkout(payload: str) -> dict:
 
     # drop fields that change between requests but are not relevant for cache key
     key_data[0].pop("last_change")
-    key_data[0]["meta"].pop("issued_at")
+    key_data[0].pop("meta")
     return key_data

saleor/webhook/transport/tests/test_utils.py

@@ -1,6 +1,6 @@
 import datetime
 import json
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 from celery import Task
@@ -10,6 +10,7 @@
 from ....core import EventDeliveryStatus
 from ....core.models import EventDeliveryAttempt
 from ...event_types import WebhookEventAsyncType
+from ..shipping import get_cache_data_for_shipping_list_methods_for_checkout
 from ..utils import WebhookResponse, handle_webhook_retry, send_webhook_using_aws_sqs
 
 
@@ -187,3 +188,19 @@ def test_send_webhook_using_aws_sqs_with_fifo_queue(mocked_boto3_client):
     # then
     _, send_message_kwargs = mocked_boto3_client.send_message.call_args
     assert send_message_kwargs["MessageGroupId"] == domain
+
+
+@patch("saleor.webhook.transport.shipping.json.loads")
+def test_get_cache_data_for_shipping_list_methods_for_checkout(mock_loads):
+    # when
+    result = get_cache_data_for_shipping_list_methods_for_checkout("test payload")
+
+    # then
+    assert result is mock_loads.return_value
+    assert mock_loads.mock_calls == [
+        call("test payload"),
+        call().__getitem__(0),
+        call().__getitem__().pop("last_change"),
+        call().__getitem__(0),
+        call().__getitem__().pop("meta"),
+    ]