Skip to content

Commit

Permalink
Merge pull request #1430 from Shopify/change-revoke-scope-return-value
Browse files Browse the repository at this point in the history
Update revoke operation to return revoked scopes instead of all scopes details
  • Loading branch information
JonathanJChang authored Aug 28, 2024
2 parents 4d3364f + ce0ff32 commit 4ed3fe6
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/three-dryers-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/shopify-app-remix': patch
---

Return revoked scopes instead of querying for scopes after revoking
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,37 @@ export const WITH_GRANTED_AND_DECLARED = buildGraphqlResponseContent({
});

export const REVOKED_WITHOUT_ERROR = buildGraphqlResponseContent({
revoked: [
{
handle: 'read_orders',
},
],
appRevokeAccessScopes: {
revoked: [
{
handle: 'write_discounts',
},
{
handle: 'read_orders',
},
],
userErrors: [],
},
});

export const REVOKED_NOTHING = buildGraphqlResponseContent({
appRevokeAccessScopes: {
revoked: [],
userErrors: [],
},
});

export const REVOKED_WITH_ERROR = buildGraphqlResponseContent({
userErrors: [
{
field: 'scopes',
messages:
'The requested list of scopes to revoke includes invalid handles.',
},
],
appRevokeAccessScopes: {
revoked: null,
userErrors: [
{
field: 'scopes',
messages:
'The requested list of scopes to revoke includes invalid handles.',
},
],
},
});

function buildGraphqlResponseContent(content: any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,36 @@ import {

import * as responses from './mock-responses';

it('returns scopes information', async () => {
it('returns successfully revoked scopes', async () => {
// GIVEN
const {scopes} = await setUpEmbeddedFlow();
await mockGraphqlRequests()(
{
body: 'AppRevokeAccessScopes',
responseContent: responses.REVOKED_WITHOUT_ERROR,
},
{
body: 'FetchAccessScopes',
responseContent: responses.WITH_GRANTED_AND_DECLARED,
},
);
await mockGraphqlRequests()({
body: 'AppRevokeAccessScopes',
responseContent: responses.REVOKED_WITHOUT_ERROR,
});

// WHEN
const result = await scopes.revoke(['write_discounts', 'read_orders']);

// THEN
expect(result).not.toBeUndefined();
expect(result.revoked).toEqual(['write_discounts', 'read_orders']);
});

it('returns successfully with empty list when graphql returns an empty list for the revoke operation', async () => {
// GIVEN
const {scopes} = await setUpEmbeddedFlow();
await mockGraphqlRequests()({
body: 'AppRevokeAccessScopes',
responseContent: responses.REVOKED_NOTHING,
});

// WHEN
const result = await scopes.revoke(['read_orders']);

// THEN
expect(result).not.toBeUndefined();
expect(result.detail.granted).toEqual(['read_orders', 'write_customers']);
expect(result.detail.required).toEqual(['read_orders', 'read_reports']);
expect(result.detail.optional).toEqual(['write_customers']);
expect(result.revoked).toEqual([]);
});

it('returns error if the list of scopes is empty', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ export async function revokeScopes(
});

const resultContent = await revokeScopesResult.json();
return resultContent.data;
return resultContent.data.appRevokeAccessScopes;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import {AdminApiContext} from '../../../clients';
import type {BasicParams} from '../../../types';

import {revokeScopes} from './client/revoke-scopes';
import {fetchScopeDetail} from './client/fetch-scopes-details';
import {mapFetchScopeDetail} from './query';

export function revokeScopesFactory(
params: BasicParams,
Expand All @@ -15,13 +13,13 @@ export function revokeScopesFactory(
return async function revoke(scopes: string[]) {
const {logger} = params;

await validateScopes(scopes);

logger.debug('Revoke scopes: ', {
shop: session.shop,
scopes,
});

await validateScopes(scopes);

const revokeScopesResult = await revokeScopes(admin, scopes);
if (revokeScopesResult.userErrors?.length > 0) {
logger.error('Failed to revoke scopes: ', {
Expand All @@ -37,9 +35,8 @@ export function revokeScopesFactory(
});
}

const scopesDetail = await fetchScopeDetail(admin);
return {
detail: mapFetchScopeDetail(scopesDetail),
revoked: revokeScopesResult.revoked.map((scope) => scope.handle),
};
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export interface ScopesApiContext {
}

export interface RevokeResponse {
detail: ScopesDetail;
revoked: string[];
}
export interface ScopesDetail {
granted: string[];
Expand Down

0 comments on commit 4ed3fe6

Please sign in to comment.