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

Update message types #782

Merged
merged 6 commits into from
Feb 10, 2021

Conversation

shaydewael
Copy link
Contributor

Summary

Addresses #774 and updates other fields that were missing

Requirements (place an x in each [ ])

@gitwave gitwave bot added the untriaged label Feb 8, 2021
@shaydewael shaydewael requested a review from seratch February 8, 2021 22:00
@shaydewael shaydewael force-pushed the update-incorrect-message-types branch from 2be890f to 67c8091 Compare February 8, 2021 22:53
@shaydewael
Copy link
Contributor Author

For context, I found the file type here: https://api.slack.com/types/file

@@ -13,16 +13,23 @@ export type MessageEvent =
export interface GenericMessageEvent {
type: 'message';
subtype: undefined;
event_ts: string;
team: string;
Copy link
Member

Choose a reason for hiding this comment

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

As this field does not always exist, we may want to make it optional.

text: string;
blocks?: (KnownBlock | Block)[];
Copy link
Member

Choose a reason for hiding this comment

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

Also, attachments is missing.

@seratch
Copy link
Member

seratch commented Feb 9, 2021

Can you update the spec file to be compatible with the new type definition?

TSError: ⨯ Unable to compile TypeScript:
Error: src/middleware/builtin.spec.ts(752,7): error TS2322: Type '{ type: "message"; subtype: "bot_message"; channel: string; user: string; ts: string; text: string; bot_id: string; channel_type: "channel"; }' is not assignable to type 'MessageEvent'.
  Property 'event_ts' is missing in type '{ type: "message"; subtype: "bot_message"; channel: string; user: string; ts: string; text: string; bot_id: string; channel_type: "channel"; }' but required in type 'BotMessageEvent'.

Apart from this build failure, this PR looks great to me 👍

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

You can merge this once the build failure is fixed.

@seratch
Copy link
Member

seratch commented Feb 9, 2021

This branch still have a lint error. You need to apply code formatter to the modified spec files. Can you run npx prettier -w src/ && npm test and push the change?

@stevengill stevengill added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:minor and removed untriaged labels Feb 10, 2021
@shaydewael shaydewael force-pushed the update-incorrect-message-types branch from b205412 to b08dd3d Compare February 10, 2021 00:33
@stevengill stevengill merged commit 5e2db65 into slackapi:main Feb 10, 2021
@seratch seratch mentioned this pull request Mar 5, 2021
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants