[MM-17422] Added code to update, delete last bot_icon time in model.bot. (#12229)

Display LHS bot Icon in web app. As part of mentioned task, Added LastIconUpdate variable in model.bot to store last update time of icon. Also added code to update/delete value of the mentioned variable when setting/deleting bot icon.

app/bot.go

@@ -328,7 +328,8 @@ func (a *App) SetBotIconImageFromMultiPartFile(botUserId string, imageData *mult
 
 // SetBotIconImage sets LHS icon for a bot.
 func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppError {
-	if _, err := a.GetBot(botUserId, true); err != nil {
+	bot, err := a.GetBot(botUserId, true)
+	if err != nil {
 		return err
 	}
 
@@ -338,12 +339,13 @@ func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppEr
 
 	// Set icon
 	file.Seek(0, 0)
-	if _, err := a.WriteFile(file, getBotIconPath(botUserId)); err != nil {
+	if _, err = a.WriteFile(file, getBotIconPath(botUserId)); err != nil {
 		return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
-	if err := a.Srv.Store.User().UpdateLastPictureUpdate(botUserId); err != nil {
+	bot.LastIconUpdate = model.GetMillis()
-		mlog.Error(err.Error())
+	if _, err = a.Srv.Store.Bot().Update(bot); err != nil {
+		return err
 	}
 	a.invalidateUserCacheAndPublish(botUserId)
 
@@ -352,18 +354,25 @@ func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppEr
 
 // DeleteBotIconImage deletes LHS icon for a bot.
 func (a *App) DeleteBotIconImage(botUserId string) *model.AppError {
-	if _, err := a.GetBot(botUserId, true); err != nil {
+	bot, err := a.GetBot(botUserId, true)
+	if err != nil {
 		return err
 	}
 
 	// Delete icon
-	if err := a.RemoveFile(getBotIconPath(botUserId)); err != nil {
+	if err = a.RemoveFile(getBotIconPath(botUserId)); err != nil {
 		return model.NewAppError("DeleteBotIconImage", "api.bot.delete_bot_icon_image.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
-	if err := a.Srv.Store.User().UpdateLastPictureUpdate(botUserId); err != nil {
+	if err = a.Srv.Store.User().UpdateLastPictureUpdate(botUserId); err != nil {
 		mlog.Error(err.Error())
 	}
+
+	bot.LastIconUpdate = int64(0)
+	if _, err = a.Srv.Store.Bot().Update(bot); err != nil {
+		return err
+	}
+
 	a.invalidateUserCacheAndPublish(botUserId)
 
 	return nil

model/bot.go

@@ -27,6 +27,7 @@ type Bot struct {
 	DisplayName    string `json:"display_name,omitempty"`
 	Description    string `json:"description,omitempty"`
 	OwnerId        string `json:"owner_id"`
+	LastIconUpdate int64  `json:"last_icon_update,omitempty"`
 	CreateAt       int64  `json:"create_at"`
 	UpdateAt       int64  `json:"update_at"`
 	DeleteAt       int64  `json:"delete_at"`

model/bot_test.go

@@ -19,9 +19,10 @@ func TestBotTrace(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
-		UpdateAt:    2,
+		CreateAt:       2,
-		DeleteAt:    3,
+		UpdateAt:       3,
+		DeleteAt:       4,
 	}
 
 	require.Equal(t, map[string]interface{}{"user_id": bot.UserId}, bot.Trace())
@@ -34,9 +35,10 @@ func TestBotClone(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
-		UpdateAt:    2,
+		CreateAt:       2,
-		DeleteAt:    3,
+		UpdateAt:       3,
+		DeleteAt:       4,
 	}
 
 	clone := bot.Clone()
@@ -64,9 +66,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -78,9 +81,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -92,9 +96,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -106,9 +111,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -120,9 +126,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    strings.Repeat("x", 1025),
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -134,9 +141,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        "",
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -148,9 +156,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
+				LastIconUpdate: 1,
 				CreateAt:       0,
-				UpdateAt:    2,
+				UpdateAt:       3,
-				DeleteAt:    3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -162,9 +171,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
+				CreateAt:       2,
 				UpdateAt:       0,
-				DeleteAt:    3,
+				DeleteAt:       4,
 			},
 			false,
 		},
@@ -176,8 +186,9 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
+				UpdateAt:       3,
 				DeleteAt:       0,
 			},
 			true,
@@ -190,8 +201,9 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
+				UpdateAt:       3,
 				DeleteAt:       0,
 			},
 			true,
@@ -204,9 +216,10 @@ func TestBotIsValid(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "a description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			true,
 		},
@@ -230,6 +243,7 @@ func TestBotPreSave(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
+		LastIconUpdate: 0,
 		DeleteAt:       0,
 	}
 
@@ -251,7 +265,8 @@ func TestBotPreUpdate(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
+		CreateAt:       2,
 		DeleteAt:       0,
 	}
 
@@ -272,9 +287,10 @@ func TestBotEtag(t *testing.T) {
 			DisplayName:    "display name",
 			Description:    "description",
 			OwnerId:        NewId(),
-			CreateAt:    1,
+			LastIconUpdate: 1,
-			UpdateAt:    2,
+			CreateAt:       2,
-			DeleteAt:    3,
+			UpdateAt:       3,
+			DeleteAt:       4,
 		}
 		bot2 := bot1
 
@@ -288,9 +304,10 @@ func TestBotEtag(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			}
 			bot2 := &Bot{
 				UserId:         NewId(),
@@ -298,9 +315,10 @@ func TestBotEtag(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        bot1.OwnerId,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			}
 
 			assert.NotEqual(t, bot1.Etag(), bot2.Etag())
@@ -312,9 +330,10 @@ func TestBotEtag(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        NewId(),
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			}
 			bot2 := &Bot{
 				UserId:         bot1.UserId,
@@ -322,9 +341,10 @@ func TestBotEtag(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        bot1.OwnerId,
-				CreateAt:    1,
+				LastIconUpdate: 1,
+				CreateAt:       2,
 				UpdateAt:       10,
-				DeleteAt:    3,
+				DeleteAt:       4,
 			}
 
 			assert.NotEqual(t, bot1.Etag(), bot2.Etag())
@@ -339,9 +359,10 @@ func TestBotToAndFromJson(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
-		UpdateAt:    2,
+		CreateAt:       2,
-		DeleteAt:    3,
+		UpdateAt:       3,
+		DeleteAt:       4,
 	}
 
 	bot2 := &Bot{
@@ -350,9 +371,10 @@ func TestBotToAndFromJson(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description 2",
 		OwnerId:        NewId(),
-		CreateAt:    4,
+		LastIconUpdate: 5,
-		UpdateAt:    5,
+		CreateAt:       6,
-		DeleteAt:    6,
+		UpdateAt:       7,
+		DeleteAt:       8,
 	}
 
 	assert.Equal(t, bot1, BotFromJson(bytes.NewReader(bot1.ToJson())))
@@ -381,9 +403,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			&BotPatch{},
 			&Bot{
@@ -392,9 +415,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 		},
 		{
@@ -405,9 +429,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			&BotPatch{
 				Username:    sToP("new_username"),
@@ -420,9 +445,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "new description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 		},
 		{
@@ -433,9 +459,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "display name",
 				Description:    "description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 			&BotPatch{
 				Username:    sToP("new_username"),
@@ -448,9 +475,10 @@ func TestBotPatch(t *testing.T) {
 				DisplayName:    "new display name",
 				Description:    "new description",
 				OwnerId:        creatorId1,
-				CreateAt:    1,
+				LastIconUpdate: 1,
-				UpdateAt:    2,
+				CreateAt:       2,
-				DeleteAt:    3,
+				UpdateAt:       3,
+				DeleteAt:       4,
 			},
 		},
 	}
@@ -487,9 +515,10 @@ func TestUserFromBot(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
-		UpdateAt:    2,
+		CreateAt:       2,
-		DeleteAt:    3,
+		UpdateAt:       3,
+		DeleteAt:       4,
 	}
 
 	bot2 := &Bot{
@@ -498,9 +527,10 @@ func TestUserFromBot(t *testing.T) {
 		DisplayName:    "display name 2",
 		Description:    "description 2",
 		OwnerId:        NewId(),
-		CreateAt:    4,
+		LastIconUpdate: 5,
-		UpdateAt:    5,
+		CreateAt:       6,
-		DeleteAt:    6,
+		UpdateAt:       7,
+		DeleteAt:       8,
 	}
 
 	assert.Equal(t, &User{
@@ -554,9 +584,10 @@ func TestBotListToAndFromJson(t *testing.T) {
 					DisplayName:    "display name",
 					Description:    "description",
 					OwnerId:        NewId(),
-					CreateAt:    1,
+					LastIconUpdate: 1,
-					UpdateAt:    2,
+					CreateAt:       2,
-					DeleteAt:    3,
+					UpdateAt:       3,
+					DeleteAt:       4,
 				},
 			},
 		},
@@ -569,9 +600,10 @@ func TestBotListToAndFromJson(t *testing.T) {
 					DisplayName:    "display name",
 					Description:    "description",
 					OwnerId:        NewId(),
-					CreateAt:    1,
+					LastIconUpdate: 1,
-					UpdateAt:    2,
+					CreateAt:       2,
-					DeleteAt:    3,
+					UpdateAt:       3,
+					DeleteAt:       4,
 				},
 
 				&Bot{
@@ -580,9 +612,10 @@ func TestBotListToAndFromJson(t *testing.T) {
 					DisplayName:    "display name",
 					Description:    "description 2",
 					OwnerId:        NewId(),
-					CreateAt:    4,
+					LastIconUpdate: 5,
-					UpdateAt:    5,
+					CreateAt:       6,
-					DeleteAt:    6,
+					UpdateAt:       7,
+					DeleteAt:       8,
 				},
 			},
 		},
@@ -602,9 +635,10 @@ func TestBotListEtag(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
-		UpdateAt:    2,
+		CreateAt:       2,
-		DeleteAt:    3,
+		UpdateAt:       3,
+		DeleteAt:       4,
 	}
 
 	bot1Updated := &Bot{
@@ -613,9 +647,10 @@ func TestBotListEtag(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    1,
+		LastIconUpdate: 1,
+		CreateAt:       2,
 		UpdateAt:       10,
-		DeleteAt:    3,
+		DeleteAt:       4,
 	}
 
 	bot2 := &Bot{
@@ -624,9 +659,10 @@ func TestBotListEtag(t *testing.T) {
 		DisplayName:    "display name",
 		Description:    "description",
 		OwnerId:        NewId(),
-		CreateAt:    4,
+		LastIconUpdate: 5,
-		UpdateAt:    5,
+		CreateAt:       6,
-		DeleteAt:    6,
+		UpdateAt:       7,
+		DeleteAt:       8,
 	}
 
 	testCases := []struct {

model/user.go

@@ -87,6 +87,7 @@ type User struct {
 	LastActivityAt         int64     `db:"-" json:"last_activity_at,omitempty"`
 	IsBot                  bool      `db:"-" json:"is_bot,omitempty"`
 	BotDescription         string    `db:"-" json:"bot_description,omitempty"`
+	BotLastIconUpdate      int64     `db:"-" json:"bot_last_icon_update,omitempty"`
 	TermsOfServiceId       string    `db:"-" json:"terms_of_service_id,omitempty"`
 	TermsOfServiceCreateAt int64     `db:"-" json:"terms_of_service_create_at,omitempty"`
 }
@@ -487,7 +488,7 @@ func (u *UserAuth) ToJson() string {
 
 // Generate a valid strong etag so the browser can cache the results
 func (u *User) Etag(showFullName, showEmail bool) string {
-	return Etag(u.Id, u.UpdateAt, u.TermsOfServiceId, u.TermsOfServiceCreateAt, showFullName, showEmail)
+	return Etag(u.Id, u.UpdateAt, u.TermsOfServiceId, u.TermsOfServiceCreateAt, showFullName, showEmail, u.BotLastIconUpdate)
 }
 
 // Remove any private data from the user object

store/sqlstore/bot_store.go

@@ -18,6 +18,7 @@ type bot struct {
 	UserId         string `json:"user_id"`
 	Description    string `json:"description"`
 	OwnerId        string `json:"owner_id"`
+	LastIconUpdate int64  `json:"last_icon_update"`
 	CreateAt       int64  `json:"create_at"`
 	UpdateAt       int64  `json:"update_at"`
 	DeleteAt       int64  `json:"delete_at"`
@@ -28,6 +29,7 @@ func botFromModel(b *model.Bot) *bot {
 		UserId:         b.UserId,
 		Description:    b.Description,
 		OwnerId:        b.OwnerId,
+		LastIconUpdate: b.LastIconUpdate,
 		CreateAt:       b.CreateAt,
 		UpdateAt:       b.UpdateAt,
 		DeleteAt:       b.DeleteAt,
@@ -89,6 +91,7 @@ func (us SqlBotStore) Get(botUserId string, includeDeleted bool) (*model.Bot, *m
 			u.FirstName AS DisplayName,
 			b.Description,
 			b.OwnerId,
+			b.LastIconUpdate,
 			b.CreateAt,
 			b.UpdateAt,
 			b.DeleteAt
@@ -145,6 +148,7 @@ func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, *model
 			    u.FirstName AS DisplayName,
 			    b.Description,
 			    b.OwnerId,
+				b.LastIconUpdate,
 			    b.CreateAt,
 			    b.UpdateAt,
 			    b.DeleteAt
@@ -205,6 +209,7 @@ func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, *model.AppError) {
 
 	oldBot.Description = bot.Description
 	oldBot.OwnerId = bot.OwnerId
+	oldBot.LastIconUpdate = bot.LastIconUpdate
 	oldBot.UpdateAt = bot.UpdateAt
 	oldBot.DeleteAt = bot.DeleteAt
 	bot = oldBot

store/sqlstore/user_store.go

@@ -60,7 +60,7 @@ func NewSqlUserStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) st
 	}
 
 	us.usersQuery = us.getQueryBuilder().
-		Select("u.*", "b.UserId IS NOT NULL AS IsBot", "COALESCE(b.Description, '') AS BotDescription").
+		Select("u.*", "b.UserId IS NOT NULL AS IsBot", "COALESCE(b.Description, '') AS BotDescription", "COALESCE(b.LastIconUpdate, 0) AS BotLastIconUpdate").
 		From("Users u").
 		LeftJoin("Bots b ON ( b.UserId = u.Id )")