From f2c955b3d3bdec2aa443811b005ec8c0c317c0b7 Mon Sep 17 00:00:00 2001
From: "Sorawis Nilparuk (Bo)" <turbo2547@hotmail.com>
Date: Fri, 3 Sep 2021 01:29:01 -0700
Subject: [PATCH] Gh 13908 fix bot missing username error (#17854)

Automatic Merge
---
 app/bot.go      |  5 +++++
 app/bot_test.go | 14 +++++++++++++-
 model/bot.go    | 20 ++++++++++++--------
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/app/bot.go b/app/bot.go
index e54476c2f3..99b53451ea 100644
--- a/app/bot.go
+++ b/app/bot.go
@@ -18,6 +18,11 @@ import (
 
 // CreateBot creates the given bot and corresponding user.
 func (a *App) CreateBot(c *request.Context, bot *model.Bot) (*model.Bot, *model.AppError) {
+	vErr := bot.IsValidCreate()
+	if vErr != nil {
+		return nil, vErr
+	}
+
 	user, nErr := a.Srv().Store.User().Save(model.UserFromBot(bot))
 	if nErr != nil {
 		var appErr *model.AppError
diff --git a/app/bot_test.go b/app/bot_test.go
index cd27ebf623..981376161e 100644
--- a/app/bot_test.go
+++ b/app/bot_test.go
@@ -26,7 +26,7 @@ func TestCreateBot(t *testing.T) {
 				OwnerId:     th.BasicUser.Id,
 			})
 			require.NotNil(t, err)
-			require.Equal(t, "model.user.is_valid.username.app_error", err.Id)
+			require.Equal(t, "model.bot.is_valid.username.app_error", err.Id)
 		})
 
 		t.Run("relative to bot", func(t *testing.T) {
@@ -55,6 +55,18 @@ func TestCreateBot(t *testing.T) {
 			require.Nil(t, bot)
 			require.Equal(t, "model.user.is_valid.email.app_error", err.Id)
 		})
+
+		t.Run("username missing", func(t *testing.T) {
+			th := Setup(t).InitBasic()
+			defer th.TearDown()
+			bot, err := th.App.CreateBot(th.Context, &model.Bot{
+				Description: "a bot",
+				OwnerId:     th.BasicUser.Id,
+			})
+			require.NotNil(t, err)
+			require.Nil(t, bot)
+			require.Equal(t, "model.bot.is_valid.username.app_error", err.Id)
+		})
 	})
 
 	t.Run("create bot", func(t *testing.T) {
diff --git a/model/bot.go b/model/bot.go
index fe9b907870..7b58108993 100644
--- a/model/bot.go
+++ b/model/bot.go
@@ -63,12 +63,8 @@ func (b *Bot) Clone() *Bot {
 	return &copy
 }
 
-// IsValid validates the bot and returns an error if it isn't configured correctly.
-func (b *Bot) IsValid() *AppError {
-	if !IsValidId(b.UserId) {
-		return NewAppError("Bot.IsValid", "model.bot.is_valid.user_id.app_error", b.Trace(), "", http.StatusBadRequest)
-	}
-
+// IsValidCreate validates bot for Create call. This skips validations of fields that are auto-filled on Create
+func (b *Bot) IsValidCreate() *AppError {
 	if !IsValidUsername(b.Username) {
 		return NewAppError("Bot.IsValid", "model.bot.is_valid.username.app_error", b.Trace(), "", http.StatusBadRequest)
 	}
@@ -85,6 +81,15 @@ func (b *Bot) IsValid() *AppError {
 		return NewAppError("Bot.IsValid", "model.bot.is_valid.creator_id.app_error", b.Trace(), "", http.StatusBadRequest)
 	}
 
+	return nil
+}
+
+// IsValid validates the bot and returns an error if it isn't configured correctly.
+func (b *Bot) IsValid() *AppError {
+	if !IsValidId(b.UserId) {
+		return NewAppError("Bot.IsValid", "model.bot.is_valid.user_id.app_error", b.Trace(), "", http.StatusBadRequest)
+	}
+
 	if b.CreateAt == 0 {
 		return NewAppError("Bot.IsValid", "model.bot.is_valid.create_at.app_error", b.Trace(), "", http.StatusBadRequest)
 	}
@@ -92,8 +97,7 @@ func (b *Bot) IsValid() *AppError {
 	if b.UpdateAt == 0 {
 		return NewAppError("Bot.IsValid", "model.bot.is_valid.update_at.app_error", b.Trace(), "", http.StatusBadRequest)
 	}
-
-	return nil
+	return b.IsValidCreate()
 }
 
 // PreSave should be run before saving a new bot to the database.