Add support for custom metadata events #86

shapirone · 2022-08-24T01:01:39Z

Summary

This PR adds support for defining events on the App Manifest.

Feature Details

When an app uses chat.postMessage from inside a function, they can optionally pass a metadata object. That metadata object will have an event_type and an event_payload. If the event_type matches one of the keys from the app manifest's events object, the event_payload needs to match the metadata schema.

If there are required properties that are missing, the message will still successfully post but the metadata will be dropped and the response will include a warning and a response_metadata object.
- There are plans to eventually support a "strict mode" that will fail the postMessage call entirely, but that's not currently supported.
If there are additional properties that are not defined in the manifest, the message will be successfully posted with metadata. If validation should catch the extra properties, the Event must set additionalProperties: false

SDK Usage

// manifest.ts
export const MyEvent = DefineEvent({
  name: "my_event",
  type: Schema.types.object,
  properties: {
    id: { type: Schema.types.string },
    summary: { type: Schema.types.string },
  },
  required: ["id", "summary"],
});

export default Manifest({
  ...,
  events: [MyEvent], 
});

// function.ts
import { MyEvent } from "../manifest.ts";

...
const post = await client.chat.postMessage({
    channel: inputs.channel,
    text: "Hello this has metadata",
    metadata: {
      event_type: MyEvent, // this turns into the "my_event" string
      event_payload: {
        id: "my_id",
        summary: "This is a summary of the event",
      },
    },
  });

Testing

Create an Event on your manifest of object type and verify manifest validation works
Turn that object into a custom type and verify manifest validation works
Use that event in a chat.postMessage call where your structure matches the expectation
Change the chat.postMessage call where the structure no longer matches
BONUS: Set up an event trigger to fire based on the message metadata being posted

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.

codecov · 2022-08-24T01:03:11Z

Codecov Report

Merging #86 (dab8bde) into main (55a193a) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   96.21%   96.34%   +0.13%     
==========================================
  Files          43       44       +1     
  Lines        1610     1669      +59     
  Branches       87       90       +3     
==========================================
+ Hits         1549     1608      +59     
  Misses         59       59              
  Partials        2        2

Impacted Files	Coverage Δ
src/events/mod.ts	`100.00% <100.00%> (ø)`
src/manifest/mod.ts	`90.04% <100.00%> (+0.59%)`	⬆️
src/mod.ts	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

filmaj · 2022-08-25T17:34:24Z

I was unable to test step 1. Here's my manifest.ts:

➜ cat manifest.ts
import { Manifest, DefineEvent, Schema } from "deno-slack-sdk/mod.ts";
import { ApprovalWorkflow } from "./workflows/approval.ts";

export const MyEvent = DefineEvent({
  name: "my_event",
  type: Schema.types.object,
  properties: {
    id: { type: Schema.types.string },
    summary: { type: Schema.types.string },
  },
  required: ["id", "summary"],
});

export default Manifest({
  name: "interactive-approval",
  description: "Approving allthethings",
  icon: "assets/icon.png",
  workflows: [ApprovalWorkflow],
  outgoingDomains: [],
  botScopes: ["commands", "chat:write", "chat:write.public"],
  events: [MyEvent],
});

And here's the output of the manifest validate command (using the latest slack-cli main branch):

➜ slak manifest validate
Check /Users/fmaj/.slack/slack-debug.log for full error logs

🚫  App manifest generated for your project is invalid. (invalid_manifest)

Error Details:

1: Event Subscription requires a Request URL (requires_request_url)
Source: /settings/event_subscriptions

2: Event Subscription requires Socket Mode if no Request URL is provided (requires_socket_mode_enabled)
Source: /settings/event_subscriptions

filmaj · 2022-08-25T18:05:56Z

Looks like there are issues with the manifest validate command, should probably update the testing instructions to just use deploy.

filmaj · 2022-08-25T18:22:15Z

Testing notes, continued:

Step 2. Using custom type in this way worked fine:

const fancyAssBoolean = DefineType({
  name: "fancyAssBoolean",
  type: Schema.types.object,
  properties: {
    aBoolean: { type: "boolean" },
  },
});

export const MyEvent = DefineEvent({
  name: "my_event",
  type: fancyAssBoolean,
  // required: ["aBoolean"],
});

If I uncomment the required prop in DefineEvent, though, I get this error:

🚫  App manifest generated for your project is invalid. (invalid_manifest)

Error Details:

1: failed to match exactly one allowed schema (failed_constraint)
Source: /events/my_event

Step 3. Use the event in a postMessage metadata. Worked great!

Step 4. Use the event in a postMessage metadata, but where structure doesn't match. So with this Event w/ Custom Type definition:

const fancyAssBoolean = DefineType({
  name: "fancyAssBoolean",
  type: Schema.types.object,
  properties: {
    aBoolean: { type: "boolean" },
  },
});

export const MyEvent = DefineEvent({
  name: "my_event",
  type: fancyAssBoolean,
  // required: ["aBoolean"],
});

I posted a message like so:

  const resp = await client.chat.postMessage({
      channel: inputs.approval_channel_id,
      blocks: renderApprovalMessage(inputs),
      metadata: {
        event_type: MyEvent,
        event_payload: {
          aBoolean: "string"
        },
      }
    });

... and no metadata is attached to the message at all.

If I posted a message like so:

  const resp = await client.chat.postMessage({
      channel: inputs.approval_channel_id,
      blocks: renderApprovalMessage(inputs),
      metadata: {
        event_type: MyEvent,
        event_payload: {
          nope: true
        },
      }
    });

