search

Shopify / shopify-app-template-remix

375 stars 151 forks source link

Admin context vs SHOP_REDACT webhook #808

Closed anastis closed 3 months ago

anastis commented 3 months ago

So, this is more of a question, and perhaps an issue in the routes/webhooks.tsx template.

The SHOP_REDACT webhook, as per the documentation, is triggered at least after 48hrs after the app has been uninstalled.

In webhooks.tsx it's pointed out (and checked against), that admin context isn't returned if the app is uninstalled, and code execution stops there. https://github.com/Shopify/shopify-app-template-remix/blob/6c12d55d544aee08118560ae9361f2e83282fd5b/app/routes/webhooks.tsx#L8-L11

However, SHOP_REDACT is handled further down, after the admin context check. https://github.com/Shopify/shopify-app-template-remix/blob/6c12d55d544aee08118560ae9361f2e83282fd5b/app/routes/webhooks.tsx#L24 However, this webhook is meant to run after the app is uninstalled, so there will be no admin context, and therefore this case statement is not reachable.

So, am I right to assume that shop/redact should be handled before the admin context check?

Having said/asked that, as soon as I add handling before the admin check, the APP_UNINSTALLED handler is grayed out by my editor. image

As I'm new to js/ts/remix/shopify-apps, I assume the static analysis tools know more than I do, so there's probably something there that I'm missing.

Could you please shed some light?

Simulating the webhook with shopify app webhook doesn't really help understand either, as it uses a hardcoded shop subdomain that isn't associated with a session nor admin context.

Thank you

lizkenyon commented 3 months ago

Hi there 👋

I think you are seeing two separate issues.

1) Great callout on the admin context not being available during the SHOP_REDACT webhook event. I put up this PR to modify the template to allow this webhook to process. Though your solution you have here should work as well!

2) In your editor I believe you are seeing a type issue with the webhook topic types, which is why the editor is graying out the APP_UNINSTALLED case. This should be resolved if you update the shopify-app-remix package to the latest version.

anastis commented 3 months ago

Thank you very much! Your solution looks so much cleaner!