[bugfix] Ensure consistent behaviour when using access token from session in GQL and REST clients #1772
Conversation
sle-c
changed the title
.changeset/clever-taxis-retire.md
Outdated
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@shopify/shopify-api': minor | |||
| @@ -53,10 +53,16 @@ export class GraphqlClient { | |||
| logger(config).debug(message); | |||
| } | |||
|
|
|||
| const customStoreAppAccessToken = | |||
There was a problem hiding this comment.
I was a little confused on why this logic existed.
config.apiSecretKey = The apps private key
config.adminApiAccessToken = The apps access token for requests.
If we create a graphql client with the app private key, we would expect requests to fail.
Seems like in the past we had told folks when setting up a merchant custom app to put the access token = config.apiSecretKey. And then later added the config.adminApiAccessToken. And this logic is to handle folks that never migrated.
We could deprecate this logic, and require merchant custom apps to set both values.
There was a problem hiding this comment.
yeah, indeed this has been deprecated
ed71298#diff-3bfd67f947f1e7d5cca6fba729bd99512b17d76b16303b6d00b235e1fb6a9237R91-R96
I think we should take this opportunity to deprecate it for good
|
holding off on merging because it's technically a breaking change for a number of apps |
WHY are these changes introduced?
Fixes #1756
Passing real session to REST and GQL clients result in inconsistent behaviour. This is due to a custom
ifcondition in the REST client that the GQL client doesn't have.This only happens when using the
testConfigtest helper. While the consistent behaviour between the two is highly encouraged. It's not recommended to use a real session when using atestConfig. It's supposed to be a quick helper for unit tests, so all requests should be mocked.For e2e tests, use a real config that points to a test Shopify store.
WHAT is this pull request doing?
This pull request includes changes to make the GraphQL (GQL) client behavior on custom app configurations consistent with the REST client. The most important changes include adding tests to ensure the correct behavior and updating the client classes to handle custom store app access tokens appropriately.
Consistent Behavior for Custom App Configurations:
.changeset/clever-taxis-retire.md: Added a minor bugfix entry to make GQL client behavior on custom app configurations consistent with the REST client.Test Enhancements:
packages/apps/shopify-api/lib/clients/admin/__tests__/admin_graphql_client.test.ts: Added a test to verify that the GraphQL client adapts to private app requests only ifisCustomStoreAppis set to false.packages/apps/shopify-api/lib/clients/admin/__tests__/rest_client.test.ts: Added a test to verify that the REST client adapts to private app requests only ifisCustomStoreAppis set to false.Client Class Updates:
packages/apps/shopify-api/lib/clients/admin/graphql/client.ts: Updated theGraphqlClientclass to use the appropriate access token based on theisCustomStoreAppconfiguration.Type of change
Checklist
pnpm changesetto create a draft changelog entry (do NOT update theCHANGELOG.mdfiles manually)