Allow mrkdwn_in, callback_id fields in message attachments

[DominikPalo]

Summary

A message attachment supports also the mrkdwn_in property, which is used to enable formatting on attachment fields (see https://api.slack.com/docs/message-formatting).

I added also the callback_id field used with interactive messages (see https://api.slack.com/docs/interactive-message-field-guide)

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]

Codecov Report

Merging #528 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #528   +/-   ##
=======================================
  Coverage   79.56%   79.56%           
=======================================
  Files           6        6           
  Lines         279      279           
  Branches       43       43           
=======================================
  Hits          222      222           
  Misses         39       39           
  Partials       18       18

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bed1fc...951fe41. Read the comment docs.

[aoberoi]

while i see that this is an accepted field, there are no values other than 'default', so i don't think it adds any value to type it (i'm not sure there will ever be any other possible values, and each additional property has a visual cost when your editor is trying to give you hints)

[DominikPalo]

thanks, I understand, but our UX team uses Walkiebot for designing flows and messages. We also plan to fetch those messages designed by Walkiebot automatically to our integrations (using the Walkiebot API), but there is a problem - Walkiebot generates JSON/JS of designed attachments with attachmet_type field included (see the attached screenshot), therefore the output is not compatible with this SDK and we have to manually remove this field from every exported message. I can contact Walkiebot and ask them to remove this field, but it seems, they strictly stick to Slack API specs, so maybe as a first step is good to remove this unnecessary field from Slack API documentation.

[aoberoi]

oh that's really cool! i'd love to make sure there's a great experience when integrating with Walkiebot, we recommend their product to loads of designers/developers.

what kind of error are you getting when you have the additional property in your API call? if you're in a javascript project, the client shouldn't care about additional properties. if you're in a typescript project, i thought our addition of the AuxiliaryArguments type would make this usage valid as well (interesting use case for the discussion @ #496)

[DominikPalo]

thanks, we use TypeScript for all our projects - but I thought before that our attachments were incompatible with the MessageAttachment because of missing the attachment_type field, but as I see now, my assumption is not true (thanks to the AuxiliaryArguments). Our incompatibility comes from missing types in the nested field MessageAttachment.actions - the current type definition for actions contains only two fields:

actions?: {
    type: string;
    text?: string;
  }[];

but the attachment actions in our project contains additional fields. So I will prepare a separate PR also with complete definitions for the attachment actions.

[aoberoi]

this works and its right, but i was a little confused by the syntax. would you mind making a named string-based enum type and then setting this type to an array of that?

[DominikPalo]

I would prefer to use a union type instead of an enum, because if a declare it as an enum type, e.g.:

export enum MrkdwnIn {
  Pretext: 'pretext',
  Text: 'text',
  Fields: 'fields'
}

MessageAttachment {
  //...
  mrkdwn_in?: MrkdwnIn[];
}

it won't be possible to define mesages in this "plain" way

const attachment: MessageAttachment = {
  mrkdwn_in: ['pretext']
}

and forces a user to use this style:

const attachment: MessageAttachment = {
  mrkdwn_in: ['MrkdownIn.Pretext']
}

so it won't be compatible with exports from tools like Walkiebot (and also with old projects, based on the older SDK).

But I can write it as a separate union type instead, to make it more readable:

export type MrkdwnIn = 'pretext' | 'text' | 'fields';

MessageAttachment {
  //...
  mrkdwn_in?: MrkdwnIn[];
}

what do you think?

[aoberoi]

😮 i guess i never really gave any thought to the intricate differences between a union type and an enum. i always thought that enums were a "more powerful" union type because TypeScript would generate a symbol with the actual map, but didn't realize the compiler would complain when you use the literal values like in your example. i tried it out here.

in light of this fact, i agree that the union type is a better solution. thanks!

[brianeletype]

Does anyone have any idea when the next release will go out that has this update in it?

Thanks

[aoberoi]

@brianeletype sorry for the wait, we'd like to release the next version ASAP too. right now i'm holding on the investigation of #531 since its been observed in the wild by a few developers and it seems pretty critical. if we think fixing that is going to take longer a day or two, then we'll release without fixing it, definitely before the end of the week.

[brianeletype]

Very timely and perfectly reasonable response. Thank you very much. I've put in a work around for the time being. Keep up the good work, @aoberoi