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

Message hook #1840

Merged
merged 6 commits into from
Oct 5, 2018
Merged

Message hook #1840

merged 6 commits into from
Oct 5, 2018

Conversation

mkraft
Copy link
Contributor

@mkraft mkraft commented Oct 5, 2018

Summary

Adds message hook.

Ticket Link

n/a

Checklist

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)
  • Needs to be implemented in mobile (link to PR or User Story)
  • Has server changes (please link)
  • Has redux changes (please link)
  • Has UI changes
  • Includes text changes and localization file (.../i18n/en.json) updates
  • Touches critical sections of the codebase (auth, posting, etc.)

@mkraft mkraft added the 2: Dev Review Requires review by a core commiter label Oct 5, 2018

this.props.pluginHooks.forEach((o) => {
if (o && o.hook) {
message = o.hook(post, message);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept the message param in case hooks want to build upon changes made by other hooks. Of course any given hook could choose to totally ignore the changes made by other hooks and simply use the post.message field.

@@ -153,6 +153,28 @@ function components(state = {}, action) {
}
}

function hooks(state = {}, action) {
Copy link
Member

Choose a reason for hiding this comment

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

We actually don't need a new reducer for this, we've been using the components reducer for practically everything. e.g. see the FilesWillUploadHook

Copy link
Contributor

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

export default PostMarkdown;
function mapStateToProps(state) {
return {
pluginHooks: state.plugins.hooks.MessageWillFormat,
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to switch this to state.plugins.components.MessageWillFormat now as well

Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

LGTM

@jwilander jwilander added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Oct 5, 2018
@jwilander jwilander merged commit 18fce64 into master Oct 5, 2018
@jwilander jwilander deleted the message-hook branch October 5, 2018 21:07
@lindy65 lindy65 added Tests/Not Needed Does not require new release tests and removed 4: Reviews Complete All reviewers have approved the pull request labels Oct 11, 2018
fincha pushed a commit to fincha/mattermost-webapp that referenced this pull request Oct 21, 2018
* Start implementing plugin hook for MessageWillFormat

* Send entire post to webhook. (mattermost#1590)

* Adds plugin-updated message to allow hooks to chain their message changes.

* Updates snaps to reflect Redux connect use.

* Reuses existing 'components' reducer.

* Fix for wrong state key bug.
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
5 participants