For missing webhooks, we will be adding dedicated issues. This task aims at refactoring the webhooks mechanism to move it away from plugins and make it a regular core module. Implementing webhooks as a plugin adds unnecessary complications and makes it harder to add new events.

• Drop WebhookPlugin and move the entire webhook mechanism into a core module.
• Reimplement all webhook events to be called directly, not via the plugin manager.

Note - this is a significant refactoring that will affect multiple internal modules; before starting, it's worth discussing the details with the core team or providing a more detailed plan about the internal changes.

Below is the proposition for moving logic from the WebhookPlugin into the core modules.

New files structure

saleor
  graphql
    webhook
      subscriptions
        |_ payload.py  // logic to build the payload moved from `subscription_payload.py`
        types  // moved from `subscription_types.py` into smaller modules
          |_ product.py
          |_ order.py
          |_ ...
  webhook
    events  // functions that directly trigger events; moved from plugins/webhook/plugin.py
      |_ product.py  // product events
      |_ order.py    // order events
      |_ ...         // other event files
    transport
      |_ async  // functions to send async events (such as `trigger_webhooks_async`, `send_webhook_request_async` task etc.)
        |_ transport.py
        |_ tasks.py
      |_ sync  // functions to send sync events
        |_ transport.py
        |_ tasks.py
      |_ utils.py            // helpers needed for the above two modules to work

Todo

• Move functions from saleor/plugins/webhook/plugin.py to saleor/webhook/events. Add webhook calls in the logic to use new functions, keeping the manager calls for other plugins to work.
  • All the transport and utility functions can be imported from the plugin at this step.
  • This work can be split into smaller PRs, module by module.
• Move transport functions from saleor/plugins/webhook/tasks.py to saleor/webhook/transport.
• Split saleor/graphql/webhook/subscription_types.py into smaller files (Split subscription_types.py into smaller modules #13470).

Few questions according to the structure.

• Can you also include here, where test files will be moved? (some of them are in graphql/webhook/, some in saleor/webhook and some in plugins/webhook. I know it is a detail, but would be good to clean up this part also.
• celery tasks should be placed in tasks.py file (it is celery pattern). The celery task should trigger the logic from async_transport.py or sync_transport.py
• where do we store subscription_types saleor/graphql/webhook/subscription_types.py?
• where do we store the logic that builds the payload based on subscription ?
• Are we ok in making imports in saleor/webhooks from saleor/graphql - we will need to do that to prepare the subscription types?

@korycins Updated the structure to include your points. Some comments:

• tests will be as close as possible to modules they're testing, same as we do currently with every other module
• tasks can go to tasks.py and be put into proper directory e.g. saleor/webhook/transport/async. AFAIR only async events have Celery tasks.
• we're not importing from GraphQL layer in saleor/webhooks, this logic is done entirely in saleor/graphql/webhooks

tasks can go to tasks.py and be put into proper directory e.g. saleor/webhook/transport/async. AFAIR only async events have Celery tasks.

No. We have an sync webhook that is called in celery task. See handle_transaction_request_task

tests will be as close as possible to modules they're testing, same as we do currently with every other module

I asked about this because, currently we have webhook tests in 3 different locations. It would be great to clean it up

we're not importing from GraphQL layer in saleor/webhooks, this logic is done entirely in saleor/graphql/webhooks

Does it mean that subscription types will be stored in saleor/grapqhl/webhooks, and saleor/webhook will be importing the subscription logic to build the payload?

I would stick to the rule that tests are next to the logic being tested, so it's a matter of organizing the logic first. Now we have two major parts:

• saleor/webhook
• saleor/graphql/webhook
  Now the tests would also be in these two places.

Currently, the webhook plugin is importing the payload generator from the GraphQL layer. In the new approach the transport layer would import from GraphQL to build subscriptions. It makes sense to keep subscription definitions in the GraphQL layer; we can consider where to put the generator itself. But since we're using GraphQL to define payloads, I think we need to import parts of this logic at some point, so I'm not sure it's a bad pattern in this case.

@maarcingebala I would also consider a different form of storing subscription types. There is no reason to store them in saleor/graphql/webhook module. They belong to their own modules, and we could consider moving them there.
So for example, orderFullyPaid, orderCreated could be stored in saleor/graphql/order. We could have a separate file for them. This would help us to keep everything connected in single place.

We discussed this topic with @fowczarek @korycins and @Air-t and here is the new structure we all agreed on.

Files structure

📦 saleor
 ├ 📂 product
 | └ 📂 webhooks
 |   └ 📜 product_created.py  // defines the `ProductCreated` class
 ├ ...
 ├ 📂 graphql
 | └ 📂 product
 |   ├ 📜 subscriptions.py  // `ProductCreated` etc.; moved from `subscription_types.py`
 |   └ 📂 tests
 |     └ 📂 subscriptions
 |       └ 📜 test_product.py  // move tests of product subscriptions
 ├ ...
 └ 📂 webhook
    └ 📂 transport
      ├ 📂 sync
      |  ├ 📜 transport.py  // defines AsyncWebhookTransport class
      |  └ 📜 tasks.py  // related Celery tasks
      └ 📂 async
         ├ 📜 transport.py  // defines SyncWebhookTransport
         └ 📜 tasks.py  // related Celery tasks

Assumptions:

• Each webhook will be moved into a separate file, e.g. saleor/product/webhooks/product_created.py. This file will define a class with webhook's configuration: name, permission, event type, legacy payload definition, etc. (example below).
• Subscriptions will be defined in the GraphQL layer e.g. saleor/graphql/product/subscriptions.py - will define product-related subscriptions such as ProductCreated.
• We'll try to move tests as closely as possible to the tested code.
  • For example, we'll try to split saleor/plugins/webhook/tests/subscription_webhooks/ tests and move them closer to subscriptions.
• saleor/plugins/webhook/tasks.py will be split into multiple files to separate async and sync logic (saleor/webhook/transport).

Example webhook definition

Each webhook will be defined in a separate file as a class. Example:

from saleor.webhook.transport import AsyncWebhookTransport


class ProductCreated(AsyncWebhookTransport):

    name = "Product Created"
    permission = "MANAGE_PRODUCTS"
    event_type = "PRODUCT_CREATED"  # to replace WebhookEventAsyncType.PRODUCT_CREATED usages in codebase
    subscription_type = "saleor.graphql.product.subscriptions.ProductCreated"

    def legacy_payload(self):
        # definition of the static legacy payload
        pass

Assumptions:

• Event maps such as class WebhookEventAsyncType, EVENT_MAP, WEBHOOK_TYPES_MAP will be generated dynamically by importing all classes, similarly to how GraphQL queries and mutations are imported in api.py.
• saleor/webhook/transport will define two base classes handling the webhook transport logic: AsyncWebhookTransport and SyncWebhookTransport. These classes will define common logic used to deliver webhooks in a unified way.

Other notes

• Move webhooks one by one in separate PRs to keep the scope of PRs small. First move an async event (e.g. ORDER_CREATED), then a sync one (e.g. SHIPPING_LIST_METHODS_FOR_CHECKOUT).