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

Add in block_suggestion interactivity handler support #1645

Merged
merged 9 commits into from
Nov 11, 2022
Merged

Add in block_suggestion interactivity handler support #1645

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cbd1289
Add in support
hello-ashleyintech Nov 2, 2022
778131d
Add unit tests
hello-ashleyintech Nov 2, 2022
8437a04
Update comment on unit test
hello-ashleyintech Nov 2, 2022
0c7b750
Another test comment update
hello-ashleyintech Nov 2, 2022
08ed916
Remove null check for optional enterprise property
hello-ashleyintech Nov 3, 2022
d858b5d
Make token property optional
hello-ashleyintech Nov 3, 2022
e54968c
Update test comments for clarity
hello-ashleyintech Nov 7, 2022
35325fe
Add in new Block Suggestion type
hello-ashleyintech Nov 8, 2022
bb91df8
Add in comment to existing block suggestions type
hello-ashleyintech Nov 8, 2022
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
12 changes: 11 additions & 1 deletion src/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import {
WorkflowStepEdit,
SubscriptionInteraction,
FunctionExecutedEvent,
SlackOptions,
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError, ErrorCode, InvalidCustomPropertyError } from './errors';
Expand Down Expand Up @@ -169,6 +170,12 @@ export interface ViewConstraints {
type?: 'view_closed' | 'view_submission';
}

export interface OptionsConstraints<C extends SlackOptions = SlackOptions> {
type?: C['type'];
block_id?: C extends SlackOptions ? string | RegExp : never;
action_id?: C extends SlackOptions ? string | RegExp : never;
}

// Passed internally to the handleError method
interface AllErrorHandlerArgs {
error: Error; // Error is not necessarily a CodedError
Expand Down Expand Up @@ -1540,8 +1547,11 @@ function buildSource<IsEnterpriseInstall extends boolean>(
// When the app is installed using org-wide deployment, team property will be null
if (
typeof bodyAsActionOrOptionsOrViewActionOrShortcut.team !== 'undefined' &&
bodyAsActionOrOptionsOrViewActionOrShortcut.team !== null
bodyAsActionOrOptionsOrViewActionOrShortcut.team !== null &&
hello-ashleyintech marked this conversation as resolved.
Show resolved Hide resolved
'enterprise_id' in bodyAsActionOrOptionsOrViewActionOrShortcut.team
) {
// TODO: Added null check in if statement, but check
// if there's a better way to bypass this Typescript error for enterprise_id
return bodyAsActionOrOptionsOrViewActionOrShortcut.team.enterprise_id;
hello-ashleyintech marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
1 change: 1 addition & 0 deletions src/SlackFunction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,4 +389,5 @@ describe('SlackFunction utils', () => {
assert.equal(res.fnKeys?.includes('reverse_approval'), false);
});
});
// TODO: Add unit tests for options
});
53 changes: 49 additions & 4 deletions src/SlackFunction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@ import {
SlackEventMiddlewareArgs,
SlackViewAction,
SlackViewMiddlewareArgs,
SlackOptions,
SlackOptionsMiddlewareArgs,
} from './types';

import {
ActionConstraints,
ViewConstraints,
OptionsConstraints,
} from './App';

