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.

Sign up for GitHub

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

Fixes and polish for stable release #2128

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Fixes and polish for stable release #2128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7966e7
Fixes and polish for stable release
misscoded Aug 7, 2024
d390d8f
Added complete, fail and inputs parameters on `SlackActionMiddlewareA…
filmaj Aug 8, 2024
7ee465b
Remote Functions: Change how webclient options are constructed in Cus…
filmaj Aug 12, 2024
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
View file
Open in desktop
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
22 changes: 17 additions & 5 deletions src/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
SlashCommand,
WorkflowStepEdit,
SlackOptions,
FunctionInputs,
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever, isBodyWithTypeEnterpriseInstall, isEventTypeToSkipAuthorize } from './helpers';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError, ErrorCode, InvalidCustomPropertyError } from './errors';
Expand Down Expand Up @@ -502,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 @@ -529,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 Expand Up @@ -964,9 +969,12 @@
retryReason: event.retryReason,
};

// Extract function-related information and augment to context
const { functionExecutionId, functionBotAccessToken } = extractFunctionContext(body);
if (functionExecutionId) { context.functionExecutionId = functionExecutionId; }
// Extract function-related information and augment context
const { functionExecutionId, functionBotAccessToken, functionInputs } = extractFunctionContext(body);
if (functionExecutionId) {
context.functionExecutionId = functionExecutionId;

Check warning on line 975 in src/App.ts

View check run for this annotation

Codecov / codecov/patch

src/App.ts#L975 

Added line #L975 was not covered by tests
if (functionInputs) { context.functionInputs = functionInputs; }
}

if (this.attachFunctionToken) {
if (functionBotAccessToken) { context.functionBotAccessToken = functionBotAccessToken; }
Expand Down Expand Up @@ -1029,6 +1037,7 @@
ack?: AckFn<any>;
complete?: FunctionCompleteFn;
fail?: FunctionFailFn;
inputs?: FunctionInputs;
} = {
body: bodyArg,
payload,
Expand Down Expand Up @@ -1088,6 +1097,7 @@
if (type === IncomingEventType.Action && context.functionExecutionId !== undefined) {
listenerArgs.complete = CustomFunction.createFunctionComplete(context, client);
listenerArgs.fail = CustomFunction.createFunctionFail(context, client);
listenerArgs.inputs = context.functionInputs;

Check warning on line 1100 in src/App.ts

View check run for this annotation

Codecov / codecov/patch

src/App.ts#L1100 

Added line #L1100 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just sharing as an option, free to ignore- all listener arguments could be created and assigned with Object.assign if we wanted to reduce the number of CustomFunction exports

}

if (token !== undefined) {
Expand Down Expand Up @@ -1599,6 +1609,7 @@
function extractFunctionContext(body: StringIndexed) {
let functionExecutionId;
let functionBotAccessToken;
let functionInputs;

// function_executed event
if (body.event && body.event.type === 'function_executed' && body.event.function_execution_id) {
Expand All @@ -1610,9 +1621,10 @@
if (body.function_data) {
functionExecutionId = body.function_data.execution_id;
functionBotAccessToken = body.bot_access_token;
functionInputs = body.function_data.inputs;

Check warning on line 1624 in src/App.ts

View check run for this annotation

Codecov / codecov/patch

src/App.ts#L1624 

Added line #L1624 was not covered by tests
}

return { functionExecutionId, functionBotAccessToken };
return { functionExecutionId, functionBotAccessToken, functionInputs };
}

// ----------------------------
Expand Down
144 changes: 104 additions & 40 deletions src/CustomFunction.spec.ts
View file
Open in desktop
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 @@ -65,8 +65,8 @@ describe('CustomFunction class', () => {
assert(fakeNext.called);
});

it('should call next if not a workflow step event', async () => {
const fn = new CustomFunction('test_view_callback_id', MOCK_MIDDLEWARE_SINGLE);
it('should call next if not a function executed event', async () => {
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,59 +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
Loading