feat: force user to choose org for schema app before doing anything that will auto-assign one

Author: rossiam

.changeset/fuzzy-ants-burn.md

@@ -0,0 +1,5 @@
+---
+"@smartthings/cli": patch
+---
+
+Force user to choose organization for schema app before doing anything that will automatically assign one.

packages/cli/src/__tests__/commands/schema.test.ts

@@ -1,12 +1,14 @@
 import { SchemaApp, SchemaEndpoint } from '@smartthings/core-sdk'
 
-import { outputItemOrList, TableCommonListOutputProducer } from '@smartthings/cli-lib'
+import { APICommand, outputItemOrList, TableCommonListOutputProducer } from '@smartthings/cli-lib'
 
 import SchemaCommand from '../../commands/schema'
+import { getSchemaAppEnsuringOrganization } from '../../lib/commands/schema-util'
 
 
+jest.mock('../../lib/commands/schema-util')
+
 describe('SchemaCommand', () => {
-	const getSpy = jest.spyOn(SchemaEndpoint.prototype, 'get').mockImplementation()
 	const listSpy = jest.spyOn(SchemaEndpoint.prototype, 'list').mockImplementation()
 
 	const outputItemOrListMock = jest.mocked(outputItemOrList<SchemaApp>)
@@ -71,16 +73,22 @@ describe('SchemaCommand', () => {
 	})
 
 	it('calls correct get endpoint', async () => {
+		const getSchemaAppMock = jest.mocked(getSchemaAppEnsuringOrganization)
+
 		await expect(SchemaCommand.run([])).resolves.not.toThrow()
 
 		const getFunction = outputItemOrListMock.mock.calls[0][4]
 
 		const schemaApp = { endpointAppId: 'schemaAppId' } as SchemaApp
-		getSpy.mockResolvedValueOnce(schemaApp)
+		getSchemaAppMock.mockResolvedValueOnce({ schemaApp, organizationWasUpdated: false })
 
 		await expect(getFunction('schemaAppId')).resolves.toStrictEqual(schemaApp)
-		expect(getSpy).toHaveBeenCalledTimes(1)
-		expect(getSpy).toHaveBeenCalledWith('schemaAppId')
+		expect(getSchemaAppMock).toHaveBeenCalledTimes(1)
+		expect(getSchemaAppMock).toHaveBeenCalledWith(
+			expect.any(APICommand),
+			'schemaAppId',
+			{ profile: 'default' },
+		)
 	})
 
 	it('calls correct list endpoint', async () => {

packages/cli/src/__tests__/commands/schema/update.test.ts

@@ -4,25 +4,31 @@ import { inputItem, IOFormat, selectFromList } from '@smartthings/cli-lib'
 
 import SchemaUpdateCommand from '../../../commands/schema/update'
 import { addSchemaPermission } from '../../../lib/aws-utils'
-import { SchemaAppWithOrganization } from '../../../lib/commands/schema-util'
+import {
+	getSchemaAppEnsuringOrganization,
+	SchemaAppWithOrganization,
+} from '../../../lib/commands/schema-util'
 
 
 jest.mock('../../../lib/aws-utils')
+jest.mock('../../../lib/commands/schema-util')
 
 
 describe('SchemaUpdateCommand', () => {
 	const updateSpy = jest.spyOn(SchemaEndpoint.prototype, 'update').mockResolvedValue({ status: 'success' })
 	const listSpy = jest.spyOn(SchemaEndpoint.prototype, 'list')
 	const logSpy = jest.spyOn(SchemaUpdateCommand.prototype, 'log').mockImplementation()
 
-	const schemaAppRequest = { appName: 'schemaApp' } as SchemaAppWithOrganization
+	const schemaAppRequest = { appName: 'schemaApp' } as SchemaApp
 	const schemaAppRequestWithOrganization = {
 		...schemaAppRequest,
 		organizationId: 'organization-id',
 	} as SchemaAppWithOrganization
 	const inputItemMock = jest.mocked(inputItem).mockResolvedValue([schemaAppRequestWithOrganization, IOFormat.JSON])
 	const addSchemaPermissionMock = jest.mocked(addSchemaPermission)
 	const selectFromListMock = jest.mocked(selectFromList).mockResolvedValue('schemaAppId')
+	const getSchemaAppMock = jest.mocked(getSchemaAppEnsuringOrganization)
+		.mockResolvedValue({ schemaApp: schemaAppRequest, organizationWasUpdated: false })
 
 	it('prompts user to select schema app', async () => {
 		const schemaAppList = [{ appName: 'schemaApp' } as SchemaApp]
@@ -42,6 +48,12 @@ describe('SchemaUpdateCommand', () => {
 				listItems: expect.any(Function),
 			}),
 		)
+		expect(getSchemaAppMock).toHaveBeenCalledTimes(1)
+		expect(getSchemaAppMock).toHaveBeenCalledWith(
+			expect.any(SchemaUpdateCommand),
+			'schemaAppId',
+			{ profile: 'default' },
+		)
 
 		const listFunction = selectFromListMock.mock.calls[0][2].listItems
 

packages/cli/src/commands/invites/schema.ts

@@ -51,8 +51,11 @@ export default class InvitesSchemaCommand extends APICommand<typeof InvitesSchem
 		type InvitationProviderFunction = () => Promise<InvitationWithAppDetails[]>
 		const listFn = (client: SmartThingsClient, appId?: string): InvitationProviderFunction =>
 			async (): Promise<InvitationWithAppDetails[]> => {
+				// We have to be careful to not use the method to get a single app. For more
+				// details see `getSchemaAppEnsuringOrganization` in schema-utils.
 				const apps = appId
-					? [await client.schema.get(appId)]
+					? (await client.schema.list({ includeAllOrganizations: true }))
+						.filter(app => app.endpointAppId === appId)
 					: await client.schema.list()
 				return (await Promise.all(apps.map(async app => {
 					return app.endpointAppId

packages/cli/src/commands/invites/schema/create.ts

@@ -14,7 +14,7 @@ import {
 	userInputProcessor,
 } from '@smartthings/cli-lib'
 
-import { chooseSchemaApp } from '../../../lib/commands/schema-util'
+import { chooseSchemaApp, getSchemaAppEnsuringOrganization } from '../../../lib/commands/schema-util'
 import { getSingleInvite, InvitationWithAppDetails, tableFieldDefinitions } from '../../../lib/commands/invites-utils'
 
 
@@ -56,7 +56,7 @@ export default class InvitesSchemaCreateCommand extends APICommand<typeof Invite
 		const updateFromUserInput = async (): Promise<string | CancelAction> => {
 			const schemaAppId = await chooseSchemaApp(this, this.flags['schema-app'])
 			if (!schemaAppsById.has(schemaAppId)) {
-				const schemaApp = await this.client.schema.get(schemaAppId)
+				const { schemaApp } = await getSchemaAppEnsuringOrganization(this, schemaAppId, this.flags)
 				schemaAppsById.set(schemaAppId, schemaApp)
 			}
 			return schemaAppId
@@ -87,9 +87,9 @@ export default class InvitesSchemaCreateCommand extends APICommand<typeof Invite
 
 	async run(): Promise<void> {
 		const createInvitation = async (_: unknown, input: SchemaAppInvitationCreate): Promise<InvitationWithAppDetails> => {
-			// We don't need the full schema app but we need to call this to force some
-			// bookkeeping in the back end for older apps.
-			await this.client.schema.get(input.schemaAppId)
+			// We don't need the full schema app but using `getSchemaAppEnsuringOrganization`
+			// ensures there is a valid organization associated with the schema app.
+			await getSchemaAppEnsuringOrganization(this, input.schemaAppId, this.flags)
 			const idWrapper = await this.client.invitesSchema.create(input)
 			return getSingleInvite(this.client, input.schemaAppId, idWrapper.invitationId)
 		}

packages/cli/src/commands/schema.ts

@@ -8,6 +8,7 @@ import {
 	outputItemOrList,
 	OutputItemOrListConfig,
 } from '@smartthings/cli-lib'
+import { getSchemaAppEnsuringOrganization } from '../lib/commands/schema-util'
 
 
 export default class SchemaCommand extends APIOrganizationCommand<typeof SchemaCommand.flags> {
@@ -56,7 +57,7 @@ export default class SchemaCommand extends APIOrganizationCommand<typeof SchemaC
 
 		await outputItemOrList(this, config, this.args.id,
 			() => this.client.schema.list({ includeAllOrganizations: this.flags['all-organizations'] }),
-			id => this.client.schema.get(id),
+			async id => (await getSchemaAppEnsuringOrganization(this, id, this.flags)).schemaApp,
 		)
 	}
 }

packages/cli/src/commands/schema/update.ts

@@ -12,7 +12,11 @@ import {
 } from '@smartthings/cli-lib'
 
 import { addSchemaPermission } from '../../lib/aws-utils'
-import { getSchemaAppUpdateFromUser, SchemaAppWithOrganization } from '../../lib/commands/schema-util'
+import {
+	getSchemaAppEnsuringOrganization,
+	getSchemaAppUpdateFromUser,
+	SchemaAppWithOrganization,
+} from '../../lib/commands/schema-util'
 
 
 export default class SchemaUpdateCommand extends APIOrganizationCommand<typeof SchemaUpdateCommand.flags> {
@@ -49,11 +53,18 @@ export default class SchemaUpdateCommand extends APIOrganizationCommand<typeof S
 			listItems: () => this.client.schema.list(),
 		})
 
+		const { schemaApp: original, organizationWasUpdated } =
+			await getSchemaAppEnsuringOrganization(this, id, this.flags)
+		if (original.certificationStatus === 'wwst' || original.certificationStatus === 'cst') {
+			const cancelMsgBase =
+				'Schema apps that have already been certified cannot be updated via the CLI'
+			const cancelMsg = organizationWasUpdated
+				? cancelMsgBase + ' so further updates are not possible.'
+				: cancelMsgBase + '.'
+			this.cancel(cancelMsg)
+		}
+
 		const getInputFromUser = async (): Promise<SchemaAppRequest> => {
-			const original = await this.client.schema.get(id)
-			if (original.certificationStatus === 'wwst' || original.certificationStatus === 'cst') {
-				this.cancel('Schema apps that have already been certified cannot be updated via the CLI.')
-			}
 			return getSchemaAppUpdateFromUser(this, original, this.flags['dry-run'])
 		}
 

packages/cli/src/lib/commands/organization-util.ts

@@ -0,0 +1,29 @@
+import { OrganizationResponse } from '@smartthings/core-sdk'
+
+import {
+	APICommand,
+	ChooseOptions,
+	chooseOptionsWithDefaults,
+	selectFromList,
+	SelectFromListConfig,
+	stringTranslateToId,
+} from '@smartthings/cli-lib'
+
+
+export const chooseOrganization = async (
+		command: APICommand<typeof APICommand.flags>,
+		appFromArg?: string,
+		options?: Partial<ChooseOptions<OrganizationResponse>>,
+): Promise<string> => {
+	const opts = chooseOptionsWithDefaults(options)
+	const config: SelectFromListConfig<OrganizationResponse> = {
+		itemName: 'organization',
+		primaryKeyName: 'organizationId',
+		sortKeyName: 'name',
+	}
+	const listItems = (): Promise<OrganizationResponse[]> => command.client.organizations.list()
+	const preselectedId = opts.allowIndex
+		? await stringTranslateToId(config, appFromArg, listItems)
+		: appFromArg
+	return selectFromList(command, config, { preselectedId, listItems })
+}

packages/cli/src/lib/commands/schema-util.ts

@@ -1,4 +1,10 @@
-import { OrganizationResponse, SchemaApp, SchemaAppRequest, SmartThingsURLProvider, ViperAppLinks } from '@smartthings/core-sdk'
+import {
+	OrganizationResponse,
+	SchemaApp,
+	SchemaAppRequest,
+	SmartThingsURLProvider,
+	ViperAppLinks,
+} from '@smartthings/core-sdk'
 
 import {
 	APICommand,
@@ -19,12 +25,16 @@ import {
 	selectFromList,
 	SelectFromListConfig,
 	staticDef,
+	stdinIsTTY,
+	stdoutIsTTY,
 	stringDef,
 	stringTranslateToId,
 	undefinedDef,
 	updateFromUserInput,
 } from '@smartthings/cli-lib'
 import { awsHelpText } from '../aws-utils'
+import { chooseOrganization } from './organization-util'
+import { CLIError } from '@oclif/core/lib/errors'
 
 
 export const SCHEMA_AWS_PRINCIPAL = '148790070172'
@@ -189,3 +199,46 @@ export const chooseSchemaApp = async (command: APICommand<typeof APICommand.flag
 		: schemaAppFromArg
 	return selectFromList(command, config, { preselectedId, listItems, autoChoose: true })
 }
+
+// The endpoint to get a schema app automatically assigns the users org to an app if it
+// doesn't have one already. This causes a problem if the app is certified because the user
+// organization is almost certainly the wrong one and the user can't change it after it's been
+// set. So, here we check to see if the app has an organization before we query it and
+// prompt the user for the correct organization.
+export const getSchemaAppEnsuringOrganization = async (
+		command: APICommand<typeof APICommand.flags>,
+		schemaAppId: string,
+		flags: {
+			json: boolean
+			yaml: boolean
+			input?: string
+			output?: string
+		},
+): Promise<{ schemaApp: SchemaApp; organizationWasUpdated: boolean }> => {
+	const apps = await command.client.schema.list()
+	const appFromList = apps.find(app => app.endpointAppId === schemaAppId)
+	if (appFromList && !appFromList.organizationId) {
+		if (flags.json || flags.yaml || flags.output || flags.input || !stdinIsTTY()  || !stdoutIsTTY()) {
+			throw new CLIError(
+				'Schema app does not have an organization associated with it.\n' +
+					`Please run "smartthings schema ${schemaAppId}" and choose an organization when prompted.`,
+			)
+		}
+		// If we found an app but it didn't have an organization, ask the user to choose one.
+		// (If we didn't find an app at all, it's safe to use the single get because that means
+		// either it doesn't exist (bad app id) or it already has an organization.)
+		console.log(
+			`The schema "${appFromList.appName}" (${schemaAppId}) does not have an organization\n` +
+				'You must choose one now.',
+		)
+		const organizationId = await chooseOrganization(command)
+		const organization = await command.client.organizations.get(organizationId)
+		// eslint-disable-next-line @typescript-eslint/naming-convention
+		const orgClient = command.client.clone({ 'X-ST-Organization': organizationId })
+		const schemaApp = await orgClient.schema.get(schemaAppId)
+		console.log(`\nSchema app "${schemaApp.appName} (${schemaAppId}) is now associated with ` +
+			`organization ${organization.name} (${organizationId}).\n`)
+		return { schemaApp, organizationWasUpdated: true }
+	}
+	return { schemaApp: await command.client.schema.get(schemaAppId), organizationWasUpdated: false }
+}