Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MM-387] Add a global welcome message field #131

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[MM-387] Add a global welcome message field #131

wants to merge 4 commits into from

Conversation

ayusht2810
Copy link
Contributor

@ayusht2810 ayusht2810 commented Apr 17, 2024 •
edited
Loading

Summary

  • Added a global welcome message field
  • Updated Readme with the addition of new field

Screenshot

image

Checklist

  • Completed dev testing
  • make test Ran test cases and ensured they are passing
  • make check-style Ran style check and ensured both webapp and server pass the checks

Ticket Link

Fixes #36

What to test?

  • Add a new field GlobalWelcomeMessage in the welcome bot config
  • Create a new user and log in the Mattermost server from the new user
  • Check for the global message present in the welcome bot DM
  • Some edge cases to test: Add only GlobalWelcomeMessage in the config, add both TeamName and GlobalWelcomeMessage in the config

@ayusht2810 ayusht2810 self-assigned this Apr 17, 2024
@ayusht2810 ayusht2810 added the 2: Dev Review Requires review by a core committer label Apr 17, 2024
@ayusht2810 ayusht2810 mentioned this pull request Apr 17, 2024
Closed
raghavaggarwal2308
raghavaggarwal2308 requested changes Apr 29, 2024
View reviewed changes
server/welcomebot.go Outdated Show resolved Hide resolved
server/hooks.go Outdated Show resolved Hide resolved
mickmister
mickmister reviewed Jul 4, 2024
View reviewed changes
Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Overall logic LGTM, just a few suggestions

server/message_template.go Outdated Show resolved Hide resolved
server/welcomebot.go Outdated Show resolved Hide resolved
server/welcomebot.go Outdated Show resolved Hide resolved
server/hooks.go Outdated Show resolved Hide resolved
server/hooks.go Outdated Show resolved Hide resolved
@mickmister mickmister added the 3: QA Review Requires review by a QA tester label Jul 4, 2024
@ayusht2810
Copy link
Contributor Author

@mickmister Fixed the review fixes. Please re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raghavaggarwal2308 raghavaggarwal2308 raghavaggarwal2308 approved these changes

@AayushChaudhary0001 AayushChaudhary0001 Awaiting requested review from AayushChaudhary0001

@mickmister mickmister Awaiting requested review from mickmister

@wiggin77 wiggin77 Awaiting requested review from wiggin77

Assignees

@ayusht2810 ayusht2810

Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

allow messages for *all* teams
3 participants
@ayusht2810 @mickmister @raghavaggarwal2308