Skip to content

Commit

Permalink
Merge pull request saleor#13601 from saleor/fix-failing-transaction-r…
Browse files Browse the repository at this point in the history
…equest-action

Fix webhook calls triggered by transactionRequestAction mutation
  • Loading branch information
korycins authored Aug 2, 2023
2 parents 9ff147e + fb2037b commit d0ce404
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 39 deletions.
17 changes: 8 additions & 9 deletions saleor/plugins/webhook/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def trigger_webhook_sync(
if timeout:
kwargs = {"timeout": timeout}

return send_webhook_request_sync(webhook.app, delivery, **kwargs)
return send_webhook_request_sync(delivery, **kwargs)


R = TypeVar("R")
Expand Down Expand Up @@ -312,7 +312,7 @@ def trigger_all_webhooks_sync(
webhook=webhook,
)

response_data = send_webhook_request_sync(webhook.app, delivery)
response_data = send_webhook_request_sync(delivery)
if parsed_response := parse_response(response_data):
return parsed_response
return None
Expand Down Expand Up @@ -594,7 +594,7 @@ def send_webhook_request_async(self, event_delivery_id):


def _send_webhook_request_sync(
app, delivery, timeout=settings.WEBHOOK_SYNC_TIMEOUT, attempt=None
delivery, timeout=settings.WEBHOOK_SYNC_TIMEOUT, attempt=None
) -> Tuple[WebhookResponse, Optional[Dict[Any, Any]]]:
event_payload = delivery.payload
data = event_payload.payload
Expand All @@ -620,7 +620,7 @@ def _send_webhook_request_sync(

try:
with webhooks_opentracing_trace(
delivery.event_type, domain, sync=True, app=app
delivery.event_type, domain, sync=True, app=webhook.app
):
response = send_webhook_using_http(
webhook.target_url,
Expand Down Expand Up @@ -667,9 +667,9 @@ def _send_webhook_request_sync(


def send_webhook_request_sync(
app, delivery, timeout=settings.WEBHOOK_SYNC_TIMEOUT
delivery, timeout=settings.WEBHOOK_SYNC_TIMEOUT
) -> Optional[Dict[Any, Any]]:
response, response_data = _send_webhook_request_sync(app, delivery, timeout)
response, response_data = _send_webhook_request_sync(delivery, timeout)
return response_data if response.status == EventDeliveryStatus.SUCCESS else None


Expand Down Expand Up @@ -814,7 +814,6 @@ def trigger_transaction_request(
call_event(
handle_transaction_request_task.delay,
delivery.id,
webhook.app,
transaction_data.event.id,
)
return None
Expand All @@ -825,7 +824,7 @@ def trigger_transaction_request(
retry_backoff=10,
retry_kwargs={"max_retries": 5},
)
def handle_transaction_request_task(self, delivery_id, app, request_event_id):
def handle_transaction_request_task(self, delivery_id, request_event_id):
delivery = get_delivery_for_webhook(delivery_id)
if not delivery:
logger.error(
Expand All @@ -841,7 +840,7 @@ def handle_transaction_request_task(self, delivery_id, app, request_event_id):
)
return None
attempt = create_attempt(delivery, self.request.id)
response, response_data = _send_webhook_request_sync(app, delivery, attempt=attempt)
response, response_data = _send_webhook_request_sync(delivery, attempt=attempt)
if response.response_status_code and response.response_status_code >= 500:
handle_webhook_retry(
self, delivery.webhook, response.content, delivery, attempt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ 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, event_delivery)
mock_request.assert_called_once_with(event_delivery)


@mock.patch("saleor.plugins.webhook.tasks.send_webhook_request_sync")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _assert_fields(payload, webhook, expected_data, response, mock_request):
)
assert delivery.payload == event_payload
assert delivery.webhook == webhook
mock_request.assert_called_once_with(webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
assert response == [
PaymentGatewayData(app_identifier=webhook_app.identifier, data=expected_data)
]
Expand Down
18 changes: 9 additions & 9 deletions saleor/plugins/webhook/tests/test_payment_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_trigger_webhook_sync(mock_request, payment_app):
WebhookEventSyncType.PAYMENT_CAPTURE, data, payment_app.webhooks.first()
)
event_delivery = EventDelivery.objects.first()
mock_request.assert_called_once_with(payment_app, event_delivery)
mock_request.assert_called_once_with(event_delivery)


@mock.patch("saleor.plugins.webhook.tasks.create_delivery_for_subscription_sync_event")
Expand All @@ -76,7 +76,7 @@ def test_trigger_webhook_sync_with_subscription(
payment_app.webhooks.first(),
payment,
)
mock_request.assert_called_once_with(payment_app, fake_delivery)
mock_request.assert_called_once_with(fake_delivery)


@mock.patch("saleor.plugins.webhook.tasks.observability.report_event_delivery_attempt")
Expand All @@ -97,7 +97,7 @@ def test_send_webhook_request_sync_failed_attempt(
mock_post().status_code = expected_data["status_code"]
mock_post().elapsed = expected_data["duration"]
# when
response_data = send_webhook_request_sync(app, event_delivery)
response_data = send_webhook_request_sync(event_delivery)
attempt = EventDeliveryAttempt.objects.first()

# then
Expand Down Expand Up @@ -130,7 +130,7 @@ def test_send_webhook_request_sync_successful_attempt(
mock_post().status_code = expected_data["status_code"]
mock_post().elapsed = expected_data["duration"]
# when
response_data = send_webhook_request_sync(app, event_delivery)
response_data = send_webhook_request_sync(event_delivery)

attempt = EventDeliveryAttempt.objects.first()

Expand Down Expand Up @@ -163,7 +163,7 @@ def test_send_webhook_request_sync_request_exception(
)

# when
response_data = send_webhook_request_sync(app, event_delivery)
response_data = send_webhook_request_sync(event_delivery)
attempt = EventDeliveryAttempt.objects.first()

# then
Expand All @@ -189,7 +189,7 @@ def test_send_webhook_request_sync_when_exception_with_response(
mock_response.status_code = 302
mock_post.side_effect = TooManyRedirects(response=mock_response)
# when
send_webhook_request_sync(app, event_delivery)
send_webhook_request_sync(event_delivery)
attempt = EventDeliveryAttempt.objects.first()

# then
Expand Down Expand Up @@ -217,7 +217,7 @@ def test_send_webhook_request_sync_json_parsing_error(
mock_post().status_code = expected_data["status_code"]

# when
response_data = send_webhook_request_sync(app, event_delivery)
response_data = send_webhook_request_sync(event_delivery)
attempt = EventDeliveryAttempt.objects.first()

# then
Expand All @@ -237,7 +237,7 @@ def test_send_webhook_request_with_proper_timeout(mock_post, event_delivery, app
mock_post().headers = {"header_key": "header_val"}
mock_post().elapsed = datetime.timedelta(seconds=1)
mock_post().status_code = 200
send_webhook_request_sync(app, event_delivery)
send_webhook_request_sync(event_delivery)
assert mock_post.call_args.kwargs["timeout"] == settings.WEBHOOK_SYNC_TIMEOUT


Expand All @@ -253,7 +253,7 @@ def test_send_webhook_request_sync_invalid_scheme(webhook, app):
payload=event_payload,
webhook=webhook,
)
send_webhook_request_sync(app, delivery)
send_webhook_request_sync(delivery)


@mock.patch("saleor.plugins.webhook.tasks.send_webhook_request_sync")
Expand Down
2 changes: 1 addition & 1 deletion saleor/plugins/webhook/tests/test_shipping_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ def test_trigger_webhook_sync(mock_request, shipping_app):
WebhookEventSyncType.SHIPPING_LIST_METHODS_FOR_CHECKOUT, data, webhook
)
event_delivery = EventDelivery.objects.first()
mock_request.assert_called_once_with(shipping_app, event_delivery)
mock_request.assert_called_once_with(event_delivery)


@mock.patch("saleor.plugins.webhook.shipping.cache.set")
Expand Down
20 changes: 10 additions & 10 deletions saleor/plugins/webhook/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def test_trigger_transaction_request(
assert generated_delivery.webhook == webhook
assert generated_delivery.payload == generated_payload

mocked_task.assert_called_once_with(generated_delivery.id, app, event.id)
mocked_task.assert_called_once_with(generated_delivery.id, event.id)


@freeze_time("2022-06-11 12:50")
Expand Down Expand Up @@ -158,7 +158,7 @@ def test_trigger_transaction_request_with_webhook_subscription(

assert generated_delivery.payload == generated_payload

mocked_task.assert_called_once_with(generated_delivery.id, app, event.id)
mocked_task.assert_called_once_with(generated_delivery.id, event.id)


@freeze_time("2022-06-11 12:50")
Expand Down Expand Up @@ -210,7 +210,7 @@ def test_handle_transaction_request_task_with_only_psp_reference(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert TransactionEvent.objects.count() == 1
Expand Down Expand Up @@ -277,7 +277,7 @@ def test_handle_transaction_request_task_with_server_error(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert mocked_webhook_retry.called
Expand Down Expand Up @@ -330,7 +330,7 @@ def test_handle_transaction_request_task_with_missing_psp_reference(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert (
Expand Down Expand Up @@ -413,7 +413,7 @@ def test_handle_transaction_request_task_with_missing_required_event_field(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert (
Expand Down Expand Up @@ -505,7 +505,7 @@ def test_handle_transaction_request_task_with_result_event(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert TransactionEvent.objects.all().count() == 2
Expand Down Expand Up @@ -596,7 +596,7 @@ def test_handle_transaction_request_task_with_only_required_fields_for_result_ev
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert TransactionEvent.objects.all().count() == 2
Expand Down Expand Up @@ -700,7 +700,7 @@ def test_handle_transaction_request_task_calls_recalculation_of_amounts(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
mocked_recalculation.assert_called_once_with(transaction, save=False)
Expand Down Expand Up @@ -764,7 +764,7 @@ def test_handle_transaction_request_task_with_available_actions(
)

# when
handle_transaction_request_task(delivery.id, app, transaction_data.event.id)
handle_transaction_request_task(delivery.id, transaction_data.event.id)

# then
assert TransactionEvent.objects.all().count() == 2
Expand Down
6 changes: 3 additions & 3 deletions saleor/plugins/webhook/tests/test_tax_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_get_taxes_for_order(
assert delivery.event_type == WebhookEventSyncType.ORDER_CALCULATE_TAXES
assert delivery.payload == payload
assert delivery.webhook == tax_order_webhook
mock_request.assert_called_once_with(tax_order_webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
mock_fetch.assert_not_called()
assert tax_data == parse_tax_data(tax_data_response)

Expand Down Expand Up @@ -181,7 +181,7 @@ def test_get_taxes_for_order_with_sync_subscription(
assert delivery.event_type == WebhookEventSyncType.ORDER_CALCULATE_TAXES
assert delivery.payload == payload
assert delivery.webhook == webhook
mock_request.assert_called_once_with(webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
mock_fetch.assert_not_called()
assert tax_data == parse_tax_data(tax_data_response)

Expand Down Expand Up @@ -222,6 +222,6 @@ def test_get_taxes_for_checkout_with_sync_subscription(
assert delivery.event_type == WebhookEventSyncType.CHECKOUT_CALCULATE_TAXES
assert delivery.payload == payload
assert delivery.webhook == webhook
mock_request.assert_called_once_with(webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
mock_fetch.assert_not_called()
assert tax_data == parse_tax_data(tax_data_response)
6 changes: 3 additions & 3 deletions saleor/plugins/webhook/tests/test_tax_webhook_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test_trigger_tax_webhook_sync(
assert delivery.event_type == event_type
assert delivery.payload == payload
assert delivery.webhook == tax_checkout_webhook
mock_request.assert_called_once_with(tax_checkout_webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
assert tax_data == parse_tax_data(tax_data_response)


Expand Down Expand Up @@ -86,7 +86,7 @@ def test_trigger_tax_webhook_sync_multiple_webhooks_first(
assert delivery.event_type == event_type
assert delivery.payload == payload
assert delivery.webhook == successful_webhook
mock_request.assert_called_once_with(successful_webhook.app, delivery)
mock_request.assert_called_once_with(delivery)
assert tax_data == parse_tax_data(tax_data_response)


Expand Down Expand Up @@ -116,7 +116,7 @@ def test_trigger_tax_webhook_sync_multiple_webhooks_last(
assert delivery.event_type == event_type
assert delivery.payload == payload
assert delivery.webhook == webhook
assert call[0] == (webhook.app, delivery)
assert call[0] == (delivery,)

assert mock_request.call_count == 3
assert tax_data == parse_tax_data(tax_data_response)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def _assert_fields(payload, webhook, expected_response, response, mock_request):
assert delivery.event_type == WebhookEventSyncType.TRANSACTION_INITIALIZE_SESSION
assert delivery.payload == event_payload
assert delivery.webhook == webhook
mock_request.assert_called_once_with(webhook_app, delivery)
mock_request.assert_called_once_with(delivery)
assert response == TransactionSessionResult(
app_identifier=webhook_app.identifier, response=expected_response, error=None
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def _assert_fields(payload, webhook, expected_response, response, mock_request):
assert delivery.event_type == WebhookEventSyncType.TRANSACTION_PROCESS_SESSION
assert delivery.payload == event_payload
assert delivery.webhook == webhook
mock_request.assert_called_once_with(webhook_app, delivery)
mock_request.assert_called_once_with(delivery)
assert response == TransactionSessionResult(
app_identifier=webhook_app.identifier, response=expected_response, error=None
)
Expand Down

0 comments on commit d0ce404

Please sign in to comment.