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

Fix #534 respond support in view_submission listeners (it works only when response_url_enabled is true) #889

Merged
merged 1 commit into from
Apr 21, 2021
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
45 changes: 45 additions & 0 deletions src/App.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,51 @@ describe('App', () => {
ack: noop,
});

// Assert
assert.equal(fakeAxiosPost.callCount, 1);
// Assert that each call to fakeAxiosPost had the right arguments
assert(fakeAxiosPost.calledWith(responseUrl, responseObject));
});
it('should be able to use respond for view_submission payloads', async () => {
// Arrange
const responseObject = { text: 'response' };
const responseUrl = 'https://fake.slack/response_url';
const fakeAxiosPost = sinon.fake.resolves({});
const overrides = buildOverrides([withNoopWebClient(), withAxiosPost(fakeAxiosPost)]);
const App = await importApp(overrides); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.view('view-id', async ({ respond }) => {
await respond(responseObject);
});
app.error(fakeErrorHandler);
await fakeReceiver.sendEvent({
ack: noop,
body: {
type: 'view_submission',
team: {},
user: {},
view: {
id: 'V111',
type: 'modal',
callback_id: 'view-id',
state: {},
title: {},
close: {},
submit: {},
},
response_urls: [
{
block_id: 'b',
action_id: 'a',
channel_id: 'C111',
response_url: 'https://fake.slack/response_url',
},
],
},
});

// Assert
assert.equal(fakeAxiosPost.callCount, 1);
// Assert that each call to fakeAxiosPost had the right arguments
Expand Down
21 changes: 15 additions & 6 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SecureContextOptions } from 'tls';
import util from 'util';
import { WebClient, ChatPostMessageArguments, addAppMetadata, WebClientOptions } from '@slack/web-api';
import { Logger, LogLevel, ConsoleLogger } from '@slack/logger';
import axios, { AxiosInstance } from 'axios';
import axios, { AxiosInstance, AxiosResponse } from 'axios';
import SocketModeReceiver from './receivers/SocketModeReceiver';
import HTTPReceiver, { HTTPReceiverOptions } from './receivers/HTTPReceiver';
import {
Expand Down Expand Up @@ -765,11 +765,10 @@ export default class App {

// Set respond() utility
if (body.response_url) {
listenerArgs.respond = (response: string | RespondArguments): Promise<any> => {
const validResponse: RespondArguments = typeof response === 'string' ? { text: response } : response;

return this.axios.post(body.response_url, validResponse);
};
listenerArgs.respond = buildRespondFn(this.axios, body.response_url);
} else if (typeof body.response_urls !== 'undefined' && body.response_urls.length > 0) {
// This can exist only when view_submission payloads - response_url_enabled: true
listenerArgs.respond = buildRespondFn(this.axios, body.response_urls[0].response_url);
Comment on lines +770 to +771
Copy link
Member

Choose a reason for hiding this comment

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

Just for my own knowledge, are we only handling the first response_urls[0] because that keeps the functionality consistent with other areas of Bolt-JS that only handle the first response url?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mwbrooks Good point. As of today, the response_urls can have only one element and we don't have any plans to have more elements in the short term. That's the reason why we can safely pick up the first element here, and thus far we can provide respond utility, which is consistent with other areas.

If the array start having multiple options in the future, we still can provide respond with the first element. When a developer would like to use a non-primary element, the right way to do so is to select the one from the array and then use axios directly with the URL. We may want to provide an easy-to-use utility for it.

}

// Set ack() utility
Expand Down Expand Up @@ -1124,6 +1123,16 @@ function selectToken(context: Context): string | undefined {
return context.botToken !== undefined ? context.botToken : context.userToken;
}

function buildRespondFn(
axiosInstance: AxiosInstance,
response_url: string,
): (response: string | RespondArguments) => Promise<AxiosResponse> {
return async (message: string | RespondArguments) => {
const normalizedArgs: RespondArguments = typeof message === 'string' ? { text: message } : message;
return axiosInstance.post(response_url, normalizedArgs);
};
}

// ----------------------------
// For listener registration methods

Expand Down
3 changes: 2 additions & 1 deletion src/types/view/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { View, PlainTextElement } from '@slack/types';
import { StringIndexed } from '../helpers';
import { AckFn } from '../utilities';
import { AckFn, RespondFn } from '../utilities';

/**
* Known view action types
Expand All @@ -19,6 +19,7 @@ export interface SlackViewMiddlewareArgs<ViewActionType extends SlackViewAction
view: this['payload'];
body: ViewActionType;
ack: ViewAckFn<ViewActionType>;
respond: RespondFn;
}

interface PlainTextElementOutput {
Expand Down