[GH-24] Implement channel welcome messages #31

vespian · 2020-02-25T20:02:56Z

Summary

This is a rough draft of the channel welcome message as requested in #24

Three new commands were added (all of them relate to the channel in which they were executed):

set_channel_welcome - sets the given text as channel welcome message
get_channel_welcome - gets the channel welcome message
delete_channel_welcome - deletes the channel welcome message

The sample workflow looks like this:

Channel welcome as seen by the user joining the channel:

Channel welcome as seen by other users in the channel:

For some reason sending ephemeral messages does not work, the mattermost-plugin-demo seems to be using plain Post messages as well [1].

I would love to hear feedback, whether this is something you had in mind. Personally I am thinking about:
a) mlog fields are not used properly throughout the plugin, Sprintf should not be used
b) the command handling function is growing into God-mode function, we should consider rewriting it, taking [2] as example
c) should we be concerned about the load of the KV store? Should we cache data locally?

I am thinking about making a) and b) a separate issue and addressing it once this PR and #30 lands. I would appreciate some feedback and information regarding c).

Thanks in advance!

[1] https://github.com/mattermost/mattermost-plugin-demo/blob/c29a9050a7cd0b1f40b761d256f1dfb6dd9fcc0a/server/channel_hooks.go#L54
[2] https://github.com/mattermost/mattermost-plugin-demo/blob/master/server/command_hooks.go

Ticket Link

Fixes #24

aaronrothschild · 2020-02-28T15:12:49Z

@vespian Thanks for your work on this!

Interesting that epehemeral messages don't work... that would be ideal IMO, as the messages would get repetitive to everyone :) @hanzei What could be causing that?
I would suggest that your confirmation messages return a bit of information with them, for example "Welcome message: "#Jungle Welcomes You!" has been stored" tells the user what the system has actually stored.

Looks good, thanks again!

vespian · 2020-02-29T20:41:17Z

I would suggest that your confirmation messages return a bit of information with them, for example "Welcome message: "#Jungle Welcomes You!" has been stored" tells the user what the system has actually stored.

Done, see commit Implement channel welcome messages - review fixes #1

hanzei

Nice work 👍 I leaved a few comment on the code style. The rest looks good.

server/command.go

server/hooks.go

hanzei · 2020-03-05T12:03:10Z

@vespian Would you be interesting on also updating the documentation in README.md?

jfrerich

Looking really good! I have some additional comments and suggestions should help implement some DRY code :) Please request a new review after you've made a round of changes! Thanks for the PR!

server/command.go

vespian · 2020-03-06T20:40:08Z

@vespian Would you be interesting on also updating the documentation in README.md?

@hanzei I am going to cherry-pick the documentation I wrote for other commands[1] and add docs for this PR on top of it. If this PR gets merged first, GH should auto-close the original PR, if not - I will simply rebase.

[1] #30

vespian · 2020-03-06T21:48:22Z

@jfrerich @hanzei Ready for another pass.

jfrerich

@vespian! Thanks for super prompt changes! I have a very small nit and then I'm approving :)

server/command.go

vespian · 2020-03-07T09:26:16Z

@jfrerich Done, ready for another pass.

hanzei

See #31 (comment)

levb · 2020-03-31T21:47:56Z

@vespian do you mind resolving the conflicts please, apologies for taking awhile to QA/merge.

DHaussermann · 2020-04-01T21:18:42Z

Thanks @vespian for this enhancement!

Tested upgrade path from previous release
Tested slash command for setting, viewing and deleting
Confirmed post correctly made when they are set
Confirmed delete works and posts are no longer made once they're removed.

I have a couple questions on this...

Ephemeral posts made by Welcome bot as feedback that the config has changed will show the Welcome Bot's user ID instead of name or display name as well as the profile image of the user who ran the command instaed of the bot profile image. I don't believe there is a separate issue to address this.
@aaronrothschild Where did we end up on the permissions issue? Currently with enterprise, user's who's roles do not permit them to manage channels can still run these commands. Should I create an issue to address this?

DHaussermann · 2020-04-09T15:44:24Z

@vespian thanks for the work on this! Are you able to resolve the issue above and make the post show the bots username and profile image or, would his require a separate PR.

vespian · 2020-04-09T16:02:51Z

@DHaussermann I was about to write a comment. Currently, I am:
a) waiting for @hanzei and @jfrerich to decide on #31 (comment).
b) there is also no decision wrt. for #31 (comment)
c) I need to do the rebase as pointed in #31 (comment)
d) I need to fix the key ordering as pointed in #31 (comment)
e) I need to address the issue outlined by you in #31 (comment)

I will be happy to do one more pass and fix/look into issues c), d), e). If we decide on a), I will also take care of reverting my earlier changes as well and make the code follow the consensus.

