Skip to content

Commit e8ae3fc

Browse files
rossiamrossiam
committed
fix: cleanup smartapps 403 handling
1 parent 11f9545 commit e8ae3fc

17 files changed

Lines changed: 136 additions & 103 deletions

File tree

‎src/__tests__/commands/smartapps.test.ts‎

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,7 @@ import type { outputItemOrList, outputItemOrListBuilder } from '../../lib/comman
2424
import type { CommandArgs } from '../../commands/smartapps.js'
2525
import type { ListDataFunction } from '../../lib/command/io-defs.js'
2626
import type { BuildOutputFormatterFlags } from '../../lib/command/output-builder.js'
27-
import type {
28-
paidAccountRequiredHandler,
29-
shortARNorURL,
30-
verboseSmartApps,
31-
} from '../../lib/command/util/smartapps-util.js'
27+
import type { shortARNorURL, verboseSmartApps } from '../../lib/command/util/smartapps-util.js'
3228
import { buildArgvMock, buildArgvMockStub } from '../test-lib/builder-mock.js'
3329

3430

@@ -61,14 +57,9 @@ jest.unstable_mockModule('../../lib/command/listing-io.js', () => ({
6157
outputItemOrListBuilder: outputItemOrListBuilderMock,
6258
}))
6359

64-
const paidAccountRequiredErrorHandlerMock = jest.fn<ReturnType<typeof paidAccountRequiredHandler>>()
65-
.mockImplementation(() => { throw Error('should exit') })
66-
const paidAccountRequiredHandlerMock = jest.fn<typeof paidAccountRequiredHandler>()
67-
.mockReturnValue(paidAccountRequiredErrorHandlerMock)
6860
const shortARNorURLMock = jest.fn<typeof shortARNorURL>()
6961
const verboseSmartAppsMock = jest.fn<typeof verboseSmartApps>()
7062
jest.unstable_mockModule('../../lib/command/util/smartapps-util.js', () => ({
71-
paidAccountRequiredHandler: paidAccountRequiredHandlerMock,
7263
shortARNorURL: shortARNorURLMock,
7364
verboseSmartApps: verboseSmartAppsMock,
7465
}))
@@ -268,30 +259,6 @@ describe('handler', () => {
268259
expect(verboseSmartAppsMock).toHaveBeenCalledTimes(0)
269260
})
270261

