slackapi / bolt-js

A framework to build Slack apps using JavaScript
https://slack.dev/bolt-js
MIT License
2.74k stars 394 forks source link

Cannot push modal view using `trigger_id` #2152

Closed kronosapiens closed 3 months ago

kronosapiens commented 3 months ago

@slack/bolt version

"@slack/bolt": "^3.17.0",

Your App and Receiver Configuration

const app = new App({
  logLevel: LogLevel.WARN,
  signingSecret: process.env.CHORES_SIGNING_SECRET,
  clientId: process.env.CHORES_CLIENT_ID,
  clientSecret: process.env.CHORES_CLIENT_SECRET,
  stateSecret: process.env.STATE_SECRET,
  customRoutes: [ common.homeEndpoint('Chores') ],
  scopes: [
    'channels:history',
    'channels:join',
    'chat:write',
    'commands',
    'groups:history',
    'users:read',
  ],
  installationStore: {
    storeInstallation: async (installation) => {
      await Admin.addHouse(installation.team.id, installation.team.name);
      await Admin.updateHouseConf(installation.team.id, CHORES_CONF, { oauth: installation });
      console.log(`chores installed @ ${installation.team.id}`);
    },
    fetchInstallation: async (installQuery) => {
      ({ choresConf } = (await Admin.getHouse(installQuery.teamId)));
      return choresConf.oauth;
    },
    deleteInstallation: async (installQuery) => {
      await Admin.updateHouseConf(installQuery.teamId, CHORES_CONF, { oauth: null });
      console.log(`chores uninstalled @ ${installQuery.teamId}`);
    },
  },
  installerOptions: { directInstall: true },
});

Node.js runtime version

"node": "^20.11.0",

Steps to reproduce:

See PR here

  1. Submit a modal view
  2. Attempt to push another view onto the stack using the callback trigger_id

Expected result:

  1. New view is pushed onto the stack

Actual result:

  1. App receives [ERROR] bolt-app Error: An API error occurred: invalid_trigger_id

This appears to be the same as this issue in the Python SDK

The issue suggests a workaround of sending an HTTP response instead of using the Bolt SDK. This is not an ideal solution; the SDK should be able to handle this common use-case directly.

Requirements

kronosapiens commented 3 months ago

Ok, I was able to update the view by moving the ack call to the end of the block as follows:

  await ack({ response_action: 'push', view });

This is a reasonable solution, but the docs should be updated to make this more clear; it is a departure from the modal semantics presented in the docs.

Update: Reading through the docs, this is described clearly in the "Getting Started" guide. However, same description should appear in the Modal docs proper, as that is likely to be most developers ongoing point of reference.

seratch commented 3 months ago

Hi @kronosapiens, thanks for writing in!

Your issue title mentions "trigger_id," but pushing a new modal using the ack() method does not require it. Also, app.view listeners are expected to call ack() once with sufficient data within 3 seconds. This requirement can be confusing, but it looks like you've already figured out what to do. Thanks for checking the existing discussions and issues!

We are unable to thoroughly review your code, but if you have a simplified question, please feel free to write in. Otherwise, if everything is clear now, would you mind closing this?

kronosapiens commented 3 months ago

Everything is fine on my end, but I would still recommend duplicating this code snippet in the modal docs.

// Update the view on submission 
app.view('modal-callback-id', async ({ ack, body }) => {
  await ack({
    response_action: 'update',
    view: buildNewModalView(body),
  });
});
zimeg commented 2 months ago

Hey @kronosapiens 👋 Sorry to hear these patterns of ack caught you by surprise! I'm thinking the current Bolt JS documentation has this case covered, and the docs at api.slack.com do too albeit in a more generic sense.

The separation of "SDK and framework specific implementations" on slack.dev and "API usage" on api.slack.com seems like an alright pattern to uphold, though I believe we can do better at surfacing implementation references... 🤔

Really appreciate the suggestion though! And of course feel free to disagree- I'm always open to shifting how I think about the docs! 📚

kronosapiens commented 2 months ago

Hey @zimeg, I appreciate you engaging with me on this.

I agree that the /concepts docs do cover this case, and that the api docs cover the case more generically. I also agree that separating the SDK and API docs makes sense, given that they are somewhat orthogonal products.

I think the major stumbling block here is that there does not seem to be a way to implement a modal flow as a series of inputs using the API, which creates a very confusing semantics. Users use the api to open the modal, and they can use the api to push modals in response to actions, but cannot use the api to update modal in response to input submissions -- this requires response_action.

At baseline, a developer should be able to implement a flow entirely inside of the API. I would argue that the quirk that pushing modal views in response to input submissions requires a response_action through ack() represents a semantics violation.

In my case, I initially implemented a multi-modal flow using actions because I had repeatedly tried and failed to push new views using the trigger_id I received from a form submission, which was the behavior I expected.