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-65] Allows users to add multiple teams to the messages using team-a,team-b in the messages #82

Merged
merged 8 commits into from
Aug 31, 2021
Merged

[mm-65] Allows users to add multiple teams to the messages using team-a,team-b in the messages #82

merged 8 commits into from
Aug 31, 2021

Conversation

maisnamrajusingh
Copy link
Contributor

Summary

I closed the previous PR that had a lot of conflicts and updated the logic here to make the code more simpler and shorter.
Users can now configure the messages using team-1, team-2 or team-1, team-2 and the bot will process the message for those teams when running /welcomebot preview team-1 or /welcomebot preview team-2

Ticket Link

#65

@mattermod
Copy link
Contributor

Hello @maisnamrajusingh,

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.

@maisnamrajusingh
fixes issue with the string.Contains catching unrelated teams and rep…
0ce939a 
…lacing it with a split and trimming trailing and previous white space
@jfrerich jfrerich requested a review from hanzei July 31, 2021 15:55
@jfrerich jfrerich added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Jul 31, 2021
jfrerich
jfrerich suggested changes Jul 31, 2021
View reviewed changes
Copy link
Contributor

@jfrerich jfrerich left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@hanzei, there is another ticket #36 that suggest the use of regex for team name matching. This would allow matching * so that all team names share the welcome message. Should we use that implementation suggestion in this PR and complete both GH issues in this PR?

I hadn't seen this PR yet #80

README.md Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
maisnamrajusingh and others added 2 commits August 3, 2021 16:35
@maisnamrajusingh @jfrerich
Update README.md
466e755 
Co-authored-by: Jason Frerich <jason.frerich@mattermost.com>
@maisnamrajusingh @jfrerich
Update server/command.go
fb21f94 
Co-authored-by: Jason Frerich <jason.frerich@mattermost.com>
hanzei
hanzei reviewed Aug 12, 2021
View reviewed changes
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Does this change effect the UserHasJoinedTeam hook?

@maisnamraju
Copy link
Contributor

Does this change effect the UserHasJoinedTeam hook?

No it doesn't.

hanzei
hanzei reviewed Aug 12, 2021
View reviewed changes
server/command.go Outdated Show resolved Hide resolved
@hanzei
Copy link
Contributor

hanzei commented Aug 12, 2021 •
edited
Loading

How do these changes ensure that users who join a team get the right message?

jfrerich
jfrerich suggested changes Aug 12, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

@hanzei hanzei added the Awaiting Submitter Action Blocked on the author label Aug 23, 2021
@maisnamrajusingh
Copy link
Contributor Author

maisnamrajusingh commented Aug 26, 2021 •
edited
Loading

How do these changes ensure that users who join a team get the right message?
I was so focussed on the message preview I totally forgot that I needed to get the message to show up when the user joins the team. 🤦‍♂️

I have updated the changes so that it hooks to the UserHasJoinedTeam hook

hanzei
hanzei approved these changes Aug 26, 2021
View reviewed changes
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hanzei hanzei requested a review from jfrerich August 26, 2021 11:47
jfrerich
jfrerich approved these changes Aug 31, 2021
View reviewed changes
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

LGTM!

@jfrerich jfrerich removed the 2: Dev Review Requires review by a core committer label Aug 31, 2021
@hanzei
Copy link
Contributor

hanzei commented Aug 31, 2021

Let's deffer QA testing to release testing.

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Aug 31, 2021
@hanzei hanzei added this to the v1.3.0 milestone Aug 31, 2021
@hanzei hanzei added the Docs/Done Required documentation has been written label Aug 31, 2021
@hanzei hanzei merged commit 03ea722 into mattermost-community:master Aug 31, 2021
@hanzei hanzei linked an issue Aug 31, 2021 that may be closed by this pull request
Allow same welcomebot message for a number of MM Teams on same server without repeating message text #65
Open
@mickmister mickmister mentioned this pull request Apr 28, 2023
Merged
mickmister added a commit that referenced this pull request May 4, 2023
@mickmister
Revert "[mm-65] Allows users to add multiple teams to the messages us…
e26e1e3 
…ing `team-a,team-b` in the messages (#82)"

This reverts commit 03ea722.
@mickmister mickmister mentioned this pull request May 4, 2023
Merged
mickmister added a commit that referenced this pull request May 10, 2023
@mickmister
Revert #82 and #79 (#109)
f982137 
* Revert "solves Gh 72 - Welcome bot will not let me add a message to private channels (#79)"

This reverts commit 517c73e.

* Revert "[mm-65] Allows users to add multiple teams to the messages using `team-a,team-b` in the messages (#82)"

This reverts commit 03ea722.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maisnamraju maisnamraju maisnamraju left review comments

@jfrerich jfrerich jfrerich approved these changes

@hanzei hanzei hanzei approved these changes

@DHaussermann DHaussermann Awaiting requested review from DHaussermann

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Docs/Done Required documentation has been written
Projects
None yet
Milestone
v1.3.0
Development

Successfully merging this pull request may close these issues.

Allow same welcomebot message for a number of MM Teams on same server without repeating message text
5 participants
@maisnamrajusingh @mattermod @maisnamraju @hanzei @jfrerich