-
Notifications
You must be signed in to change notification settings - Fork 64
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know of any other endpoint that might make sense, this was the cheapest one I could think of.
I'd rather not have to make the request at all, but we have no other means of detecting whether a token hasn't been manually destroyed (e.g. by the merchant logging out of their Admin).
Co-authored-by: Paulo Margarido <[email protected]>
@@ -29,7 +29,7 @@ export function verifyToken(routes: Routes, accessMode: AccessMode = DEFAULT_ACC | |||
try { | |||
// make a request to make sure oauth has succeeded, retry otherwise | |||
const client = new Shopify.Clients.Rest(session.shop, session.accessToken) | |||
await client.get({ path: "metafields", query: {'limit': 1} }) | |||
await client.get({path: 'shop'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I know what the response of this updated path going to be? Just trying to understand if it would cause any latency issues as I've seen metafields API timing out during auth process multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @snehapawar9910
Just trying to understand if it would cause any latency issues as I've seen metafields API timing out during auth process multiple times.
I don't think you should see any of these issues with this change. The response body should just be an object with your shop name [and other details] - Paulo and I agree this is the cheapest call we can make here. As I mentioned above I am not a huge fan of making a request for data to check on the session, but, I wanted to unblock those using this library from not being able to publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JaKXz any reason to why the metafields
REST endpoint is now deprecated? From my understanding, as of 2022-04
, this REST endpoint still exists and nothing looks to be deprecated: https://shopify.dev/api/admin-rest/2022-04/resources/metafield#get-metafields
Is it maybe because this code calls into an older API version (or it's calling the un-versioned endpoint), and the deprecation you are mentioning is the value_type
deprecation? The reason I ask is that our app still relies on the API call I've quoted below, and I'd prefer to keep relying on the metafields
resource and version it to 2022-01
rather than switch over to the shop
resource.
Some inputs would be great here... :)
const response = await fetch(
`https://${session.shop}/admin/metafields.json`,
{
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-Shopify-Access-Token': session.accessToken,
},
},
);
WHY are these changes introduced?
Fixes #118
We were using a call to the metafields endpoint to verify the oauth token was correct. Unfortunately that call has been deprecated so this solution should resolve the app verification issues.
WHAT is this pull request doing?
This changes the path to the longer lived shop endpoint to solve our short-term issues.
@paulomarg @mllemango do we have an endpoint that's just a heartbeat / session check? I think this could be improved if we did.
Type of change
Checklist