Then the above metadata is attached to the message - even though the structure is wrong 🤔

shapirone · 2022-08-25T19:24:29Z

Step 2. Using custom type in this way worked fine:
If I uncomment the required prop in DefineEvent, though, I get this error:

Good find @filmaj, but I consider this unrelated to this PR. It's happening because we don't support overriding properties on the CustomType and is falling back to a generic error. We should share this finding with the primitives team and they can decide what to do about it (improve the error or allow this functionality).

Step 4. Use the event in a postMessage metadata, but where structure doesn't match. So with this Event w/ Custom Type definition:

I posted a message like so:

  const resp = await client.chat.postMessage({
      channel: inputs.approval_channel_id,
      blocks: renderApprovalMessage(inputs),
      metadata: {
        event_type: MyEvent,
        event_payload: {
          aBoolean: "string"
        },
      }
    });

... and no metadata is attached to the message at all.

This is expected since aBoolean: "string" fails the validation check because it's not a valid boolean value. If you console.log(resp) I expect you get the warning back!

If I posted a message like so:

  const resp = await client.chat.postMessage({
      channel: inputs.approval_channel_id,
      blocks: renderApprovalMessage(inputs),
      metadata: {
        event_type: MyEvent,
        event_payload: {
          nope: true
        },
      }
    });

Then the above metadata is attached to the message - even though the structure is wrong 🤔

This is also expected. The aBoolean property is not marked as required, so validation is okay with it not being passed. Since fancyAssBoolean doesn't specify that additionalProperties: false the validation allows unexpected nope property to be passed.

selfcontained · 2022-08-25T19:37:02Z

src/events/mod.ts

+  constructor(
+    public definition: Def,
+  ) {
+    this.id = definition.name;


Should we just call this name?

I chose id because it matches Custom Types and events is pretty much the same. That being said, if we went with name it will match Datastores (and the property that's being set). I don't have a strong preference here, open to changing if anyone else does!

Consistency is good, so am fine w/ this, just couldn't recall if we had odd naming in Types due to callback_id => id => name refactoring, so thought if so, maybe could keep this clearer here.

jmplexi · 2022-08-25T20:03:05Z

src/events/mod.ts

+  toJSON() {
+    return this.generateReferenceString();
+  }
+


Is it intended for toString() and toJSON() to do the same thing?

Yep! Overwriting these prototype methods is a pattern we follow elsewhere (like in types and workflows) to make sure that when a developer references the CustomEvent instance, we convert it to what our API endpoints expect for the manifest and API clients rather than the full object.

toJSON() supports the case where we convert an object into a string to pass to our APIs
toString()supports the case where devs may want to do something like This is the event: ${MyEvent}

filmaj

The testing steps, including creating a custom event w/ custom type, and registering an event trigger on that thing, worked. Well done sir.

selfcontained

Looks good, and works as described.

Add support for custom metadata events

651e17e

shapirone force-pushed the neil-support-custom-events branch from 3218af2 to 651e17e Compare August 25, 2022 16:46

shapirone marked this pull request as ready for review August 25, 2022 16:50

shapirone requested a review from a team as a code owner August 25, 2022 16:50

shapirone added 2 commits August 25, 2022 09:56

Update test

13bdbd1

moar tests

332a73a

selfcontained reviewed Aug 25, 2022

View reviewed changes

jmplexi reviewed Aug 25, 2022

View reviewed changes

filmaj approved these changes Aug 29, 2022

View reviewed changes

selfcontained approved these changes Aug 29, 2022

View reviewed changes

Merge branch 'main' into neil-support-custom-events

dab8bde

shapirone merged commit 6650c12 into main Aug 29, 2022

shapirone deleted the neil-support-custom-events branch August 29, 2022 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for custom metadata events #86

Add support for custom metadata events #86

shapirone commented Aug 24, 2022 •

edited

codecov bot commented Aug 24, 2022 •

edited

filmaj commented Aug 25, 2022

filmaj commented Aug 25, 2022

filmaj commented Aug 25, 2022

shapirone commented Aug 25, 2022

selfcontained Aug 25, 2022

shapirone Aug 25, 2022

selfcontained Aug 29, 2022

jmplexi Aug 25, 2022

shapirone Aug 25, 2022

filmaj left a comment

selfcontained left a comment

Add support for custom metadata events #86

Add support for custom metadata events #86

Conversation

shapirone commented Aug 24, 2022 • edited

Summary

Feature Details

SDK Usage

Testing

Requirements (place an x in each [ ])

codecov bot commented Aug 24, 2022 • edited

Codecov Report

filmaj commented Aug 25, 2022

filmaj commented Aug 25, 2022

filmaj commented Aug 25, 2022

shapirone commented Aug 25, 2022

selfcontained Aug 25, 2022

Choose a reason for hiding this comment

shapirone Aug 25, 2022

Choose a reason for hiding this comment

selfcontained Aug 29, 2022

Choose a reason for hiding this comment

jmplexi Aug 25, 2022

Choose a reason for hiding this comment

shapirone Aug 25, 2022

Choose a reason for hiding this comment

filmaj left a comment

Choose a reason for hiding this comment

selfcontained left a comment

Choose a reason for hiding this comment

shapirone commented Aug 24, 2022 •

edited

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

codecov bot commented Aug 24, 2022 •

edited