From 9e6da03ab1a7df9327af469b14549e2285085bcc Mon Sep 17 00:00:00 2001
From: pacop <1923775+pacop@users.noreply.github.com>
Date: Mon, 22 Apr 2024 09:40:08 +0200
Subject: [PATCH] MM-57391: improve error message (#26630)

The error reporte when moving channels and failing has been improved to show
that the problem was the repeated name on the team.
The error message has been unified with MM-53756

Co-authored-by: Mattermost Build <build@mattermost.com>
---
 server/channels/api4/channel_test.go          | 23 +++++++++++++++++++
 server/channels/app/channel.go                |  9 +++++---
 .../channels/store/sqlstore/channel_store.go  |  2 +-
 .../channels/store/storetest/channel_store.go |  7 +++---
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go
index 1d4c1f515e..64140e2466 100644
--- a/server/channels/api4/channel_test.go
+++ b/server/channels/api4/channel_test.go
@@ -4801,6 +4801,29 @@ func TestMoveChannel(t *testing.T) {
 		require.Equal(t, team2.Id, ch.TeamId)
 	})
 
+	t.Run("Should return custom error with repeated channel", func(t *testing.T) {
+		channelT1 := &model.Channel{
+			DisplayName: "repeated",
+			Name:        "repeated",
+			Type:        model.ChannelTypePrivate,
+			TeamId:      team1.Id,
+		}
+		channelT1, _, err := th.Client.CreateChannel(context.TODO(), channelT1)
+		require.NoError(t, err)
+
+		channelT2 := &model.Channel{
+			DisplayName: "repeated",
+			Name:        "repeated",
+			Type:        model.ChannelTypePrivate,
+			TeamId:      team2.Id,
+		}
+		_, _, err = th.Client.CreateChannel(context.TODO(), channelT2)
+		require.NoError(t, err)
+
+		_, _, err = th.SystemAdminClient.MoveChannel(context.Background(), channelT1.Id, team2.Id, false)
+		require.EqualError(t, err, "A channel with that name already exists on the same team.")
+	})
+
 	t.Run("Should move private channel", func(t *testing.T) {
 		channel := th.CreatePrivateChannel()
 		ch, _, err := th.SystemAdminClient.MoveChannel(context.Background(), channel.Id, team1.Id, false)
diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go
index f5f2d5331e..45986a12a5 100644
--- a/server/channels/app/channel.go
+++ b/server/channels/app/channel.go
@@ -622,12 +622,12 @@ func (a *App) UpdateChannel(c request.CTX, channel *model.Channel) (*model.Chann
 	_, err := a.Srv().Store().Channel().Update(c, channel)
 	if err != nil {
 		var appErr *model.AppError
+		var uniqueConstraintErr *store.ErrUniqueConstraint
 		var invErr *store.ErrInvalidInput
 		switch {
+		case errors.As(err, &uniqueConstraintErr):
+			return nil, model.NewAppError("UpdateChannel", store.ChannelExistsError, nil, "", http.StatusBadRequest).Wrap(err)
 		case errors.As(err, &invErr):
-			if invErr.Entity == "Channel" && invErr.Field == "Name" {
-				return nil, model.NewAppError("UpdateChannel", store.ChannelExistsError, nil, "", http.StatusBadRequest).Wrap(err)
-			}
 			return nil, model.NewAppError("UpdateChannel", "app.channel.update.bad_id", nil, "", http.StatusBadRequest).Wrap(err)
 		case errors.As(err, &appErr):
 			return nil, appErr
@@ -3149,10 +3149,13 @@ func (a *App) MoveChannel(c request.CTX, team *model.Team, channel *model.Channe
 	channel.TeamId = team.Id
 	if _, err := a.Srv().Store().Channel().Update(c, channel); err != nil {
 		var appErr *model.AppError
+		var uniqueConstraintErr *store.ErrUniqueConstraint
 		var invErr *store.ErrInvalidInput
 		switch {
 		case errors.As(err, &invErr):
 			return model.NewAppError("MoveChannel", "app.channel.update.bad_id", nil, "", http.StatusBadRequest).Wrap(err)
+		case errors.As(err, &uniqueConstraintErr):
+			return model.NewAppError("MoveChannel", store.ChannelExistsError, nil, "", http.StatusBadRequest).Wrap(err)
 		case errors.As(err, &appErr):
 			return appErr
 		default:
diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go
index a845a1ca43..b0957f39cf 100644
--- a/server/channels/store/sqlstore/channel_store.go
+++ b/server/channels/store/sqlstore/channel_store.go
@@ -745,7 +745,7 @@ func (s SqlChannelStore) updateChannelT(transaction *sqlxTxWrapper, channel *mod
 		WHERE Id=:Id`, channel)
 	if err != nil {
 		if IsUniqueConstraintError(err, []string{"Name", "channels_name_teamid_key"}) {
-			return nil, store.NewErrInvalidInput("Channel", "Name", channel.Name)
+			return nil, store.NewErrUniqueConstraint("Name")
 		}
 		return nil, errors.Wrapf(err, "failed to update channel with id=%s", channel.Id)
 	}
diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go
index 4bb0a75f60..60a2cbc2c3 100644
--- a/server/channels/store/storetest/channel_store.go
+++ b/server/channels/store/storetest/channel_store.go
@@ -364,10 +364,9 @@ func testChannelStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) {
 
 	// Make sure that the error correctly reports the wrong field to be Name
 	// See https://mattermost.atlassian.net/browse/MM-53756
-	var invalidInputErr *store.ErrInvalidInput
-	require.ErrorAs(t, err, &invalidInputErr)
-	require.Equal(t, invalidInputErr.Entity, "Channel")
-	require.Equal(t, invalidInputErr.Field, "Name")
+	var uniqueConstraintErr *store.ErrUniqueConstraint
+	require.ErrorAs(t, err, &uniqueConstraintErr)
+	require.Contains(t, uniqueConstraintErr.Columns, "Name")
 }
 
 func testGetChannelUnread(t *testing.T, rctx request.CTX, ss store.Store) {