From 250fc78da5a2093bcb5aac8c0b7341f1a943d0bf Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Thu, 3 Oct 2019 14:21:29 -0700 Subject: [PATCH 01/10] Add view_closed functionality and consolidate matchConstraints --- src/App.ts | 36 +++++++--- src/helpers.ts | 6 +- src/middleware/builtin.ts | 134 +++++++++++++++++++------------------- src/types/view/index.ts | 26 +++++++- 4 files changed, 121 insertions(+), 81 deletions(-) diff --git a/src/App.ts b/src/App.ts index 1669d6c9e..22d66c8e4 100644 --- a/src/App.ts +++ b/src/App.ts @@ -12,8 +12,7 @@ import { onlyEvents, matchEventType, matchMessage, - onlyViewSubmits, - matchCallbackId, + onlyViewActions, } from './middleware/builtin'; import { processMiddleware } from './middleware/process'; import { ConversationStore, conversationContext, MemoryStore } from './conversation-store'; @@ -89,6 +88,11 @@ export interface ActionConstraints { callback_id?: string | RegExp; } +export interface ViewConstraints { + callback_id?: string | RegExp; + type?: 'view_closed' | 'view_submission'; +} + export interface ErrorHandler { (error: CodedError): void; } @@ -304,9 +308,23 @@ export default class App { ); } - public view(callbackId: string | RegExp, ...listeners: Middleware[]): void { + public view( + callbackId: string | RegExp, + ...listeners: Middleware[] + ): void; + public view( + constraints: ViewConstraints, + ...listeners: Middleware[] + ): void; + public view( + callbackIdOrConstraints: string | RegExp | ViewConstraints, + ...listeners: Middleware[]): void { + const constraints: ViewConstraints = + (typeof callbackIdOrConstraints === 'string' || util.types.isRegExp(callbackIdOrConstraints)) ? + { callback_id: callbackIdOrConstraints, type: 'view_submission' } : callbackIdOrConstraints; + this.listeners.push( - [onlyViewSubmits, matchCallbackId(callbackId), ...listeners] as Middleware[], + [onlyViewActions, matchConstraints(constraints), ...listeners] as Middleware[], ); } @@ -370,7 +388,7 @@ export default class App { payload: (type === IncomingEventType.Event) ? (bodyArg as SlackEventMiddlewareArgs['body']).event : - (type === IncomingEventType.ViewSubmitAction) ? + (type === IncomingEventType.ViewAction) ? (bodyArg as SlackViewMiddlewareArgs['body']).view : (type === IncomingEventType.Action && isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])) ? @@ -398,7 +416,7 @@ export default class App { } else if (type === IncomingEventType.Options) { const optionListenerArgs = listenerArgs as SlackOptionsMiddlewareArgs; optionListenerArgs.options = optionListenerArgs.payload; - } else if (type === IncomingEventType.ViewSubmitAction) { + } else if (type === IncomingEventType.ViewAction) { const viewListenerArgs = listenerArgs as SlackViewMiddlewareArgs; viewListenerArgs.view = viewListenerArgs.payload; } @@ -477,11 +495,11 @@ function buildSource( const source: AuthorizeSourceData = { teamId: ((type === IncomingEventType.Event || type === IncomingEventType.Command) ? (body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string : - (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewSubmitAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).team.id as string : + (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).team.id as string : assertNever(type)), enterpriseId: ((type === IncomingEventType.Event || type === IncomingEventType.Command) ? (body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string : - (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewSubmitAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).team.enterprise_id as string : + (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).team.enterprise_id as string : undefined), userId: ((type === IncomingEventType.Event) ? @@ -490,7 +508,7 @@ function buildSource( ((body as SlackEventMiddlewareArgs['body']).event.channel !== undefined && (body as SlackEventMiddlewareArgs['body']).event.channel.creator !== undefined) ? (body as SlackEventMiddlewareArgs['body']).event.channel.creator as string : ((body as SlackEventMiddlewareArgs['body']).event.subteam !== undefined && (body as SlackEventMiddlewareArgs['body']).event.subteam.created_by !== undefined) ? (body as SlackEventMiddlewareArgs['body']).event.subteam.created_by as string : undefined) : - (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewSubmitAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).user.id as string : + (type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewAction) ? (body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).user.id as string : (type === IncomingEventType.Command) ? (body as SlackCommandMiddlewareArgs['body']).user_id as string : undefined), conversationId: channelId, diff --git a/src/helpers.ts b/src/helpers.ts index f427732f6..a6415d891 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -15,7 +15,7 @@ export enum IncomingEventType { Action, Command, Options, - ViewSubmitAction, + ViewAction, } /** @@ -54,9 +54,9 @@ export function getTypeAndConversation(body: any): { type?: IncomingEventType, c conversationId: actionBody.channel !== undefined ? actionBody.channel.id : undefined, }; } - if (body.type === 'view_submission') { + if (body.type === 'view_submission' || body.type === 'view_closed') { return { - type: IncomingEventType.ViewSubmitAction, + type: IncomingEventType.ViewAction, }; } return {}; diff --git a/src/middleware/builtin.ts b/src/middleware/builtin.ts index 28458bda2..1961c4234 100644 --- a/src/middleware/builtin.ts +++ b/src/middleware/builtin.ts @@ -16,9 +16,9 @@ import { BlockElementAction, ContextMissingPropertyError, SlackViewMiddlewareArgs, - ViewOutput, + ViewClosedAction, } from '../types'; -import { ActionConstraints } from '../App'; +import { ActionConstraints, ViewConstraints } from '../App'; import { ErrorCode, errorWithCode } from '../errors'; /** @@ -74,107 +74,92 @@ export const onlyEvents: Middleware }; /** - * Middleware that filters out any event that isn't a view_submission + * Middleware that filters out any event that isn't a view_submission or view_closed event */ -export const onlyViewSubmits: Middleware = ({ view, next }) => { - // Filter out anything that isn't a view_submission - if (view === undefined) { - return; - } - - // It matches so we should continue down this middleware listener chain - next(); -}; - -// TODO: is there a way to consolidate the next two methods without too much type refactoring? -export function matchCallbackId( - callbackId: string | RegExp, -): Middleware { - return ({ payload, next, context }) => { - let tempMatches: RegExpMatchArray | null; - - if (!isCallbackIdentifiedBody(payload)) { +export const onlyViewActions: Middleware = ({ view, next }) => { + // Filter out anything that doesn't have a view + if (view === undefined) { return; } - if (typeof callbackId === 'string') { - if (payload.callback_id !== callbackId) { - return; - } - } else { - tempMatches = payload.callback_id.match(callbackId); - - if (tempMatches !== null) { - context['callbackIdMatches'] = tempMatches; - } else { - return; - } - } + // It matches so we should continue down this middleware listener chain next(); }; -} /** * Middleware that checks for matches given constraints */ export function matchConstraints( - constraints: ActionConstraints, - ): Middleware { + constraints: ActionConstraints | ViewConstraints, + ): Middleware { return ({ payload, body, next, context }) => { // TODO: is putting matches in an array actually helpful? there's no way to know which of the regexps contributed // which matches (and in which order) let tempMatches: RegExpMatchArray | null; - if (constraints.block_id !== undefined) { + // Narrow type for ActionConstraints + if ('block_id' in constraints || 'action_id' in constraints) { if (!isBlockPayload(payload)) { return; } - if (typeof constraints.block_id === 'string') { - if (payload.block_id !== constraints.block_id) { - return; + // Check block_id + if (constraints.block_id !== undefined) { + + if (typeof constraints.block_id === 'string') { + if (payload.block_id !== constraints.block_id) { + return; + } + } else { + tempMatches = payload.block_id.match(constraints.block_id); + + if (tempMatches !== null) { + context['blockIdMatches'] = tempMatches; + } else { + return; + } } - } else { - tempMatches = payload.block_id.match(constraints.block_id); + } - if (tempMatches !== null) { - context['blockIdMatches'] = tempMatches; + // Check action_id + if (constraints.action_id !== undefined) { + if (typeof constraints.action_id === 'string') { + if (payload.action_id !== constraints.action_id) { + return; + } } else { - return; + tempMatches = payload.action_id.match(constraints.action_id); + + if (tempMatches !== null) { + context['actionIdMatches'] = tempMatches; + } else { + return; + } } } } - if (constraints.action_id !== undefined) { - if (!isBlockPayload(payload)) { - return; - } + // Check callback_id + if ('callback_id' in constraints && constraints.callback_id !== undefined) { + let callbackId: string = ''; - if (typeof constraints.action_id === 'string') { - if (payload.action_id !== constraints.action_id) { - return; - } + if (isViewBody(body)) { + callbackId = body['view']['callback_id']; } else { - tempMatches = payload.action_id.match(constraints.action_id); - - if (tempMatches !== null) { - context['actionIdMatches'] = tempMatches; + if (isCallbackIdentifiedBody(body)) { + callbackId = body['callback_id']; } else { return; } } - } - if (constraints.callback_id !== undefined) { - if (!isCallbackIdentifiedBody(body)) { - return; - } if (typeof constraints.callback_id === 'string') { - if (body.callback_id !== constraints.callback_id) { + if (callbackId !== constraints.callback_id) { return; } } else { - tempMatches = body.callback_id.match(constraints.callback_id); + tempMatches = callbackId.match(constraints.callback_id); if (tempMatches !== null) { context['callbackIdMatches'] = tempMatches; @@ -184,6 +169,11 @@ export function matchConstraints( } } + // Check type + if ('type' in constraints) { + if (body.type !== constraints.type) return; + } + next(); }; } @@ -328,7 +318,10 @@ export function directMention(): Middleware> } function isBlockPayload( - payload: SlackActionMiddlewareArgs['payload'] | SlackOptionsMiddlewareArgs['payload'], + payload: + | SlackActionMiddlewareArgs['payload'] + | SlackOptionsMiddlewareArgs['payload'] + | SlackViewMiddlewareArgs['payload'], ): payload is BlockElementAction | OptionsRequest<'block_suggestion'> { return (payload as BlockElementAction | OptionsRequest<'block_suggestion'>).action_id !== undefined; } @@ -336,16 +329,21 @@ function isBlockPayload( type CallbackIdentifiedBody = | InteractiveMessage | DialogSubmitAction - | ViewOutput | MessageAction | OptionsRequest<'interactive_message' | 'dialog_suggestion'>; function isCallbackIdentifiedBody( - body: SlackActionMiddlewareArgs['body'] | SlackOptionsMiddlewareArgs['body'] | SlackViewMiddlewareArgs['payload'], + body: SlackActionMiddlewareArgs['body'] | SlackOptionsMiddlewareArgs['body'], ): body is CallbackIdentifiedBody { return (body as CallbackIdentifiedBody).callback_id !== undefined; } +function isViewBody( + body: SlackActionMiddlewareArgs['body'] | SlackOptionsMiddlewareArgs['body'] | SlackViewMiddlewareArgs['body'], +): body is (ViewSubmitAction | ViewClosedAction) { + return (body as ViewSubmitAction).view !== undefined; +} + function isEventArgs( args: AnyMiddlewareArgs, ): args is SlackEventMiddlewareArgs { diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 901b3e55a..1d04bb4d6 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -7,7 +7,7 @@ import { RespondArguments, AckFn } from '../utilities'; export interface SlackViewMiddlewareArgs { payload: ViewOutput; view: this['payload']; - body: ViewSubmitAction; + body: ViewSubmitAction | ViewClosedAction; ack: AckFn; } @@ -40,6 +40,30 @@ export interface ViewSubmitAction { token: string; } +/** + * A Slack view_closed event wrapped in the standard metadata. + * + * This describes the entire JSON-encoded body of a view_closed event. + */ +export interface ViewClosedAction { + type: 'view_closed'; + team: { + id: string; + domain: string; + enterprise_id?: string; // undocumented + enterprise_name?: string; // undocumented + }; + user: { + id: string; + name: string; + team_id?: string; // undocumented + }; + view: ViewOutput; + api_app_id: string; + token: string; + is_cleared: boolean; +} + export interface ViewOutput { id: string; callback_id: string; From 36f970d299f03df247446e87af3a2062495a86ec Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Thu, 3 Oct 2019 14:34:37 -0700 Subject: [PATCH 02/10] Add getTypeAndConversation() tests for views --- src/helpers.spec.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/helpers.spec.ts b/src/helpers.spec.ts index eb0038f18..ad27217ac 100644 --- a/src/helpers.spec.ts +++ b/src/helpers.spec.ts @@ -77,6 +77,21 @@ describe('getTypeAndConversation()', () => { }); }); + describe('view types', () => { + // Arrange + const dummyViewBodies = createFakeViews(); + + dummyViewBodies.forEach((viewBody) => { + it(`should find Action type for ${viewBody.type}`, () => { + // Act + const typeAndConversation = getTypeAndConversation(viewBody); + + // Assert + assert(typeAndConversation.type === IncomingEventType.ViewAction); + }); + }); + }); + describe('invalid events', () => { // Arrange const fakeEventBody = { @@ -150,3 +165,18 @@ function createFakeOptions(conversationId: string): any[] { }, ]; } + +function createFakeViews(): any[] { + return [ + // Body for a view_submission event + { + type: 'view_submission', + view: { id: 'V123' }, + }, + // Body for a view_closed event + { + type: 'view_closed', + view: { id: 'V456' }, + }, + ]; +} From 6b3564d932ed5c3cd0a5df0c7b0d26730e929fb7 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Fri, 4 Oct 2019 10:34:47 -0700 Subject: [PATCH 03/10] Add unknownConstraints check --- src/App.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/App.ts b/src/App.ts index 22d66c8e4..fea58c9a9 100644 --- a/src/App.ts +++ b/src/App.ts @@ -323,6 +323,16 @@ export default class App { (typeof callbackIdOrConstraints === 'string' || util.types.isRegExp(callbackIdOrConstraints)) ? { callback_id: callbackIdOrConstraints, type: 'view_submission' } : callbackIdOrConstraints; + // Fail early if the constraints contain invalid keys + const unknownConstraintKeys = Object.keys(constraints) + .filter(k => (k !== 'callback_id' && k !== 'type')); + if (unknownConstraintKeys.length > 0) { + this.logger.error( + `View listener cannot be attached using unknown constraint keys: ${unknownConstraintKeys.join(', ')}`, + ); + return; + } + this.listeners.push( [onlyViewActions, matchConstraints(constraints), ...listeners] as Middleware[], ); From 51e68114cbc830d0c658a888b7a3c695a4e60fc5 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Fri, 4 Oct 2019 10:36:29 -0700 Subject: [PATCH 04/10] add kaz test --- src/App.spec.ts | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index af4a203ef..cda101d2c 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -437,20 +437,19 @@ describe('App', () => { respond: noop, ack: noop, }, - // TODO: https://github.com/slackapi/bolt/issues/263 - // { - // body: { - // type: 'view_closed', - // channel: {}, - // user: {}, - // team: {}, - // view: { - // callback_id: 'view_callback_id', - // } - // }, - // respond: noop, - // ack: noop, - // }, + { + body: { + type: 'view_closed', + channel: {}, + user: {}, + team: {}, + view: { + callback_id: 'view_callback_id', + } + }, + respond: noop, + ack: noop, + }, ]; } From ab6909fc41e86507bfe26373304c51c352fe0aca Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 09:54:22 -0700 Subject: [PATCH 05/10] Kaz test --- src/App.spec.ts | 17 +++++++++-------- src/App.ts | 2 +- src/ExpressReceiver.ts | 4 ++-- src/middleware/builtin.ts | 4 ++-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index cda101d2c..cb294cc13 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -432,7 +432,7 @@ describe('App', () => { team: {}, view: { callback_id: 'view_callback_id', - } + }, }, respond: noop, ack: noop, @@ -445,7 +445,7 @@ describe('App', () => { team: {}, view: { callback_id: 'view_callback_id', - } + }, }, respond: noop, ack: noop, @@ -466,11 +466,12 @@ describe('App', () => { // Act const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) }); app.use((_args) => { ackFn(); }); - app.action('block_action_id', ({ }) => { actionFn(); }) - app.action({ callback_id: 'message_action_callback_id' }, ({ }) => { actionFn(); }) - app.action({ callback_id: 'interactive_message_callback_id' }, ({ }) => { actionFn(); }) - app.action({ callback_id: 'dialog_submission_callback_id' }, ({ }) => { actionFn(); }) - app.view('view_callback_id', ({ }) => { viewFn(); }) + app.action('block_action_id', ({ }) => { actionFn(); }); + app.action({ callback_id: 'message_action_callback_id' }, ({ }) => { actionFn(); }); + app.action({ callback_id: 'interactive_message_callback_id' }, ({ }) => { actionFn(); }); + app.action({ callback_id: 'dialog_submission_callback_id' }, ({ }) => { actionFn(); }); + app.view('view_callback_id', ({ }) => { viewFn(); }); + app.view({ callback_id: 'view_callback_id', type: 'view_closed' }, ({ }) => { viewFn(); }); app.options('external_select_action_id', ({ }) => { optionsFn(); }); app.options({ callback_id: 'dialog_suggestion_callback_id' }, ({ }) => { optionsFn(); }); @@ -480,7 +481,7 @@ describe('App', () => { // Assert assert.equal(actionFn.callCount, 4); - assert.equal(viewFn.callCount, 1); + assert.equal(viewFn.callCount, 2); assert.equal(optionsFn.callCount, 2); assert.equal(ackFn.callCount, dummyReceiverEvents.length); assert(fakeErrorHandler.notCalled); diff --git a/src/App.ts b/src/App.ts index ef6dd713b..d82b40741 100644 --- a/src/App.ts +++ b/src/App.ts @@ -330,7 +330,7 @@ export default class App { const constraints: ViewConstraints = (typeof callbackIdOrConstraints === 'string' || util.types.isRegExp(callbackIdOrConstraints)) ? { callback_id: callbackIdOrConstraints, type: 'view_submission' } : callbackIdOrConstraints; - + const test = constraints['type']; // Fail early if the constraints contain invalid keys const unknownConstraintKeys = Object.keys(constraints) .filter(k => (k !== 'callback_id' && k !== 'type')); diff --git a/src/ExpressReceiver.ts b/src/ExpressReceiver.ts index 4032f8ce7..8609dd7dd 100644 --- a/src/ExpressReceiver.ts +++ b/src/ExpressReceiver.ts @@ -33,7 +33,7 @@ export default class ExpressReceiver extends EventEmitter implements Receiver { constructor({ signingSecret = '', logger = new ConsoleLogger(), - endpoints = { events: '/slack/events' } + endpoints = { events: '/slack/events' }, }: ExpressReceiverOptions) { super(); @@ -256,7 +256,7 @@ function parseRequestBody( // Parse this body anyway return JSON.parse(stringBody); } catch (e) { - logger.error(`Failed to parse body as JSON data for content-type: ${contentType}`) + logger.error(`Failed to parse body as JSON data for content-type: ${contentType}`); throw e; } } diff --git a/src/middleware/builtin.ts b/src/middleware/builtin.ts index 1d150e7ab..8fdc99597 100644 --- a/src/middleware/builtin.ts +++ b/src/middleware/builtin.ts @@ -5,18 +5,18 @@ import { SlackCommandMiddlewareArgs, SlackEventMiddlewareArgs, SlackOptionsMiddlewareArgs, + SlackViewMiddlewareArgs, SlackEvent, SlackAction, SlashCommand, ViewSubmitAction, + ViewClosedAction, OptionsRequest, InteractiveMessage, DialogSubmitAction, MessageAction, BlockElementAction, ContextMissingPropertyError, - SlackViewMiddlewareArgs, - ViewClosedAction, } from '../types'; import { ActionConstraints, ViewConstraints } from '../App'; import { ErrorCode, errorWithCode } from '../errors'; From c89ddbb3198fe90ef27e48f427f32f6b29cb9978 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 10:19:56 -0700 Subject: [PATCH 06/10] Add unknown view event type check --- src/App.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/App.ts b/src/App.ts index d82b40741..96fcb9f16 100644 --- a/src/App.ts +++ b/src/App.ts @@ -330,10 +330,9 @@ export default class App { const constraints: ViewConstraints = (typeof callbackIdOrConstraints === 'string' || util.types.isRegExp(callbackIdOrConstraints)) ? { callback_id: callbackIdOrConstraints, type: 'view_submission' } : callbackIdOrConstraints; - const test = constraints['type']; - // Fail early if the constraints contain invalid keys + // Fail early if the constraints contain invalid keys const unknownConstraintKeys = Object.keys(constraints) - .filter(k => (k !== 'callback_id' && k !== 'type')); + .filter(k => ( k !== 'callback_id' && k !== 'type')); if (unknownConstraintKeys.length > 0) { this.logger.error( `View listener cannot be attached using unknown constraint keys: ${unknownConstraintKeys.join(', ')}`, @@ -341,6 +340,13 @@ export default class App { return; } + if (constraints.type !== undefined && validViewTypes.includes(constraints.type)) { + this.logger.error( + `View listener cannot be attached using unknown view event type ${constraints.type}`, + ); + return; + } + this.listeners.push( [onlyViewActions, matchConstraints(constraints), ...listeners] as Middleware[], ); @@ -498,6 +504,8 @@ export default class App { const tokenUsage = 'Apps used in one workspace should be initialized with a token. Apps used in many workspaces ' + 'should be initialized with a authorize.'; +const validViewTypes = ['view_submission', 'view_closed']; + /** * Helper which builds the data structure the authorize hook uses to provide tokens for the context. */ From 43f0ce0a55f0e92b317ed1c75060dd8663543cc4 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 10:56:50 -0700 Subject: [PATCH 07/10] view_closed cleanup --- src/App.ts | 23 +++++++++++++---------- src/middleware/builtin.ts | 10 +++++++--- src/types/view/index.ts | 9 +++++++-- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/App.ts b/src/App.ts index 96fcb9f16..02d79eb2c 100644 --- a/src/App.ts +++ b/src/App.ts @@ -34,6 +34,8 @@ import { InteractiveMessage, Receiver, ReceiverEvent, + SlackViewAction, + ViewSubmitAction, } from './types'; import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers'; import { ErrorCode, CodedError, errorWithCode, asCodedError } from './errors'; @@ -316,23 +318,23 @@ export default class App { ); } - public view( + public view( callbackId: string | RegExp, - ...listeners: Middleware[] + ...listeners: Middleware>[] ): void; - public view( + public view( constraints: ViewConstraints, - ...listeners: Middleware[] + ...listeners: Middleware>[] ): void; - public view( + public view( callbackIdOrConstraints: string | RegExp | ViewConstraints, - ...listeners: Middleware[]): void { + ...listeners: Middleware>[]): void { const constraints: ViewConstraints = (typeof callbackIdOrConstraints === 'string' || util.types.isRegExp(callbackIdOrConstraints)) ? { callback_id: callbackIdOrConstraints, type: 'view_submission' } : callbackIdOrConstraints; // Fail early if the constraints contain invalid keys const unknownConstraintKeys = Object.keys(constraints) - .filter(k => ( k !== 'callback_id' && k !== 'type')); + .filter(k => (k !== 'callback_id' && k !== 'type')); if (unknownConstraintKeys.length > 0) { this.logger.error( `View listener cannot be attached using unknown constraint keys: ${unknownConstraintKeys.join(', ')}`, @@ -340,7 +342,7 @@ export default class App { return; } - if (constraints.type !== undefined && validViewTypes.includes(constraints.type)) { + if (constraints.type !== undefined && !validViewTypes.includes(constraints.type)) { this.logger.error( `View listener cannot be attached using unknown view event type ${constraints.type}`, ); @@ -365,6 +367,7 @@ export default class App { // with "finally" type error situations. // Introspect the body to determine what type of incoming event is being handled, and any channel context const { type, conversationId } = getTypeAndConversation(body); + console.log(type); // If the type could not be determined, warn and exit if (type === undefined) { this.logger.warn('Could not determine the type of an incoming event. No listeners will be called.'); @@ -413,7 +416,7 @@ export default class App { (type === IncomingEventType.Event) ? (bodyArg as SlackEventMiddlewareArgs['body']).event : (type === IncomingEventType.ViewAction) ? - (bodyArg as SlackViewMiddlewareArgs['body']).view : + (bodyArg as SlackViewMiddlewareArgs['body']).view : (type === IncomingEventType.Action && isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])) ? (bodyArg as SlackActionMiddlewareArgs['body']).actions[0] : @@ -504,7 +507,7 @@ export default class App { const tokenUsage = 'Apps used in one workspace should be initialized with a token. Apps used in many workspaces ' + 'should be initialized with a authorize.'; -const validViewTypes = ['view_submission', 'view_closed']; +const validViewTypes = ['view_closed', 'view_submission']; /** * Helper which builds the data structure the authorize hook uses to provide tokens for the context. diff --git a/src/middleware/builtin.ts b/src/middleware/builtin.ts index 8fdc99597..66f518484 100644 --- a/src/middleware/builtin.ts +++ b/src/middleware/builtin.ts @@ -17,6 +17,7 @@ import { MessageAction, BlockElementAction, ContextMissingPropertyError, + SlackViewAction, } from '../types'; import { ActionConstraints, ViewConstraints } from '../App'; import { ErrorCode, errorWithCode } from '../errors'; @@ -346,9 +347,12 @@ function isCallbackIdentifiedBody( } function isViewBody( - body: SlackActionMiddlewareArgs['body'] | SlackOptionsMiddlewareArgs['body'] | SlackViewMiddlewareArgs['body'], -): body is (ViewSubmitAction | ViewClosedAction) { - return (body as ViewSubmitAction).view !== undefined; + body: + SlackActionMiddlewareArgs['body'] + | SlackOptionsMiddlewareArgs['body'] + | SlackViewMiddlewareArgs['body'], +): body is SlackViewAction { + return (body as SlackViewAction).view !== undefined; } function isEventArgs( diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 1d04bb4d6..503cf6d4c 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -1,13 +1,18 @@ import { StringIndexed } from '../helpers'; import { RespondArguments, AckFn } from '../utilities'; +/** + * Known view action types + */ +export type SlackViewAction = ViewSubmitAction | ViewClosedAction; +// /** * Arguments which listeners and middleware receive to process a view submission event from Slack. */ -export interface SlackViewMiddlewareArgs { +export interface SlackViewMiddlewareArgs { payload: ViewOutput; view: this['payload']; - body: ViewSubmitAction | ViewClosedAction; + body: ViewActionType; ack: AckFn; } From 479cf3ab4f6bc972a43a5a7e34aaf7ce78f72aed Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 10:58:04 -0700 Subject: [PATCH 08/10] clarity in error message --- src/App.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App.ts b/src/App.ts index 02d79eb2c..85b0cdaea 100644 --- a/src/App.ts +++ b/src/App.ts @@ -344,7 +344,7 @@ export default class App { if (constraints.type !== undefined && !validViewTypes.includes(constraints.type)) { this.logger.error( - `View listener cannot be attached using unknown view event type ${constraints.type}`, + `View listener cannot be attached using unknown view event type: ${constraints.type}`, ); return; } From b99bb8dbb8ca37793017170417192a7198e9d559 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 11:05:25 -0700 Subject: [PATCH 09/10] Cleanup doo dah --- src/App.ts | 11 +++++------ src/middleware/builtin.ts | 2 +- src/types/view/index.ts | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/App.ts b/src/App.ts index 85b0cdaea..2150cf10d 100644 --- a/src/App.ts +++ b/src/App.ts @@ -32,10 +32,9 @@ import { OptionsSource, BlockAction, InteractiveMessage, + SlackViewAction, Receiver, ReceiverEvent, - SlackViewAction, - ViewSubmitAction, } from './types'; import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers'; import { ErrorCode, CodedError, errorWithCode, asCodedError } from './errors'; @@ -318,15 +317,15 @@ export default class App { ); } - public view( + public view( callbackId: string | RegExp, ...listeners: Middleware>[] ): void; - public view( + public view( constraints: ViewConstraints, ...listeners: Middleware>[] ): void; - public view( + public view( callbackIdOrConstraints: string | RegExp | ViewConstraints, ...listeners: Middleware>[]): void { const constraints: ViewConstraints = @@ -416,7 +415,7 @@ export default class App { (type === IncomingEventType.Event) ? (bodyArg as SlackEventMiddlewareArgs['body']).event : (type === IncomingEventType.ViewAction) ? - (bodyArg as SlackViewMiddlewareArgs['body']).view : + (bodyArg as SlackViewMiddlewareArgs['body']).view : (type === IncomingEventType.Action && isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])) ? (bodyArg as SlackActionMiddlewareArgs['body']).actions[0] : diff --git a/src/middleware/builtin.ts b/src/middleware/builtin.ts index 66f518484..8054191b6 100644 --- a/src/middleware/builtin.ts +++ b/src/middleware/builtin.ts @@ -350,7 +350,7 @@ function isViewBody( body: SlackActionMiddlewareArgs['body'] | SlackOptionsMiddlewareArgs['body'] - | SlackViewMiddlewareArgs['body'], + | SlackViewMiddlewareArgs['body'], ): body is SlackViewAction { return (body as SlackViewAction).view !== undefined; } diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 503cf6d4c..411df4d51 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -9,7 +9,7 @@ export type SlackViewAction = ViewSubmitAction | ViewClosedAction; /** * Arguments which listeners and middleware receive to process a view submission event from Slack. */ -export interface SlackViewMiddlewareArgs { +export interface SlackViewMiddlewareArgs { payload: ViewOutput; view: this['payload']; body: ViewActionType; From a7cd1d746b480adae0d77e920ba5f1ef518244a8 Mon Sep 17 00:00:00 2001 From: Shane DeWael Date: Mon, 7 Oct 2019 11:06:59 -0700 Subject: [PATCH 10/10] a wild console.log was stranded. i successfully rescued it and brought it home where it belongs --- src/App.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/App.ts b/src/App.ts index 2150cf10d..90fafe27c 100644 --- a/src/App.ts +++ b/src/App.ts @@ -366,7 +366,6 @@ export default class App { // with "finally" type error situations. // Introspect the body to determine what type of incoming event is being handled, and any channel context const { type, conversationId } = getTypeAndConversation(body); - console.log(type); // If the type could not be determined, warn and exit if (type === undefined) { this.logger.warn('Could not determine the type of an incoming event. No listeners will be called.');