mattermost / mattermost-developer-documentation

Mattermost developer documentation.
https://developers.mattermost.com
BSD 3-Clause "New" or "Revised" License
80 stars 389 forks source link

[GH-1309] Correct Attachments documentation #1324

Closed Omar8345 closed 7 months ago

Omar8345 commented 8 months ago

Summary

This pull request corrects misinformation in the attachments documentation, in the end of the documentation, it instructs the developer to add an attachments key in the props field, which won't work, I have edited the documentation and the curl and HTTPS examples to add the attachments key in the top-level of the post's JSON.

Ticket Link

Fixes https://github.com/mattermost/mattermost-developer-documentation/issues/1309

mattermost-build commented 8 months ago

Hello @Omar8345,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

cwarnermm commented 8 months ago

Thanks, @Omar8345, for this developer documentation PR. Your proposed changes will be technically reviewed by Engineering.

Omar8345 commented 7 months ago

Thanks for the PR, @Omar8345! I think there might be some confusion going on here because the syntax in those examples matches how POST /api/v4/posts expects the payload to look.

When you tried this, were you perhaps using an incoming webhook? I think the payload sent to a webhook uses the attachments field without props like in your changes.

If that's the case, I think we could instead add a separate example for posting message attachments using a webhook. If we do that, we should probably also update the existing examples to be posting to https://{your-mattermost-site}/api/v4/posts since that's the correct API endpoint for that

Thanks for your catch, I have made the corresponding changes requested and added a section for how to post a message attachment using a webhook.

Omar8345 commented 7 months ago

Actually, would you be able to apply those? I guess I don't have the required permission to do that here

Applied your suggested edits! I was a bit confused 😅

Edit: I just made a commit because I discovered that the curl and HTTP requests in the first FAQ wasn't putting attachments key inside props field

cwarnermm commented 7 months ago

@Omar8345 - Can you update your branch with latest master, please? Thanks!

Omar8345 commented 7 months ago

Done @cwarnermm

cwarnermm commented 7 months ago

@hmhealey - This PR is ready for your review.