import {
Expand Down Expand Up @@ -45,17 +48,21 @@ export interface CompleteFunctionArgs {
export type AllSlackFunctionExecutedMiddlewareArgs =
SlackFunctionExecutedMiddlewareArgs &
SlackActionMiddlewareArgs &
SlackOptionsMiddlewareArgs &
AllMiddlewareArgs;

interface FunctionInteractivityMiddleware {
constraints: FunctionInteractivityConstraints,
handler: Middleware<SlackActionMiddlewareArgs> | Middleware<SlackViewMiddlewareArgs>
handler: Middleware<SlackActionMiddlewareArgs> |
Middleware<SlackViewMiddlewareArgs> |
Middleware<SlackOptionsMiddlewareArgs>;
}

type FunctionInteractivityConstraints = ActionConstraints | ViewConstraints;
type FunctionInteractivityConstraints = ActionConstraints | ViewConstraints | OptionsConstraints;
// an array of Action constraints keys as strings
type ActionConstraintsKeys = Extract<(keyof ActionConstraints), string>[];
type ViewConstraintsKeys = Extract<(keyof ViewConstraints), string>[];
type OptionsConstraintsKeys = Extract<(keyof OptionsConstraints), string>[];

interface SlackFnValidateResult { pass: boolean, msg?: string }
export interface ManifestDefinitionResult {
Expand Down Expand Up @@ -194,6 +201,42 @@ export class SlackFunction {
return this;
}

/**
* Attach a block_suggestion interactivity handler to your SlackFunction
*
* ```
* @param handler Provide a handler function
* @returns SlackFunction instance
*/
// TODO: Slack Options pass in here
// Examine Options middleware
// and define options constraints that are similar to ActionConstraints
public blockSuggestion<
Options extends SlackOptions = SlackOptions,
Constraints extends OptionsConstraints<Options> = OptionsConstraints<Options>,
>(
actionIdOrConstraints: string | RegExp | Constraints,
handler: Middleware<SlackOptionsMiddlewareArgs>,
): this {
// normalize constraints
const constraints: OptionsConstraints = (
typeof actionIdOrConstraints === 'string' ||
util.types.isRegExp(actionIdOrConstraints)
) ?
{ action_id: actionIdOrConstraints } :
actionIdOrConstraints;

// declare our valid constraints keys
const validConstraintsKeys: OptionsConstraintsKeys = ['action_id', 'block_id', 'type'];
// cast to string array for convenience
const validConstraintsKeysAsStrings = validConstraintsKeys as string[];

errorIfInvalidConstraintKeys(constraints, validConstraintsKeysAsStrings, handler);

this.interactivityHandlers.push({ constraints, handler });
return this;
}

/**
* Attach a view_submission or view_closed interactivity handler
* to your SlackFunction
Expand Down Expand Up @@ -387,7 +430,7 @@ export function isFunctionExecutedEvent(args: AnyMiddlewareArgs): boolean {

export function isFunctionInteractivityEvent(args: AnyMiddlewareArgs & AllMiddlewareArgs): boolean {
const allowedInteractivityTypes = [
'block_actions', 'view_submission', 'view_closed'];
'block_actions', 'view_submission', 'view_closed', 'block_suggestion'];
if (args.body === undefined || args.body === null) return false;
return (
allowedInteractivityTypes.includes(args.body.type) &&
Expand Down Expand Up @@ -466,7 +509,9 @@ export function validate(callbackId: string, handler: Middleware<SlackEventMiddl
export function errorIfInvalidConstraintKeys(
constraints: FunctionInteractivityConstraints,
validKeys: string[],
handler: Middleware<SlackActionMiddlewareArgs> | Middleware<SlackViewMiddlewareArgs<SlackViewAction>>,
handler: Middleware<SlackActionMiddlewareArgs> |
Middleware<SlackViewMiddlewareArgs<SlackViewAction>> |
Middleware<SlackOptionsMiddlewareArgs>,
): void {
const invalidKeys = Object.keys(constraints).filter(
(key) => !validKeys.includes(key),
Expand Down
54 changes: 40 additions & 14 deletions src/types/options/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,36 +36,62 @@ export type KnownOptionsPayloadFromType<T extends string> = Extract<SlackOptions
*/
export interface BlockSuggestion extends StringIndexed {
Copy link
Contributor Author

@hello-ashleyintech hello-ashleyintech Nov 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely would love a double check on this schema and thoughts on the below. 👇

Right now the schema is a hybrid between Fil's schema (which I'm guessing might be part of a newer block_suggestions approach?) and the original Block Suggestions schema added to the options object, which also contains some legacy things (such as InteractiveMessage and DialogSuggestion), about a year and a half ago.

If we use just the new schema, we do get a few errors (ex: the enterprise_id property not being able to be accessed error thrown in App.ts that is called out above, as well as some issues with accessing certain properties in the unit tests where the new and old schemas deviate). This is causing concern for me that using just the new schema might accidentally wipe compatibility for how folks are currently using this with Bolt within .options(), if they are at all. Would love some thoughts on how to approach this, whether it's keeping it as this hybrid approach or trying to migrate everything to the new approach and mitigating any errors that pop up!

Copy link
Member

@srajiang srajiang Nov 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

At the moment I lean against trying to consume these types, as internally in bolt-js we already have a model of block_suggestion which needs to be compatible with legacy and next-gen event payloads. I don't want to be using different types internally for blockSuggestion handling vs. .options block suggestion handling events because the underlying events aren't different between next-gen and today's block_suggestion (nor do we want them to drift too much).

It would also add additional dependency for us on deno_slack_sdk to be selectively consuming deno_slack_sdk function handler types. Currently we're keeping this dependency limited to manifest generation.

Would love to know @filmaj's thoughts on this though since sounds like this could be a candidate for the shared types work he's been prototyping.

Copy link
Contributor

Choose a reason for hiding this comment

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

The 'original' Block Suggestions schema that @hello-ashleyintech links to looks no longer valid to me. I just played around with this PR and it worked great, but, the block suggestion payloads coming in look more like the type I wrote up in the deno-slack-sdk than the original schema in bolt-js.

As for how to go about that, I leave it to you. Whether you want to update the original schema in bolt or consume the deno one, your call. But, definitely, do some testing on your end to inspect the shape of that payload (in both enterprise and non-enterprise workspaces since the shape changes somewhat based on that. I have access to one that Jim set up, ping me if you want in on that).

Copy link
Contributor Author

@hello-ashleyintech hello-ashleyintech Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Update to this: I ended up adding a new type for Block Suggestions in the next-gen interactivity scenario and left the old Block Suggestions type under options as is! this is to continue legacy support of the original BlockSuggestions while still enabling the next-gen work. See this comment for more context!

Copy link
Contributor

Choose a reason for hiding this comment

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

So therefore, depending on 1.0 vs. 2.0 context, the payload shapes differ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for confirming! What a headache, though...

type: 'block_suggestion';
block_id: string;
action_id: string;
value: string;

api_app_id: string;
block_id: string;
channel?: {
id: string;
name: string;
};
is_enterprise_install?: boolean;
enterprise?: {
id: string;
name: string;
} | null;
hello-ashleyintech marked this conversation as resolved.
Show resolved Hide resolved
message?: {
app_id: string;
blocks: {
block_id: string;
type: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
}[];
is_locked: boolean;
latest_reply?: string;
metadata?: {
event_type: string;
event_payload: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
};
};
reply_count: number;
reply_users: string[];
reply_users_count: number;
team: string;
text: string;
thread_ts: string;
ts: string;
type: 'message';
user: string;
};
team: {
id: string;
domain: string;
enterprise_id?: string;
enterprise_name?: string;
} | null;
channel?: {
id: string;
name: string;
};
user: {
id: string;
name: string;
team_id?: string;
};
token: string; // legacy verification token
hello-ashleyintech marked this conversation as resolved.
Show resolved Hide resolved
value: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
container: StringIndexed;
// exists for blocks in either a modal or a home tab
view?: ViewOutput;
// exists for enterprise installs
is_enterprise_install?: boolean;
enterprise?: {
id: string;
name: string;
};
}

/**
Expand Down