Adding APP_UNINSTALLED webhook #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MuhammadFarag
approved these changes
May 17, 2021
Comment on lines
+93
to
+106
| $response = Registry::register( | ||
| path: '/webhooks', | ||
| topic: Topics::APP_UNINSTALLED, | ||
| shop: $shop, | ||
| accessToken: $session->getAccessToken(), | ||
| ); | ||
| if ($response->isSuccess()) { | ||
| Log::debug("Registered APP_UNINSTALLED webhook for shop $shop"); | ||
| } else { | ||
| Log::error( | ||
| "Failed to register APP_UNINSTALLED webhook for shop $shop with response body: " . | ||
| print_r($response->getBody(), true) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Maybe we can think of a way to pull this code into the library. Webhooks path could be a configuration. We already know the topics from the Registery::addHandler. Not part of this PR though
There was a problem hiding this comment.
What I mean, we don't need to explicitly pass arguments to register. Maybe we can have a method registerAll that would register all webhooks.
There was a problem hiding this comment.
Hm, interesting, that's a really good idea. I just wonder if it wouldn't 'hide' the topics a little bit (and not sure if that's even a problem). We could also just make that an app method, though it's probably generic enough that it can go into the lib.
paulomarg
force-pushed
the
add-oauth-begin
branch
from
f0162db
to
148d64b
Compare
paulomarg
force-pushed
the
add_uninstall_webhook
branch
from
af6a78f
to
84216cb
Compare
Closed
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Now that we have OAuth working, we can add a webhook handler for the
APP_UNINSTALLEDevent, which will delete all of a shop's sessions in the database to make sure that shop goes through OAuth again if it ever reinstalls the app.WHAT is this pull request doing?
Using the library to handle the uninstallation webhook, and wiping all sessions for the shop when that happens, to ensure it is not considered installed in the future.
Type of change
Checklist