Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote Functions: Change how webclient options are constructed in CustomFunction #2190

Merged
merged 3 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output",
"lint": "eslint --fix --ext .ts src",
"mocha": "TS_NODE_PROJECT=tsconfig.json nyc mocha --config .mocharc.json \"src/**/*.spec.ts\"",
"test": "npm run lint && npm run mocha && npm run test:types",
"test": "npm run build && npm run lint && npm run mocha && npm run test:types",
"test:coverage": "npm run mocha && nyc report --reporter=text",
"test:types": "tsd",
"watch": "npx nodemon --watch 'src' --ext 'ts' --exec npm run build"
},
Expand Down
6 changes: 5 additions & 1 deletion src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,10 @@
}
}

public get webClientOptions(): WebClientOptions {
return this.clientOptions;

Check warning on line 507 in src/App.ts

View check run for this annotation

Codecov / codecov/patch

src/App.ts#L507

Added line #L507 was not covered by tests
}

/**
* Register a new middleware, processed in the order registered.
*
Expand Down Expand Up @@ -530,7 +534,7 @@
* Register CustomFunction middleware
*/
public function(callbackId: string, ...listeners: CustomFunctionMiddleware): this {
const fn = new CustomFunction(callbackId, listeners);
const fn = new CustomFunction(callbackId, listeners, this.webClientOptions);

Check warning on line 537 in src/App.ts

View check run for this annotation

Codecov / codecov/patch

src/App.ts#L537

Added line #L537 was not covered by tests
const m = fn.getMiddleware();
this.middleware.push(m);
return this;
Expand Down
141 changes: 102 additions & 39 deletions src/CustomFunction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ import 'mocha';
import { assert } from 'chai';
import sinon from 'sinon';
import rewiremock from 'rewiremock';
import { WebClient } from '@slack/web-api';
import {
CustomFunction,
SlackCustomFunctionMiddlewareArgs,
AllCustomFunctionMiddlewareArgs,
CustomFunctionMiddleware,
CustomFunctionExecuteMiddlewareArgs,
} from './CustomFunction';
import { Override } from './test-helpers';
import { AllMiddlewareArgs, AnyMiddlewareArgs, Middleware } from './types';
import { createFakeLogger, Override } from './test-helpers';
import { AllMiddlewareArgs, Middleware } from './types';
import { CustomFunctionInitializationError } from './errors';

async function importCustomFunction(overrides: Override = {}): Promise<typeof import('./CustomFunction')> {
Expand All @@ -26,36 +27,35 @@ const MOCK_MIDDLEWARE_MULTIPLE = [MOCK_FN, MOCK_FN_2];
describe('CustomFunction class', () => {
describe('constructor', () => {
it('should accept single function as middleware', async () => {
const fn = new CustomFunction('test_callback_id', MOCK_MIDDLEWARE_SINGLE);
const fn = new CustomFunction('test_callback_id', MOCK_MIDDLEWARE_SINGLE, {});
assert.isNotNull(fn);
});

it('should accept multiple functions as middleware', async () => {
const fn = new CustomFunction('test_callback_id', MOCK_MIDDLEWARE_MULTIPLE);
const fn = new CustomFunction('test_callback_id', MOCK_MIDDLEWARE_MULTIPLE, {});
assert.isNotNull(fn);
});
});

describe('getMiddleware', () => {
it('should not call next if a function_executed event', async () => {
const fn = new CustomFunction('test_executed_callback_id', MOCK_MIDDLEWARE_SINGLE);
const cbId = 'test_executed_callback_id';
const fn = new CustomFunction(cbId, MOCK_MIDDLEWARE_SINGLE, {});
const middleware = fn.getMiddleware();
const fakeEditArgs = createFakeFunctionExecutedEvent() as unknown as
SlackCustomFunctionMiddlewareArgs & AllMiddlewareArgs;
const fakeEditArgs = createFakeFunctionExecutedEvent(cbId);

const fakeNext = sinon.spy();
fakeEditArgs.next = fakeNext;

await middleware(fakeEditArgs);

assert(fakeNext.notCalled);
assert(fakeNext.notCalled, 'next called!');
});

it('should call next if valid custom function but mismatched callback_id', async () => {
const fn = new CustomFunction('bad_executed_callback_id', MOCK_MIDDLEWARE_SINGLE);
const fn = new CustomFunction('bad_executed_callback_id', MOCK_MIDDLEWARE_SINGLE, {});
const middleware = fn.getMiddleware();
const fakeEditArgs = createFakeFunctionExecutedEvent() as unknown as
SlackCustomFunctionMiddlewareArgs & AllMiddlewareArgs;
const fakeEditArgs = createFakeFunctionExecutedEvent();

const fakeNext = sinon.spy();
fakeEditArgs.next = fakeNext;
Expand All @@ -66,7 +66,7 @@ describe('CustomFunction class', () => {
});

it('should call next if not a function executed event', async () => {
const fn = new CustomFunction('test_view_callback_id', MOCK_MIDDLEWARE_SINGLE);
const fn = new CustomFunction('test_view_callback_id', MOCK_MIDDLEWARE_SINGLE, {});
const middleware = fn.getMiddleware();
const fakeViewArgs = createFakeViewEvent() as unknown as
SlackCustomFunctionMiddlewareArgs & AllMiddlewareArgs;
Expand Down Expand Up @@ -120,8 +120,7 @@ describe('CustomFunction class', () => {

describe('isFunctionEvent', () => {
it('should return true if recognized function_executed payload type', async () => {
const fakeExecuteArgs = createFakeFunctionExecutedEvent() as unknown as SlackCustomFunctionMiddlewareArgs
& AllMiddlewareArgs;
const fakeExecuteArgs = createFakeFunctionExecutedEvent();

const { isFunctionEvent } = await importCustomFunction();
const eventIsFunctionExcuted = isFunctionEvent(fakeExecuteArgs);
Expand All @@ -130,7 +129,8 @@ describe('CustomFunction class', () => {
});

it('should return false if not a function_executed payload type', async () => {
const fakeExecutedEvent = createFakeFunctionExecutedEvent() as unknown as AnyMiddlewareArgs;
const fakeExecutedEvent = createFakeFunctionExecutedEvent();
// @ts-expect-error expected invalid payload type
fakeExecutedEvent.payload.type = 'invalid_type';

const { isFunctionEvent } = await importCustomFunction();
Expand All @@ -142,10 +142,10 @@ describe('CustomFunction class', () => {

describe('enrichFunctionArgs', () => {
it('should remove next() from all original event args', async () => {
const fakeExecutedEvent = createFakeFunctionExecutedEvent() as unknown as AnyMiddlewareArgs;
const fakeExecutedEvent = createFakeFunctionExecutedEvent();

const { enrichFunctionArgs } = await importCustomFunction();
const executeFunctionArgs = enrichFunctionArgs(fakeExecutedEvent);
const executeFunctionArgs = enrichFunctionArgs(fakeExecutedEvent, {});

assert.notExists(executeFunctionArgs.next);
});
Expand All @@ -154,7 +154,7 @@ describe('CustomFunction class', () => {
const fakeArgs = createFakeFunctionExecutedEvent();

const { enrichFunctionArgs } = await importCustomFunction();
const functionArgs = enrichFunctionArgs(fakeArgs);
const functionArgs = enrichFunctionArgs(fakeArgs, {});

assert.exists(functionArgs.inputs);
assert.exists(functionArgs.complete);
Expand All @@ -163,60 +163,123 @@ describe('CustomFunction class', () => {
});

describe('custom function utility functions', () => {
it('complete should call functions.completeSuccess', async () => {
// TODO
describe('`complete` factory function', () => {
it('complete should call functions.completeSuccess', async () => {
const client = new WebClient('sometoken');
const completeMock = sinon.stub(client.functions, 'completeSuccess').resolves();
const complete = CustomFunction.createFunctionComplete({ isEnterpriseInstall: false, functionExecutionId: 'Fx1234' }, client);
await complete();
assert(completeMock.called, 'client.functions.completeSuccess not called!');
});
it('should throw if no functionExecutionId present on context', () => {
const client = new WebClient('sometoken');
assert.throws(() => {
CustomFunction.createFunctionComplete({ isEnterpriseInstall: false }, client);
});
});
});

it('fail should call functions.completeError', async () => {
// TODO
describe('`fail` factory function', () => {
it('fail should call functions.completeError', async () => {
const client = new WebClient('sometoken');
const completeMock = sinon.stub(client.functions, 'completeError').resolves();
const complete = CustomFunction.createFunctionFail({ isEnterpriseInstall: false, functionExecutionId: 'Fx1234' }, client);
await complete({ error: 'boom' });
assert(completeMock.called, 'client.functions.completeError not called!');
});
it('should throw if no functionExecutionId present on context', () => {
const client = new WebClient('sometoken');
assert.throws(() => {
CustomFunction.createFunctionFail({ isEnterpriseInstall: false }, client);
});
});
});

it('inputs should map to function payload inputs', async () => {
const fakeExecuteArgs = createFakeFunctionExecutedEvent() as unknown as AllCustomFunctionMiddlewareArgs;
const fakeExecuteArgs = createFakeFunctionExecutedEvent();

const { enrichFunctionArgs } = await importCustomFunction();
const enrichedArgs = enrichFunctionArgs(fakeExecuteArgs);
const enrichedArgs = enrichFunctionArgs(fakeExecuteArgs, {});

assert.isTrue(enrichedArgs.inputs === fakeExecuteArgs.event.inputs);
});
});

describe('processFunctionMiddleware', () => {
it('should call each callback in user-provided middleware', async () => {
const { ...fakeArgs } = createFakeFunctionExecutedEvent() as unknown as AllCustomFunctionMiddlewareArgs;
const { ...fakeArgs } = createFakeFunctionExecutedEvent();
const { processFunctionMiddleware } = await importCustomFunction();

const fn1 = sinon.spy((async ({ next: continuation }) => {
await continuation();
}) as Middleware<CustomFunctionExecuteMiddlewareArgs>);
const fn2 = sinon.spy(async () => {});
const fn2 = sinon.spy(async () => {
});
const fakeMiddleware = [fn1, fn2] as CustomFunctionMiddleware;

await processFunctionMiddleware(fakeArgs, fakeMiddleware);

assert(fn1.called);
assert(fn2.called);
assert(fn1.called, 'first user-provided middleware not called!');
assert(fn2.called, 'second user-provided middleware not called!');
});
});
});

function createFakeFunctionExecutedEvent() {
function createFakeFunctionExecutedEvent(callbackId?: string): AllCustomFunctionMiddlewareArgs {
const func = {
type: 'function',
id: 'somefunc',
callback_id: callbackId || 'callback_id',
title: 'My dope function',
input_parameters: [],
output_parameters: [],
app_id: 'A1234',
date_created: 123456,
date_deleted: 0,
date_updated: 123456,
};
const base = {
bot_access_token: 'xoxb-abcd-1234',
event_ts: '123456.789',
function_execution_id: 'Fx1234',
workflow_execution_id: 'Wf1234',
type: 'function_executed',
} as const;
const inputs = { message: 'test123', recipient: 'U012345' };
const event = {
function: func,
inputs,
...base,
} as const;
return {
event: {
inputs: { message: 'test123', recipient: 'U012345' },
},
payload: {
type: 'function_executed',
function: {
callback_id: 'test_executed_callback_id',
},
inputs: { message: 'test123', recipient: 'U012345' },
bot_access_token: 'xwfp-123',
body: {
api_app_id: 'A1234',
event,
event_id: 'E1234',
event_time: 123456,
team_id: 'T1234',
token: 'xoxb-1234',
type: 'event_callback',
},
client: new WebClient('faketoken'),
complete: () => Promise.resolve({ ok: true }),
context: {
functionBotAccessToken: 'xwfp-123',
functionExecutionId: 'test_executed_callback_id',
isEnterpriseInstall: false,
},
event,
fail: () => Promise.resolve({ ok: true }),
inputs,
logger: createFakeLogger(),
message: undefined,
next: () => Promise.resolve(),
payload: {
function: func,
inputs: { message: 'test123', recipient: 'U012345' },
...base,
},
say: undefined,
};
}

Expand Down
32 changes: 18 additions & 14 deletions src/CustomFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
WebClient,
FunctionsCompleteErrorResponse,
FunctionsCompleteSuccessResponse,
WebClientOptions,
} from '@slack/web-api';
import {
Middleware,
Expand Down Expand Up @@ -62,14 +63,18 @@ export class CustomFunction {
/** Function callback_id */
public callbackId: string;

private appWebClientOptions: WebClientOptions;

private middleware: CustomFunctionMiddleware;

public constructor(
callbackId: string,
middleware: CustomFunctionExecuteMiddleware,
clientOptions: WebClientOptions,
) {
validate(callbackId, middleware);

this.appWebClientOptions = clientOptions;
this.callbackId = callbackId;
this.middleware = middleware;
}
Expand All @@ -88,7 +93,7 @@ export class CustomFunction {
}

private async processEvent(args: AllCustomFunctionMiddlewareArgs): Promise<void> {
const functionArgs = enrichFunctionArgs(args);
const functionArgs = enrichFunctionArgs(args, this.appWebClientOptions);
const functionMiddleware = this.getFunctionMiddleware();
return processFunctionMiddleware(functionArgs, functionMiddleware);
}
Expand All @@ -99,7 +104,6 @@ export class CustomFunction {

/**
* Factory for `complete()` utility
* @param args function_executed event
*/
public static createFunctionComplete(context: Context, client: WebClient): FunctionCompleteFn {
const token = selectToken(context);
Expand All @@ -119,19 +123,18 @@ export class CustomFunction {

/**
* Factory for `fail()` utility
* @param args function_executed event
*/
public static createFunctionFail(context: Context, client: WebClient): FunctionFailFn {
const token = selectToken(context);
const { functionExecutionId } = context;

if (!functionExecutionId) {
const errorMsg = 'No function_execution_id found';
throw new CustomFunctionCompleteFailError(errorMsg);
}

return (params: Parameters<FunctionFailFn>[0]) => {
const { error } = params ?? {};
const { functionExecutionId } = context;

if (!functionExecutionId) {
const errorMsg = 'No function_execution_id found';
throw new CustomFunctionCompleteFailError(errorMsg);
}

return client.functions.completeError({
token,
Expand Down Expand Up @@ -169,7 +172,6 @@ export function validate(callbackId: string, middleware: CustomFunctionExecuteMi

/**
* `processFunctionMiddleware()` invokes each listener middleware
* @param args function_executed event
*/
export async function processFunctionMiddleware(
args: AllCustomFunctionMiddlewareArgs,
Expand Down Expand Up @@ -202,20 +204,22 @@ function selectToken(context: Context): string | undefined {
* - events will *not* continue down global middleware chain to subsequent listeners
* 2. augments args with step lifecycle-specific properties/utilities
* */
export function enrichFunctionArgs(args: any): AllCustomFunctionMiddlewareArgs {
export function enrichFunctionArgs(
args: AllCustomFunctionMiddlewareArgs, webClientOptions: WebClientOptions,
): AllCustomFunctionMiddlewareArgs {
const { next: _next, ...functionArgs } = args;
const enrichedArgs: any = { ...functionArgs };
const enrichedArgs = { ...functionArgs };
const token = selectToken(functionArgs.context);

// Making calls with a functionBotAccessToken establishes continuity between
// a function_executed event and subsequent interactive events (actions)
const client = new WebClient(token, { ...functionArgs.client });
const client = new WebClient(token, webClientOptions);
enrichedArgs.client = client;

// Utility args
enrichedArgs.inputs = enrichedArgs.event.inputs;
enrichedArgs.complete = CustomFunction.createFunctionComplete(enrichedArgs.context, client);
enrichedArgs.fail = CustomFunction.createFunctionFail(enrichedArgs.context, client);

return enrichedArgs;
return enrichedArgs as AllCustomFunctionMiddlewareArgs; // TODO: dangerous casting as it obfuscates missing `next()`
}
Loading