I am short on time so I am afraid that if there is going to be more changes above that, somebody from MM will have to take over or we can simply close the PR right now.

jfrerich · 2020-04-09T18:43:21Z

@vespian Thanks for the hard work. Here's the order of items, in order of importance, to get this merged.

If you are short on time, I've got no problem merging without all the fixes and I'll make the changes soon after!

At a minimum, if you could fix 1 and 2, that would be enough to merge. It would be great to see the others, but I'll leave that for you to decide. Thanks again for the hard work!

I need to do the rebase as pointed in [GH-24] Implement channel welcome messages #31 (comment)
1. Must happen before merge
I need to fix the key ordering as pointed in [GH-24] Implement channel welcome messages #31 (comment)
1. simple fix to reverse key order
waiting for @hanzei and @jfrerich to decide on [GH-24] Implement channel welcome messages #31 (comment).
1. Please do the revert as suggested by Hanzei
there is also no decision wrt. for [GH-24] Implement channel welcome messages #31 (comment)
1. Let's have the Bot send the DM
2. Also use @levb's suggestion for attempt to send ephemeral message [GH-24] Implement channel welcome messages #31 (comment)
I need to address the issue outlined by you in [GH-24] Implement channel welcome messages #31 (comment)
1. Use Bot to post feedback

jfrerich · 2020-04-09T18:45:52Z

2. @aaronrothschild Where did we end up on the permissions issue? Currently with enterprise, user's who's roles do not permit them to manage channels can still run these commands. Should I create an issue to address this?

@aaronrothschild, @DHaussermann, if there is an actionable item in for this, can you file an issue in the GitHub repo?

DHaussermann · 2020-04-09T19:43:36Z

@jfrerich I have created #41 to adress 2. above.

vespian · 2020-04-12T11:33:16Z

I need to do the rebase as pointed in #31 (comment)
   Must happen before merge

Done

I need to fix the key ordering as pointed in #31 (comment)
simple fix to reverse key order

Done

waiting for @hanzei and @jfrerich to decide on #31 (comment).
Please do the revert as suggested by Hanzei

Done

there is also no decision wrt. for #31 (comment)
Let's have the Bot send the DM
Also use @levb's suggestion for attempt to send ephemeral message #31 (comment)

Done

I need to address the issue outlined by you in #31 (comment)
Use Bot to post feedback

Once we fixed/reverted using return commandResponse{}, this seems no longer be the issue.

vespian · 2020-04-12T11:35:59Z

@hanzei @jfrerich @DHaussermann All done, please have another look.

New changes are in commits 0ae8880 e29d8fb d8914a9

hanzei

LGTM

aaronrothschild

LGTM, this is great @vespian

jfrerich · 2020-04-13T15:27:35Z

Fantastic work, @vespian! Thanks for the latest round of commits. 🚀

DHaussermann

Tested and passed.

Issue with bot ID and profile when settings are changes is resolved.
Welcome bot posts are now ephemeral so they are visible by other users 🎉
Brief round of regression testing.
LGTM!
Thanks @vespian this last round of improvements is great!

DHaussermann · 2020-04-14T20:31:23Z

@jfrerich or @hanzei - I don't have permissions to add/remove labels in this repo.

QA Review Done
Reviews Complete

vespian mentioned this pull request Feb 25, 2020

When a user joins a channel, present them with a Welcome message #24

Closed

vespian force-pushed the prozlach/GH-24_channel_welcome branch 3 times, most recently from 27e32d5 to 26c1d97 Compare February 25, 2020 20:21

hanzei requested review from jfrerich and hanzei February 26, 2020 21:21

hanzei added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Feb 26, 2020

hanzei requested a review from aaronrothschild February 26, 2020 21:22

aaronrothschild removed the 1: PM Review Requires review by a product manager label Feb 28, 2020

aaronrothschild approved these changes Feb 28, 2020

View reviewed changes

vespian force-pushed the prozlach/GH-24_channel_welcome branch from 131cb65 to 3e33d03 Compare February 29, 2020 20:42

vespian changed the title ~~Implement channel welcome messages~~ [GH-24] Implement channel welcome messages Mar 1, 2020

hanzei requested changes Mar 5, 2020

View reviewed changes

hanzei added the Docs/Needed Requires documentation label Mar 5, 2020

jfrerich suggested changes Mar 5, 2020

View reviewed changes

server/command.go Show resolved Hide resolved

server/command.go Show resolved Hide resolved

vespian force-pushed the prozlach/GH-24_channel_welcome branch 4 times, most recently from 4c6f77f to 5db6350 Compare March 6, 2020 20:49

jfrerich self-requested a review March 6, 2020 23:06

jfrerich suggested changes Mar 6, 2020

