Add webhook _id and api_version to webhook handler

[lizkenyon]

Description

Fixes #1247

Shopify recommends as a best practice to check the x-shopify-webhook-id to verify you are not processing duplicate webhooks. We currently are not passing this information into the webhook handler. This PR will add the webhook_id and api_version to the webhook handler to bring the ruby library in line with the Javascript library in terms of information the handler has access to.

This could be resolved in a couple of approaches outlined below

Add webhook_id and api_version to handle function

This is what is currently implemented in this PR. The webhook_id and api_version are added to the webhook request object from the headers. They are then explicitly passed to the handle function.

This would be a breaking change, but would be a straightforward fix. Developers would need to add webhook_id and api_version to their handle implementation.

Add metadata

If we are concerned, that in the future we may need to add even more information to the handle function and would require more breaking changes we could instead change the method signature to include the body and metadata. Where metadata could be a catch all of all the additional information. This would make any future additions easier but would require slight more initial refactoring for developers on the initial change.

      sig do
        abstract.params(body: T::Hash[String, T.untyped], metadata: T::Hash[String, T.untyped]).void
      end
      def handle(body:, metadata); end
    end

Add new handle function

If we are concerned about introducing a breaking change, we could introduce a new handle function. (eg. handle_with_webhook_and_version then developers would be able to implement the new function if they wanted access to the webhook_id and api_version. This could get complicated if we need to add more information in the future. (We would need to add another new function?)

How has this been tested?

Please, describe the tests that you ran to verify your changes.

Checklist:

• [ ] My commit message follow the pattern described in here
• [ ] I have performed a self-review of my own code.
• [ ] I have added tests that prove my fix is effective or that my feature works.
• [ ] I have updated the project documentation.
• [ ] I have added a changelog line.

[paulomarg]

I think we can make this a minor change by adding a static value to the webhook handler - if we add

module Handler
  FLAG_TO_USE_ALL_FIELDS = T.let(false, T::Boolean)

and then do

module TestHelpers
  class FakeWebhookHandler
    include ShopifyAPI::Webhooks::Handler
    FLAG_TO_USE_ALL_FIELDS = true

we can then use if handler.class::USE_ALL_FIELDS in the registry to call the appropriate version of the function.

We can add deprecation logs when using the old format to give folks time to fix it before a major version. We'd need to declare the abstract class in a way that could deal with both cases, but I think that should be possible with some creative typing?

Do you think that would work?

I agree that there is a bit of a deeper problem here. I see two ways that apps can make this future-proof:

• declare their handlers with a **metadata argument
• change the function to take in a hash / object

I'd lean towards the second option because the first one is somewhat easy to forget and can trip people up - the second option is more reliable IMO.

[lizkenyon]

Tested this and verified that: 1) The deprecation warning is shown when you use the old handler 2) When using the new handler you have access to the additional fields.