Check if a webhook is registered before calling Shopify

[paulomarg]

WHY are these changes introduced?

Our current webhook code runs into issues if we try to register a webhook that is already there, because Shopify won't accept a webhookSubscriptionCreate call if there is already a handler in place for that topic. It also doesn't accept webhookSubscriptionUpdate calls if the callback URL hasn't changed.

This PR aims to make sure the library works around those restrictions so that apps can simply request to register a webhook and trust it will be there.

WHAT is this pull request doing?

It defines a few possible execution paths for webhook registration. First of all, we check if there is already a handler for the given topic. Then:

• If the topic has never been registered, we simply register it as before, and add it to the registry's callbacks
• If the topic exists, but its callback URL changed, we update the address on Shopify before adding it to the registry
• If it exists and the URL is the same, we simply skip calling Shopify entirely and add it to the registry

NOTE: one unintended side effect of this change is that we're always adding new entries to the registry (as in on every successful OAuth flow). This was masked in the original code by the fact that we never succeeded in subsequent register calls because Shopify would fail. I will patch that up so that we replace the callback in the registry as well if it is there.
UPDATE: the above note was properly fixed by the PR.

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
• Minor: New feature (non-breaking change which adds functionality)
• Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

[carmelal]

This all makes sense to me, I think!

Question, though: what if the user doesn't realize the topic already exists, is it just up to them to make sure not to accidentally overwrite something unintentionally? Is there a way for us to make it harder for them to mess up?

[paulomarg]

That's a very good question. I would argue that the overwrite is desirable - the only way to register webhooks is via the app code itself, so the app has the final word in how it handles a certain topic. The latest running version of the app should always dictate how to handle webhooks as we don't want the registered values to get out of sync with the app code for a topic.

Another point to consider is that the only thing we allow changing is the endpoint that gets called for a topic if it is already registered with Shopify.

This is my personal view on it though - I'd like to hear how others feel about this!