View reviewed changes

server/command.go Outdated Show resolved Hide resolved

server/command.go Outdated Show resolved Hide resolved

server/command.go Outdated Show resolved Hide resolved

server/command.go Outdated Show resolved Hide resolved

server/command.go Outdated Show resolved Hide resolved

vespian force-pushed the prozlach/GH-24_channel_welcome branch from 4f4cf36 to 6e15dad Compare March 7, 2020 09:25

hanzei removed the Docs/Needed Requires documentation label Mar 7, 2020

hanzei requested changes Mar 31, 2020

View reviewed changes

DHaussermann mentioned this pull request Apr 9, 2020

Restrict Channel specific Welcome Bot message config to users who have permission to manage channels #41

Closed

Pawel Rozlach and others added 3 commits April 12, 2020 10:24

Implement channel welcome messages

d65437d

DRY up the command.go code by moving commands to separate functions

a33b3c0

Review fixes mattermost-community#1

c6e6f60

vespian force-pushed the prozlach/GH-24_channel_welcome branch from 8e7b6ec to c6e6f60 Compare April 12, 2020 08:26

vespian requested a review from iomodo as a code owner April 12, 2020 08:26

vespian added 3 commits April 12, 2020 10:31

Change keys prefix ordering

0ae8880

Switch back to sending ephemeral posts

e29d8fb

Send an opportunistic ephemeral post and a DM

d8914a9

vespian force-pushed the prozlach/GH-24_channel_welcome branch from d941432 to d8914a9 Compare April 12, 2020 10:19

hanzei approved these changes Apr 13, 2020

View reviewed changes

hanzei removed the request for review from iomodo April 13, 2020 11:14

aaronrothschild approved these changes Apr 13, 2020

View reviewed changes

aaronrothschild removed the 1: PM Review Requires review by a product manager label Apr 13, 2020

DHaussermann approved these changes Apr 14, 2020

View reviewed changes

levb added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Apr 15, 2020

levb merged commit b24433b into mattermost-community:master Apr 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GH-24] Implement channel welcome messages #31

[GH-24] Implement channel welcome messages #31

vespian commented Feb 25, 2020 •

edited

Loading

aaronrothschild commented Feb 28, 2020

vespian commented Feb 29, 2020

hanzei left a comment

hanzei commented Mar 5, 2020

jfrerich left a comment

vespian commented Mar 6, 2020

vespian commented Mar 6, 2020

jfrerich left a comment

vespian commented Mar 7, 2020

hanzei left a comment

levb commented Mar 31, 2020

DHaussermann commented Apr 1, 2020

DHaussermann commented Apr 9, 2020

vespian commented Apr 9, 2020

jfrerich commented Apr 9, 2020 •

edited

Loading

jfrerich commented Apr 9, 2020

DHaussermann commented Apr 9, 2020

vespian commented Apr 12, 2020 •

edited

Loading

vespian commented Apr 12, 2020 •

edited

Loading

hanzei left a comment

aaronrothschild left a comment

jfrerich commented Apr 13, 2020

DHaussermann left a comment

DHaussermann commented Apr 14, 2020

[GH-24] Implement channel welcome messages #31

[GH-24] Implement channel welcome messages #31

Conversation

vespian commented Feb 25, 2020 • edited Loading

Summary

Ticket Link

aaronrothschild commented Feb 28, 2020

vespian commented Feb 29, 2020

hanzei left a comment

Choose a reason for hiding this comment

hanzei commented Mar 5, 2020

jfrerich left a comment

Choose a reason for hiding this comment

vespian commented Mar 6, 2020

vespian commented Mar 6, 2020

jfrerich left a comment

Choose a reason for hiding this comment

vespian commented Mar 7, 2020

hanzei left a comment

Choose a reason for hiding this comment

levb commented Mar 31, 2020

DHaussermann commented Apr 1, 2020

DHaussermann commented Apr 9, 2020

vespian commented Apr 9, 2020

jfrerich commented Apr 9, 2020 • edited Loading

jfrerich commented Apr 9, 2020

DHaussermann commented Apr 9, 2020

vespian commented Apr 12, 2020 • edited Loading

vespian commented Apr 12, 2020 • edited Loading

hanzei left a comment

Choose a reason for hiding this comment

aaronrothschild left a comment

Choose a reason for hiding this comment

jfrerich commented Apr 13, 2020

DHaussermann left a comment

Choose a reason for hiding this comment

DHaussermann commented Apr 14, 2020

vespian commented Feb 25, 2020 •

edited

Loading

jfrerich commented Apr 9, 2020 •

edited

Loading

vespian commented Apr 12, 2020 •

edited

Loading

vespian commented Apr 12, 2020 •

edited

Loading