From 53ee077cfdff7ceb4366cdeac15492b4bae2082d Mon Sep 17 00:00:00 2001
From: Scott Bishel <scott.bishel@mattermost.com>
Date: Sun, 10 Nov 2019 08:07:49 -0700
Subject: [PATCH] MM-18005: Improve display message when attempting to add an
 outsider to a group-synced team via new team invite flow (#12966)

* Update group errors being returned

* update based on merge, error already added

* remove unnecessary translation

* Revert "remove unnecessary translation"

This reverts commit 5a4ae89618ddca5ea256009aab167f3e071f488c.

* remove unnecessary translation
---
 app/command_invite_test.go | 71 ++++++++++++++++++++++++++++++++++++++
 i18n/en.json               |  4 +--
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/app/command_invite_test.go b/app/command_invite_test.go
index 354b373b77..f79117b5f5 100644
--- a/app/command_invite_test.go
+++ b/app/command_invite_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/model"
 )
@@ -42,6 +43,15 @@ func TestInviteProvider(t *testing.T) {
 	userAndInvalidPrivate := "@" + basicUser3.Username + " ~" + privateChannel2.Name
 	deactivatedUserPublicChannel := "@" + deactivatedUser.Username + " ~" + channel.Name
 
+	groupChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE)
+	var err *model.AppError
+	_, err = th.App.AddChannelMember(th.BasicUser.Id, groupChannel, "", "")
+	require.Nil(t, err)
+	groupChannel.GroupConstrained = model.NewBool(true)
+	groupChannel, _ = th.App.UpdateChannel(groupChannel)
+
+	groupChannelNonUser := "@" + th.BasicUser2.Username + " ~" + groupChannel.Name
+
 	tests := []struct {
 		desc     string
 		expected string
@@ -97,6 +107,11 @@ func TestInviteProvider(t *testing.T) {
 			expected: "api.command_invite.user_not_in_team.app_error",
 			msg:      basicUser4.Username,
 		},
+		{
+			desc:     "try to add a user not part of the group to a group channel",
+			expected: "api.command_invite.group_constrained_user_denied",
+			msg:      groupChannelNonUser,
+		},
 		{
 			desc:     "try to add a user to a private channel with no permission",
 			expected: "api.command_invite.private_channel.app_error",
@@ -116,3 +131,59 @@ func TestInviteProvider(t *testing.T) {
 		})
 	}
 }
+
+func TestInviteGroup(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.BasicTeam.GroupConstrained = model.NewBool(true)
+	var err *model.AppError
+	_, _ = th.App.AddTeamMember(th.BasicTeam.Id, th.BasicUser.Id)
+	_, err = th.App.AddTeamMember(th.BasicTeam.Id, th.BasicUser2.Id)
+	require.Nil(t, err)
+	th.BasicTeam, _ = th.App.UpdateTeam(th.BasicTeam)
+
+	privateChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE)
+
+	groupChannelUser1 := "@" + th.BasicUser.Username + " ~" + privateChannel.Name
+	groupChannelUser2 := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name
+	basicUser3 := th.CreateUser()
+	groupChannelUser3 := "@" + basicUser3.Username + " ~" + privateChannel.Name
+
+	InviteP := InviteProvider{}
+	args := &model.CommandArgs{
+		T:         func(s string, args ...interface{}) string { return s },
+		ChannelId: th.BasicChannel.Id,
+		TeamId:    th.BasicTeam.Id,
+		Session:   model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}},
+	}
+
+	tests := []struct {
+		desc     string
+		expected string
+		msg      string
+	}{
+		{
+			desc:     "try to add an existing user part of the group to a group channel",
+			expected: "api.command_invite.user_already_in_channel.app_error",
+			msg:      groupChannelUser1,
+		},
+		{
+			desc:     "try to add a user part of the group to a group channel",
+			expected: "api.command_invite.success",
+			msg:      groupChannelUser2,
+		},
+		{
+			desc:     "try to add a user NOT part of the group to a group channel",
+			expected: "api.command_invite.user_not_in_team.app_error",
+			msg:      groupChannelUser3,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			actual := InviteP.DoCommand(th.App, args, test.msg).Text
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}
diff --git a/i18n/en.json b/i18n/en.json
index c20c213a7f..f565df6d60 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -764,7 +764,7 @@
   },
   {
     "id": "api.command_invite.group_constrained_user_denied",
-    "translation": "User cannot be added to this channel because it is constrained to group members only."
+    "translation": "This channel is managed by groups.  This user is not part of a group that is synched to this channel."
   },
   {
     "id": "api.command_invite.hint",
@@ -1864,7 +1864,7 @@
   },
   {
     "id": "api.team.add_members.user_denied",
-    "translation": "Team membership denied to the following users because of group constraints: {{ .UserIDs }}"
+    "translation": "This team is managed by groups.  This user is not part of a group that is synched to this team."
   },
   {
     "id": "api.team.add_user_to_team.added",