Normalize Payload Dispatching - prep for more payload types #15

selfcontained · 2022-06-23T19:23:38Z

Summary

This was some work I did to prep for adding support for more payload types that we'll be receiving for interactive events.

I've consolidated how the payloads are dispatched in DispatchPayload() that both the hosted and local-run entry points use now. Each now provides a list of the function files that should be looked for when dispatching, which allows us to diverge in the local run scenario and use the file defined in the function definitions source_file. This let me remove the payload callback_id overriding we were doing as well.

I've also set things up so we can drop in some handlers for the interactivity payloads as well, which will come in a followup PR.

Testing

I've updated the LoadModule() tests to account for the changes I made.

To test manually, you can pull this repo down and update your slack.json file's start hook like so:

"start": "deno run -q --config=deno.jsonc --allow-read --allow-net file:///Users/<your-user>/<path-to>/deno-slack-runtime/src/local-run.ts"

Then run deno run to run locally and it will use the local runtime.

Requirements (place an `x` in each `[ ]`)

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

… payloads for a function

…f directory

selfcontained · 2022-06-23T19:25:31Z

src/dispatch-payload.ts

+  // deno-lint-ignore no-explicit-any
+  let resp: any = {};
+
+  switch (validEventType) {


This is where we'll add a few more case statements for new interactivity event types.

selfcontained · 2022-06-23T19:27:13Z

src/types.ts

  body: Body;
  context: {
    bot_access_token: string;
    variables: EnvironmentVariables;
  };
 };

+export type ValidInvocationPayloadBody = FunctionInvocationBody;


This will get extended to support new interactivity event payloads as well, so just setting it up.

selfcontained · 2022-06-23T19:30:31Z

src/dispatch-payload.ts

@@ -0,0 +1,62 @@
+import { LoadFunctionModule } from "./load-function-module.ts";


The bulk of the logic in this file come from consolidating mod.ts and local-run.ts

shapirone

LGTM! Verified local run still works and added comments but none are blocking 🎉

shapirone · 2022-06-23T23:31:50Z

src/local-run.ts

  // The CLI expects a JSON payload to be output to stdout
  // This is formalized in the `run` hook of the CLI/SDK Tech Spec:
  // https://corp.quip.com/0gDvAsqoaaYE/Proposal-CLI-SDK-Interface#temp:C:fOC1991c5aec8994d0db01d26260
-  console.log("{}");
+  console.log(JSON.stringify(resp || {}));


❔Do we get anything from logging the response here for the CLI?

Yah, the CLI uses this output as the response to the HTTP request that delivered the event/payload. It will let some interactivity handlers, like view_submission, to do things like return validation error responses.

shapirone · 2022-06-24T21:29:11Z

src/tests/load-function-module.test.ts

-      },
-      Error,
+    const jsModule = await LoadFunctionModule(
+      [`${functionsDir}/nonexistnent.ts`],


nit: It literally doesn't matter at all, but this nonexistent filename has a typo 😂

🤣 feels slightly appropriate then

shapirone · 2022-06-24T21:32:34Z

src/tests/load-function-module.test.ts

    },
  );

+  await t.step("should load the correct file when given multiple", async () => {


💭 Wondering if we should have two tests here:

Validates that a ts file will be used instead of a js file when they both exist.

Validates that it'll use the existing js file over a ts file that doesn't exist.

Good call, I'll add this as an issue and flag it good first issue (and also turn on issues for this repo)

shapirone · 2022-06-24T21:35:54Z

src/types.ts

+
+export const EventTypes = {
+  FUNCTION_EXECUTED: "function_executed",
+} as const;


nit: I feel like our types.ts files normally only contain TS types, but EventTypes is also an actual value. I feel like there is not really a perfect spot for it that already exists, but I was thinking we could:

Put this in a separate file named something like event-types.ts

Place it at the top of dispatch-payload.ts until we need it somewhere else

Leave it here 🤷

I'll move it to it's own file, but if it's ok I'll do it in the PR that adds the new types, it'll be a bit easier there w/ the changes I have around that area. By easier, I mean I won't have to deal with merge conflicts cause I'm lazeeee

selfcontained added 7 commits June 8, 2022 14:21

preliminary work to add support for block_actions and view_submission…

6a3ef80

… payloads for a function

consolidate dispatching payload accross local and deployed runtime

897e2b2

updating main runtime entrypoint to provide potential files instead o…

5ad2329

…f directory

adding view_closed handling

328d2c2

removing from type for now

a1f4aa0

pulling out new event types for now

7a15e75

removing import

50e609b

selfcontained commented Jun 23, 2022

View reviewed changes

shifting back to where it was

737d94e

selfcontained force-pushed the bradh-normalize-payload-dispatch branch from 30b24d2 to 737d94e Compare June 23, 2022 19:29

selfcontained commented Jun 23, 2022

View reviewed changes

selfcontained requested a review from shapirone June 23, 2022 19:32

shapirone approved these changes Jun 24, 2022

View reviewed changes

selfcontained mentioned this pull request Jun 24, 2022

Add additional tests for LoadFunctionModule test #16

Open

3 tasks

selfcontained merged commit ba6eea9 into main Jun 24, 2022

selfcontained deleted the bradh-normalize-payload-dispatch branch June 24, 2022 22:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize Payload Dispatching - prep for more payload types #15

Normalize Payload Dispatching - prep for more payload types #15

selfcontained commented Jun 23, 2022

selfcontained Jun 23, 2022

selfcontained Jun 23, 2022

selfcontained Jun 23, 2022

shapirone left a comment

shapirone Jun 23, 2022

selfcontained Jun 24, 2022

shapirone Jun 24, 2022

selfcontained Jun 24, 2022

shapirone Jun 24, 2022

selfcontained Jun 24, 2022

selfcontained Jun 24, 2022

shapirone Jun 24, 2022

selfcontained Jun 24, 2022

		@@ -0,0 +1,62 @@
		import { LoadFunctionModule } from "./load-function-module.ts";

Normalize Payload Dispatching - prep for more payload types #15

Normalize Payload Dispatching - prep for more payload types #15

Conversation

selfcontained commented Jun 23, 2022

Summary

Testing

Requirements (place an x in each [ ])

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shapirone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Requirements (place an `x` in each `[ ]`)