271-
it('uses paid account required handler for errors', async () => {
272-
const listItems = await listAppsForArgs({})
273-
274-
const error = { response: { status: 403 } }
275-
apiAppsListMock.mockRejectedValueOnce(error)
276-
277-
await expect(listItems()).rejects.toThrow('should exit')
278-
279-
expect(paidAccountRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith('Listing Smart Apps')
280-
expect(paidAccountRequiredErrorHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
281-
})
282-
283-
it('uses paid account required handler for errors in verbose mode', async () => {
284-
const listItems = await listAppsForArgs({ verbose: true })
285-
286-
const error = { response: { status: 403 } }
287-
verboseSmartAppsMock.mockRejectedValueOnce(error)
288-
289-
await expect(listItems()).rejects.toThrow('should exit')
290-
291-
expect(paidAccountRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith('Listing Smart Apps')
292-
expect(paidAccountRequiredErrorHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
293-
})
294-
295262
it('omits accountId when no organization header is set', async () => {
296263
clientMock.config.headers = {}
297264

‎src/__tests__/commands/smartapps/confirm.test.ts‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
} from '../../../lib/command/api-organization-command.js'
1515
import type { ChooseFunction } from '../../../lib/command/util/util-util.js'
1616
import type { chooseSmartAppFn } from '../../../lib/command/util/smartapps-choose.js'
17+
import type { paidPlanRequiredHandler } from '../../../lib/command/util/smartapps-util.js'
1718
import { buildArgvMock } from '../../test-lib/builder-mock.js'
1819

1920

@@ -35,6 +36,12 @@ jest.unstable_mockModule('../../../lib/command/util/smartapps-choose.js', () =>
3536
chooseSmartAppFn: chooseSmartAppFnMock,
3637
}))
3738

39+
const paidPlanRequiredHandlerMock = jest.fn<typeof paidPlanRequiredHandler>()
40+
.mockImplementation(() => { throw Error('should exit') })
41+
jest.unstable_mockModule('../../../lib/command/util/smartapps-util.js', () => ({
42+
paidPlanRequiredHandler: paidPlanRequiredHandlerMock,
43+
}))
44+
3845
const consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => { /*no-op*/ })
3946

4047

@@ -63,7 +70,7 @@ test('builder', () => {
6370
expect(epilogMock).toHaveBeenCalledTimes(1)
6471
})
6572

66-
test('handler', async () => {
73+
describe('handler', () => {
6774
const apiAppsRegisterMock = jest.fn<typeof SmartAppsEndpoint.prototype.register>()
6875
const command = {
6976
client: {
@@ -72,19 +79,37 @@ test('handler', async () => {
7279
config: { headers: { 'X-ST-Organization': 'organization-id' } },
7380
},
7481
} as unknown as APIOrganizationCommand<ArgumentsCamelCase<CommandArgs>>
75-
apiOrganizationCommandMock.mockResolvedValueOnce(command)
76-
chooseSmartAppMock.mockResolvedValueOnce('chosen-id')
7782
const inputArgv = {
7883
profile: 'default',
7984
id: 'id-from-cmd-line',
8085
} as ArgumentsCamelCase<CommandArgs>
8186

82-
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
87+
beforeEach(() => {
88+
apiOrganizationCommandMock.mockResolvedValueOnce(command)
89+
chooseSmartAppMock.mockResolvedValueOnce('chosen-id')
90+
})
91+
92+
it('sends confirmation request', async () => {
93+
apiAppsRegisterMock.mockResolvedValueOnce()
94+
95+
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
96+
97+
expect(apiOrganizationCommandMock).toHaveBeenCalledExactlyOnceWith(inputArgv)
98+
expect(chooseSmartAppFnMock)
99+
.toHaveBeenCalledExactlyOnceWith({ appType: 'API_ONLY', accountId: 'organization-id' })
100+
expect(chooseSmartAppMock).toHaveBeenCalledExactlyOnceWith(command, 'id-from-cmd-line')
101+
expect(apiAppsRegisterMock).toHaveBeenCalledExactlyOnceWith('chosen-id')
102+
expect(paidPlanRequiredHandlerMock).not.toHaveBeenCalled()
103+
expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('Confirmation request sent'))
104+
})
105+
106+
it('uses paid plan required handler for errors confirming app', async () => {
107+
const error = { response: { status: 403 } }
108+
apiAppsRegisterMock.mockRejectedValueOnce(error)
109+
110+
await expect(cmd.handler(inputArgv)).rejects.toThrow('should exit')
83111

84-
expect(apiOrganizationCommandMock).toHaveBeenCalledExactlyOnceWith(inputArgv)
85-
expect(chooseSmartAppFnMock)
86-
.toHaveBeenCalledExactlyOnceWith({ appType: 'API_ONLY', accountId: 'organization-id' })
87-
expect(chooseSmartAppMock).toHaveBeenCalledExactlyOnceWith(command, 'id-from-cmd-line')
88-
expect(apiAppsRegisterMock).toHaveBeenCalledExactlyOnceWith('chosen-id')
89-
expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('Confirmation request sent'))
112+
expect(paidPlanRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
113+
expect(consoleLogSpy).not.toHaveBeenCalled()
114+
})
90115
})

‎src/__tests__/commands/smartapps/oauth/generate.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
import { userInputProcessor } from '../../../../lib/command/input-processor.js'
2626
import type { ChooseFunction } from '../../../../lib/command/util/util-util.js'
2727
import type { chooseSmartAppFn } from '../../../../lib/command/util/smartapps-choose.js'
28+
import type { paidPlanRequiredHandler } from '../../../../lib/command/util/smartapps-util.js'
2829
import type { objectDef, stringDef, updateFromUserInput } from '../../../../lib/item-input/index.js'
2930
import type { CommandArgs } from '../../../../commands/smartapps/oauth/generate.js'
3031
import { buildArgvMock, buildArgvMockStub } from '../../../test-lib/builder-mock.js'
@@ -70,6 +71,12 @@ jest.unstable_mockModule('../../../../lib/command/util/smartapps-input-primitive
7071
oauthAppScopeDef: oauthAppScopeDefMock,
7172
}))
7273

74+
const paidPlanRequiredHandlerMock = jest.fn<typeof paidPlanRequiredHandler>()
75+
.mockImplementation(() => { throw Error('should exit') })
76+
jest.unstable_mockModule('../../../../lib/command/util/smartapps-util.js', () => ({
77+
paidPlanRequiredHandler: paidPlanRequiredHandlerMock,
78+
}))
79+
7380
const updateFromUserInputMock = jest.fn<typeof updateFromUserInput>()
7481
const objectDefMock = jest.fn<typeof objectDef>()
7582
const stringDefMock = jest.fn<typeof stringDef>()
@@ -147,6 +154,19 @@ describe('handler', () => {
147154

148155
expect(apiAppsRegenerateOauthMock)
149156
.toHaveBeenCalledExactlyOnceWith('chosen-app-id', generateRequest)
157+
expect(paidPlanRequiredHandlerMock).not.toHaveBeenCalled()
158+
})
159+
160+
it('uses paid plan required handler for errors regenerating OAuth credentials', async () => {
161+
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
162+
const actionFunction = inputAndOutputItemMock.mock.calls[0][2]
163+
164+
const error = { response: { status: 403 } }
165+
apiAppsRegenerateOauthMock.mockRejectedValueOnce(error)
166+
167+
await expect(actionFunction(undefined, {} as GenerateSmartAppOAuthRequest)).rejects.toThrow('should exit')
168+
169+
expect(paidPlanRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
150170
})
151171

152172
it('accepts app id from command line', async () => {

‎src/__tests__/commands/smartapps/oauth/update.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type {
2626
import { userInputProcessor } from '../../../../lib/command/input-processor.js'
2727
import type { ChooseFunction } from '../../../../lib/command/util/util-util.js'
2828
import { type chooseSmartAppFn } from '../../../../lib/command/util/smartapps-choose.js'
29+
import type { paidPlanRequiredHandler } from '../../../../lib/command/util/smartapps-util.js'
2930
import type { objectDef, stringDef, updateFromUserInput } from '../../../../lib/item-input/index.js'
3031
import type { CommandArgs } from '../../../../commands/smartapps/oauth/update.js'
3132
import { buildArgvMock, buildArgvMockStub } from '../../../test-lib/builder-mock.js'
@@ -75,6 +76,12 @@ jest.unstable_mockModule('../../../../lib/command/util/smartapps-input-primitive
7576
corsDomainsDef: corsDomainsDefMock,
7677
}))
7778

79+
const paidPlanRequiredHandlerMock = jest.fn<typeof paidPlanRequiredHandler>()
80+
.mockImplementation(() => { throw Error('should exit') })
81+
jest.unstable_mockModule('../../../../lib/command/util/smartapps-util.js', () => ({
82+
paidPlanRequiredHandler: paidPlanRequiredHandlerMock,
83+
}))
84+
7885
const updateFromUserInputMock = jest.fn<typeof updateFromUserInput>()
7986
const objectDefMock = jest.fn<typeof objectDef>()
8087
const stringDefMock = jest.fn<typeof stringDef>()
@@ -152,6 +159,19 @@ describe('handler', () => {
152159

153160
expect(apiAppsUpdateOauthMock)
154161
.toHaveBeenCalledExactlyOnceWith('chosen-app-id', updateRequest)
162+
expect(paidPlanRequiredHandlerMock).not.toHaveBeenCalled()
163+
})
164+
165+
it('uses paid plan required handler for errors updating OAuth settings', async () => {
166+
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
167+
const actionFunction = inputAndOutputItemMock.mock.calls[0][2]
168+
169+
const error = { response: { status: 403 } }
170+
apiAppsUpdateOauthMock.mockRejectedValueOnce(error)
171+
172+
await expect(actionFunction(undefined, {} as SmartAppOAuthRequest)).rejects.toThrow('should exit')
173+
174+
expect(paidPlanRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
155175
})
156176

157177
it('accepts app id from command line', async () => {

‎src/__tests__/commands/smartapps/settings/update.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type {
2323
import type { ChooseFunction } from '../../../../lib/command/util/util-util.js'
2424
import type { chooseSmartAppFn } from '../../../../lib/command/util/smartapps-choose.js'
2525
import type { buildTableOutput } from '../../../../lib/command/util/smartapps-table.js'
26+
import type { paidPlanRequiredHandler } from '../../../../lib/command/util/smartapps-util.js'
2627
import type { CommandArgs } from '../../../../commands/smartapps/settings/update.js'
2728
import { buildArgvMock, buildArgvMockStub } from '../../../test-lib/builder-mock.js'
2829
import { CustomCommonOutputProducer } from '../../../../lib/command/format.js'
@@ -59,6 +60,12 @@ jest.unstable_mockModule('../../../../lib/command/util/smartapps-table.js', () =
5960
buildTableOutput: buildTableOutputMock,
6061
}))
6162

63+
const paidPlanRequiredHandlerMock = jest.fn<typeof paidPlanRequiredHandler>()
64+
.mockImplementation(() => { throw Error('should exit') })
65+
jest.unstable_mockModule('../../../../lib/command/util/smartapps-util.js', () => ({
66+
paidPlanRequiredHandler: paidPlanRequiredHandlerMock,
67+
}))
68+
6269

6370
const { default: cmd } = await import('../../../../commands/smartapps/settings/update.js')
6471

@@ -125,6 +132,7 @@ describe('handler', () => {
125132
expect(await executeFunction(undefined, settings)).toBe(updatedSettings)
126133

127134
expect(apiAppsUpdateSettingsMock).toHaveBeenCalledExactlyOnceWith('chosen-id', settings)
135+
expect(paidPlanRequiredHandlerMock).not.toHaveBeenCalled()
128136

129137
const config = inputAndOutputItemMock.mock.calls[0][1] as
130138
CustomCommonOutputProducer<SmartAppSettingsResponse>
@@ -135,6 +143,18 @@ describe('handler', () => {
135143
expect(buildTableOutputMock).toHaveBeenCalledExactlyOnceWith(tableGeneratorMock, updatedSettings)
136144
})
137145

146+
it('uses paid plan required handler for errors updating settings', async () => {
147+
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
148+
const executeFunction = inputAndOutputItemMock.mock.calls[0][2]
149+
150+
const error = { response: { status: 403 } }
151+
apiAppsUpdateSettingsMock.mockRejectedValueOnce(error)
152+
153+
await expect(executeFunction(undefined, {} as SmartAppSettingsResponse)).rejects.toThrow('should exit')
154+
155+
expect(paidPlanRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
156+
})
157+
138158
it('passes command line id on to chooseApp', async () => {
139159
await expect(cmd.handler({ ...inputArgv, id: 'argv-id' })).resolves.not.toThrow()
140160

‎src/__tests__/commands/smartapps/update.test.ts‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type {
2323
import type { InputProcessor, userInputProcessor } from '../../../lib/command/input-processor.js'
2424
import type { ChooseFunction } from '../../../lib/command/util/util-util.js'
2525
import type { chooseSmartAppFn } from '../../../lib/command/util/smartapps-choose.js'
26-
import type { paidAccountRequiredHandler } from '../../../lib/command/util/smartapps-util.js'
26+
import type { paidPlanRequiredHandler } from '../../../lib/command/util/smartapps-util.js'
2727
import type { getAppUpdateRequestFromUser } from '../../../lib/command/util/smartapps-user-input-update.js'
2828
import type { CommandArgs } from '../../../commands/smartapps/update.js'
2929
import { buildArgvMock, buildArgvMockStub } from '../../test-lib/builder-mock.js'
@@ -63,12 +63,10 @@ jest.unstable_mockModule('../../../lib/command/util/smartapps-choose.js', () =>
6363
chooseSmartAppFn: chooseSmartAppFnMock,
6464
}))
6565

66-
const paidAccountRequiredErrorHandlerMock = jest.fn<ReturnType<typeof paidAccountRequiredHandler>>()
66+
const paidPlanRequiredHandlerMock = jest.fn<typeof paidPlanRequiredHandler>()
6767
.mockImplementation(() => { throw Error('should exit') })
68-
const paidAccountRequiredHandlerMock = jest.fn<typeof paidAccountRequiredHandler>()
69-
.mockReturnValue(paidAccountRequiredErrorHandlerMock)
7068
jest.unstable_mockModule('../../../lib/command/util/smartapps-util.js', () => ({
71-
paidAccountRequiredHandler: paidAccountRequiredHandlerMock,
69+
paidPlanRequiredHandler: paidPlanRequiredHandlerMock,
7270
}))
7371

7472
const getAppUpdateRequestFromUserMock = jest.fn<typeof getAppUpdateRequestFromUser>()
@@ -163,18 +161,7 @@ describe('handler', () => {
163161
expect(getAppUpdateRequestFromUserMock).toHaveBeenCalledExactlyOnceWith(command, 'chosen-app-id')
164162
})
165163

166-
it('uses paid account required handler for errors choosing app', async () => {
167-
const error = { response: { status: 403 } }
168-
chooseSmartAppMock.mockRejectedValueOnce(error)
169-
170-
await expect(cmd.handler(inputArgv)).rejects.toThrow('should exit')
171-
172-
expect(paidAccountRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith('Listing Smart Apps')
173-
expect(paidAccountRequiredErrorHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
174-
expect(inputAndOutputItemMock).not.toHaveBeenCalled()
175-
})
176-
177-
it('uses paid account required handler for errors updating app', async () => {
164+
it('uses paid plan required handler for errors updating app', async () => {
178165
await expect(cmd.handler(inputArgv)).resolves.not.toThrow()
179166
const executeUpdate = inputAndOutputItemMock.mock.calls[0][2]
180167

@@ -183,8 +170,7 @@ describe('handler', () => {
183170

184171
await expect(executeUpdate(undefined, {} as SmartAppUpdateRequest)).rejects.toThrow('should exit')
185172

186-
expect(paidAccountRequiredHandlerMock).toHaveBeenCalledWith('Updating a Smart App')
187-
expect(paidAccountRequiredErrorHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
173+
expect(paidPlanRequiredHandlerMock).toHaveBeenCalledExactlyOnceWith(error)
188174
})
189175

190176
it('passes resolved organization header to chooseSmartAppFn as accountId', async () => {

‎src/__tests__/lib/command/util/smartapps-util.test.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,39 +18,35 @@ jest.unstable_mockModule('../../../../lib/util.js', () => ({
1818

1919

2020
const {
21-
paidAccountRequiredHandler,
21+
paidPlanRequiredHandler,
2222
shortARNorURL,
2323
verboseSmartApps,
2424
} = await import('../../../../lib/command/util/smartapps-util.js')
2525

2626

27-
describe('paidAccountRequiredHandler', () => {
27+
describe('paidPlanRequiredHandler', () => {
2828
it('exits with a helpful message on 403', () => {
29-
const handler = paidAccountRequiredHandler('Doing a Thing')
30-
31-
expect(() => handler({ response: { status: 403 } })).toThrow('should exit')
29+
expect(() => paidPlanRequiredHandler({ response: { status: 403 } })).toThrow('should exit')
3230

3331
expect(fatalErrorMock).toHaveBeenCalledExactlyOnceWith(
34-
'Doing a Thing requires a paid account. Visit' +
32+
'Creating or modifying a Smart App requires a paid plan. Visit' +
3533
'\n https://developer.smartthings.com/console/service-integrations/manage-plans' +
3634
'\nto upgrade.',
3735
)
3836
})
3937

4038
it('rethrows other errors', () => {
41-
const handler = paidAccountRequiredHandler('Doing a Thing')
4239
const otherError = { response: { status: 500 } }
4340

44-
expect(() => handler(otherError)).toThrow(expect.objectContaining(otherError))
41+
expect(() => paidPlanRequiredHandler(otherError)).toThrow(expect.objectContaining(otherError))
4542

4643
expect(fatalErrorMock).not.toHaveBeenCalled()
4744
})
4845

4946
it('rethrows errors without a response', () => {
50-
const handler = paidAccountRequiredHandler('Doing a Thing')
5147
const otherError = Error('network failure')
5248

53-
expect(() => handler(otherError)).toThrow(otherError)
49+
expect(() => paidPlanRequiredHandler(otherError)).toThrow(otherError)
5450

5551
expect(fatalErrorMock).not.toHaveBeenCalled()
5652
})

‎src/commands/smartapps.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@ import {
2727
outputItemOrListBuilder,
2828
} from '../lib/command/listing-io.js'
2929
import { tableFieldDefinitions } from '../lib/command/util/smartapps-table.js'
30-
import {
31-
paidAccountRequiredHandler,
32-
shortARNorURL,
33-
verboseSmartApps,
34-
} from '../lib/command/util/smartapps-util.js'
30+
import { shortARNorURL, verboseSmartApps } from '../lib/command/util/smartapps-util.js'
3531

3632

3733
export type CommandArgs = APIOrganizationCommandFlags & AllOrganizationFlags & OutputItemOrListFlags & {
@@ -69,7 +65,7 @@ const builder = (yargs: Argv): Argv<CommandArgs> =>
6965
['$0 smartapps', 'list all Smart Apps'],
7066
[
7167
'$0 smartapps 1',
72-
'display details for the first smart app in the list retrieved by running "smartthings smartapps"',
68+
'display details for the first Smart App in the list retrieved by running "smartthings smartapps"',
7369
],
7470
['$0 smartapps 5dfd6626-ab1d-42da-bb76-90def3153998', 'display details for a Smart App by id'],
7571
['$0 smartapps --verbose', 'include URLs and ARNs in the output'],
@@ -106,8 +102,7 @@ const handler = async (argv: ArgumentsCamelCase<CommandArgs>): Promise<void> =>
106102

107103
const listForClient = (client: SmartThingsClient, accountId?: string): Promise<ListedSmartApp[]> => {
108104
const options = accountId ? { ...appListOptions, accountId } : appListOptions
109-
return (argv.verbose ? verboseSmartApps(client, options) : client.smartapps.list(options))
110-
.catch(paidAccountRequiredHandler('Listing Smart Apps'))
105+
return argv.verbose ? verboseSmartApps(client, options) : client.smartapps.list(options)
111106
}
112107

113108
if (argv.allOrganizations) {

0 commit comments

Comments
 (0)