Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-15324 Allow plugins override ephemeral posts #2759

Merged
merged 5 commits into from May 16, 2019
Merged

MM-15324 Allow plugins override ephemeral posts #2759

merged 5 commits into from May 16, 2019

Conversation

scottleedavis
Copy link
Contributor

@scottleedavis scottleedavis commented May 9, 2019

Summary

This PR enables ephemeral posts with a post.prop.type to override a post by a webapp plugin.

e.g. keeps the system_ephemeral post.type, but allows post.props.type to override

		p.API.SendEphemeralPost(args.UserId, &model.Post{
			UserId:    p.botId,
			ChannelId: args.ChannelId,
			Message:   "Bot ephemeral link",
			Props: model.StringInterface{
				"type": "system_ephemeral_test_plugin",
			},
		})

This has been validated by using /test-ephemeral-post-override from the test bot plugin

Ticket Link

Fixes mattermost/mattermost#10797

Notes

I did not add a test to post_message_view_test.jsx as I wasn't sure if it was needed and I am unfamiliar with snapshot testing. (happy to do it if requested).
I did not add a test for post_body.jsx as no tests currently exist for the component.

@hanzei hanzei added the 2: Dev Review Requires review by a core commiter label May 9, 2019
@hanzei
Copy link
Contributor

hanzei commented May 14, 2019

@cpoile Gently reminder about this PR. Would be great if we can get this in v5.12.

Copy link
Member

@cpoile cpoile left a comment

Choose a reason for hiding this comment

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

Thanks @scottleedavis, this is really nice.
Just making sure I understand what's happening here: you have to add your own props to an ephemeral post, and that's how it'll pick up that this is post that 'hasPlugin'?

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels May 15, 2019
@hanzei hanzei added this to the v5.12.0 milestone May 15, 2019
@scottleedavis
Copy link
Contributor Author

Thanks @cpoile ! That is correct. I chose this route over change the post type to have a prefix 'system_ephemeral_<custom_name>', as a lot of things would need to change, and I wanted to preserve the behavior with as little change as possible.

@hanzei hanzei assigned cpoile and unassigned cpoile May 15, 2019
@hanzei hanzei merged commit 6746f7d into mattermost:master May 16, 2019
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels May 17, 2019
@jasonblais
Copy link
Contributor

@hanzei @scottleedavis does this need any documentation at https://developers.mattermost.com/extend/plugins/?

@jasonblais jasonblais self-assigned this Jun 4, 2019
@scottleedavis
Copy link
Contributor Author

@jasonblais I think it would be a good idea. Otherwise it isn't straight forward to use props to solve the issue. I haven't done documentation updates yet, happy to do so if requested. :)

@jasonblais
Copy link
Contributor

@scottleedavis It would be great if you had time to help with the doc! :)

@aaronrothschild
Copy link
Contributor

Documented via: #3024 . Thanks Scott!

@aaronrothschild aaronrothschild added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Aug 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Done Required documentation has been written
Projects
None yet
7 participants