[MM-27622] Publish messages as SystemBot when a user is required (#17598)

Automatic Merge
2021-06-09 17:40:22 +02:00
--- a/app/app.go
+++ b/app/app.go
@@ -237,6 +237,17 @@ func (a *App) getWarnMetricStatusAndDisplayTextsForId(warnMetricId string, T i18

 //nolint:golint,unused,deadcode
 func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId string, isE0Edition bool) *model.AppError {
+	// get warn metrics bot
+	warnMetricsBot, err := a.GetWarnMetricsBot()
+	if err != nil {
+		return err
+	}
+
+	warnMetric, ok := model.WarnMetricsTable[warnMetricId]
+	if !ok {
+		return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.invalid_metric.app_error", nil, "", http.StatusInternalServerError)
+	}
+
 	perPage := 25
 	userOptions := &model.UserGetOptions{
 		Page:     0,
@@ -264,30 +275,12 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st
 		}
 	}

-	T := i18n.GetUserTranslations(sysAdmins[0].Locale)
-	warnMetricsBot := &model.Bot{
-		Username:    model.BOT_WARN_METRIC_BOT_USERNAME,
-		DisplayName: T("app.system.warn_metric.bot_displayname"),
-		Description: "",
-		OwnerId:     sysAdmins[0].Id,
-	}
-
-	bot, err := a.getOrCreateWarnMetricsBot(warnMetricsBot)
-	if err != nil {
-		return err
-	}
-
-	warnMetric, ok := model.WarnMetricsTable[warnMetricId]
-	if !ok {
-		return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.invalid_metric.app_error", nil, "", http.StatusInternalServerError)
-	}
-
 	for _, sysAdmin := range sysAdmins {
 		T := i18n.GetUserTranslations(sysAdmin.Locale)
-		bot.DisplayName = T("app.system.warn_metric.bot_displayname")
-		bot.Description = T("app.system.warn_metric.bot_description")
+		warnMetricsBot.DisplayName = T("app.system.warn_metric.bot_displayname")
+		warnMetricsBot.Description = T("app.system.warn_metric.bot_description")

-		channel, appErr := a.GetOrCreateDirectChannel(c, bot.UserId, sysAdmin.Id)
+		channel, appErr := a.GetOrCreateDirectChannel(c, warnMetricsBot.UserId, sysAdmin.Id)
 		if appErr != nil {
 			return appErr
 		}
@@ -298,7 +291,7 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st
 		}

 		botPost := &model.Post{
-			UserId:    bot.UserId,
+			UserId:    warnMetricsBot.UserId,
 			ChannelId: channel.Id,
 			Type:      model.POST_SYSTEM_WARN_METRIC_STATUS,
 			Message:   "",
@@ -334,7 +327,7 @@ func (a *App) notifyAdminsOfWarnMetricStatus(c *request.Context, warnMetricId st
 				},
 				Integration: &model.PostActionIntegration{
 					Context: model.StringInterface{
-						"bot_user_id": bot.UserId,
+						"bot_user_id": warnMetricsBot.UserId,
 						"force_ack":   false,
 					},
 					URL: postActionUrl,
--- a/app/app_iface.go
+++ b/app/app_iface.go
@@ -382,6 +382,8 @@ type AppIface interface {
 	VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError
 	//GetUserStatusesByIds used by apiV4
 	GetUserStatusesByIds(userIDs []string) ([]*model.Status, *model.AppError)
+	//nolint:golint,unused,deadcode
+	GetWarnMetricsBot() (*model.Bot, *model.AppError)
 	AccountMigration() einterfaces.AccountMigrationInterface
 	ActivateMfa(userID, token string) *model.AppError
 	AddChannelsToRetentionPolicy(policyID string, channelIDs []string) *model.AppError
@@ -736,6 +738,7 @@ type AppIface interface {
 	GetStatusFromCache(userID string) *model.Status
 	GetStatusesByIds(userIDs []string) (map[string]interface{}, *model.AppError)
 	GetSubscriptionStats() (*model.SubscriptionStats, *model.AppError)
+	GetSystemBot() (*model.Bot, *model.AppError)
 	GetTeam(teamID string) (*model.Team, *model.AppError)
 	GetTeamByInviteId(inviteId string) (*model.Team, *model.AppError)
 	GetTeamByName(name string) (*model.Team, *model.AppError)
--- a/app/bot.go
+++ b/app/bot.go
@@ -91,7 +91,65 @@ func (a *App) CreateBot(c *request.Context, bot *model.Bot) (*model.Bot, *model.
 }

 //nolint:golint,unused,deadcode
-func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.AppError) {
+func (a *App) GetWarnMetricsBot() (*model.Bot, *model.AppError) {
+	perPage := 1
+	userOptions := &model.UserGetOptions{
+		Page:     0,
+		PerPage:  perPage,
+		Role:     model.SYSTEM_ADMIN_ROLE_ID,
+		Inactive: false,
+	}
+
+	sysAdminList, err := a.GetUsers(userOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(sysAdminList) == 0 {
+		return nil, model.NewAppError("GetWarnMetricsBot", "app.bot.get_warn_metrics_bot.empty_admin_list.app_error", nil, "", http.StatusInternalServerError)
+	}
+
+	T := i18n.GetUserTranslations(sysAdminList[0].Locale)
+	warnMetricsBot := &model.Bot{
+		Username:    model.BOT_WARN_METRIC_BOT_USERNAME,
+		DisplayName: T("app.system.warn_metric.bot_displayname"),
+		Description: "",
+		OwnerId:     sysAdminList[0].Id,
+	}
+
+	return a.getOrCreateBot(warnMetricsBot)
+}
+
+func (a *App) GetSystemBot() (*model.Bot, *model.AppError) {
+	perPage := 1
+	userOptions := &model.UserGetOptions{
+		Page:     0,
+		PerPage:  perPage,
+		Role:     model.SYSTEM_ADMIN_ROLE_ID,
+		Inactive: false,
+	}
+
+	sysAdminList, err := a.GetUsers(userOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(sysAdminList) == 0 {
+		return nil, model.NewAppError("GetSystemBot", "app.bot.get_system_bot.empty_admin_list.app_error", nil, "", http.StatusInternalServerError)
+	}
+
+	T := i18n.GetUserTranslations(sysAdminList[0].Locale)
+	systemBot := &model.Bot{
+		Username:    model.BOT_SYSTEM_BOT_USERNAME,
+		DisplayName: T("app.system.system_bot.bot_displayname"),
+		Description: "",
+		OwnerId:     sysAdminList[0].Id,
+	}
+
+	return a.getOrCreateBot(systemBot)
+}
+
+func (a *App) getOrCreateBot(botDef *model.Bot) (*model.Bot, *model.AppError) {
 	botUser, appErr := a.GetUserByUsername(botDef.Username)
 	if appErr != nil {
 		if appErr.StatusCode != http.StatusNotFound {
@@ -116,9 +174,9 @@ func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.A
 				default:
 					code = "app.user.save.existing.app_error"
 				}
-				return nil, model.NewAppError("getOrCreateWarnMetricsBot", code, nil, invErr.Error(), http.StatusBadRequest)
+				return nil, model.NewAppError("getOrCreateBot", code, nil, invErr.Error(), http.StatusBadRequest)
 			default:
-				return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.user.save.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+				return nil, model.NewAppError("getOrCreateBot", "app.user.save.app_error", nil, nErr.Error(), http.StatusInternalServerError)
 			}
 		}
 		botDef.UserId = user.Id
@@ -132,14 +190,14 @@ func (a *App) getOrCreateWarnMetricsBot(botDef *model.Bot) (*model.Bot, *model.A
 			case errors.As(nErr, &nAppErr): // in case we haven't converted to plain error.
 				return nil, nAppErr
 			default: // last fallback in case it doesn't map to an existing app error.
-				return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.bot.createbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError)
+				return nil, model.NewAppError("getOrCreateBot", "app.bot.createbot.internal_error", nil, nErr.Error(), http.StatusInternalServerError)
 			}
 		}
 		return savedBot, nil
 	}

 	if botUser == nil {
-		return nil, model.NewAppError("getOrCreateWarnMetricsBot", "app.bot.createbot.internal_error", nil, "", http.StatusInternalServerError)
+		return nil, model.NewAppError("getOrCreateBot", "app.bot.createbot.internal_error", nil, "", http.StatusInternalServerError)
 	}

 	//return the bot for this user
--- a/app/bot_test.go
+++ b/app/bot_test.go
@@ -915,6 +915,44 @@ func TestDeleteBotIconImage(t *testing.T) {
 	})
 }

+func TestGetSystemBot(t *testing.T) {
+	t.Run("An error should be returned if there are no sysadmins in the instance", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		require.Nil(t, th.App.PermanentDeleteAllUsers(th.Context))
+
+		_, err := th.App.GetSystemBot()
+		require.NotNil(t, err)
+		require.Equal(t, "app.bot.get_system_bot.empty_admin_list.app_error", err.Id)
+	})
+
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	t.Run("The bot should be created the first time it's retrieved", func(t *testing.T) {
+		// assert no bot with username exists
+		_, err := th.App.GetUserByUsername(model.BOT_SYSTEM_BOT_USERNAME)
+		require.NotNil(t, err)
+
+		bot, err := th.App.GetSystemBot()
+		require.Nil(t, err)
+		require.Equal(t, bot.Username, model.BOT_SYSTEM_BOT_USERNAME)
+	})
+
+	t.Run("The bot should be correctly retrieved if it exists already", func(t *testing.T) {
+		// assert that the bot is now present
+		botUser, err := th.App.GetUserByUsername(model.BOT_SYSTEM_BOT_USERNAME)
+		require.Nil(t, err)
+		require.True(t, botUser.IsBot)
+
+		bot, err := th.App.GetSystemBot()
+		require.Nil(t, err)
+		require.Equal(t, bot.Username, model.BOT_SYSTEM_BOT_USERNAME)
+		require.Equal(t, bot.UserId, botUser.Id)
+	})
+}
+
 func sToP(s string) *string {
 	return &s
 }
--- a/app/channel.go
+++ b/app/channel.go
@@ -710,6 +710,21 @@ func (a *App) UpdateChannelPrivacy(c *request.Context, oldChannel *model.Channel
 }

 func (a *App) postChannelPrivacyMessage(c *request.Context, user *model.User, channel *model.Channel) *model.AppError {
+	var authorId string
+	var authorUsername string
+	if user != nil {
+		authorId = user.Id
+		authorUsername = user.Username
+	} else {
+		systemBot, err := a.GetSystemBot()
+		if err != nil {
+			return model.NewAppError("postChannelPrivacyMessage", "api.channel.post_channel_privacy_message.error", nil, err.Error(), http.StatusInternalServerError)
+		}
+
+		authorId = systemBot.UserId
+		authorUsername = systemBot.Username
+	}
+
 	message := (map[string]string{
 		model.CHANNEL_OPEN:    i18n.T("api.channel.change_channel_privacy.private_to_public"),
 		model.CHANNEL_PRIVATE: i18n.T("api.channel.change_channel_privacy.public_to_private"),
@@ -718,9 +733,9 @@ func (a *App) postChannelPrivacyMessage(c *request.Context, user *model.User, ch
 		ChannelId: channel.Id,
 		Message:   message,
 		Type:      model.POST_CHANGE_CHANNEL_PRIVACY,
-		UserId:    user.Id,
+		UserId:    authorId,
 		Props: model.StringInterface{
-			"username": user.Username,
+			"username": authorUsername,
 		},
 	}

@@ -746,14 +761,18 @@ func (a *App) RestoreChannel(c *request.Context, channel *model.Channel, userID
 	message.Add("channel_id", channel.Id)
 	a.Publish(message)

-	user, nErr := a.Srv().Store.User().Get(context.Background(), userID)
-	if nErr != nil {
-		var nfErr *store.ErrNotFound
-		switch {
-		case errors.As(nErr, &nfErr):
-			return nil, model.NewAppError("RestoreChannel", MissingAccountError, nil, nfErr.Error(), http.StatusNotFound)
-		default:
-			return nil, model.NewAppError("RestoreChannel", "app.user.get.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	var user *model.User
+	if userID != "" {
+		var nErr error
+		user, nErr = a.Srv().Store.User().Get(context.Background(), userID)
+		if nErr != nil {
+			var nfErr *store.ErrNotFound
+			switch {
+			case errors.As(nErr, &nfErr):
+				return nil, model.NewAppError("RestoreChannel", MissingAccountError, nil, nfErr.Error(), http.StatusNotFound)
+			default:
+				return nil, model.NewAppError("RestoreChannel", "app.user.get.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+			}
 		}
 	}

@@ -773,6 +792,28 @@ func (a *App) RestoreChannel(c *request.Context, channel *model.Channel, userID
 		if _, err := a.CreatePost(c, post, channel, false, true); err != nil {
 			mlog.Warn("Failed to post unarchive message", mlog.Err(err))
 		}
+	} else {
+		a.Srv().Go(func() {
+			systemBot, err := a.GetSystemBot()
+			if err != nil {
+				mlog.Error("Failed to post unarchive message", mlog.Err(err))
+				return
+			}
+
+			post := &model.Post{
+				ChannelId: channel.Id,
+				Message:   i18n.T("api.channel.restore_channel.unarchived", map[string]interface{}{"Username": systemBot.Username}),
+				Type:      model.POST_CHANNEL_RESTORED,
+				UserId:    systemBot.UserId,
+				Props: model.StringInterface{
+					"username": systemBot.Username,
+				},
+			}
+
+			if _, err := a.CreatePost(c, post, channel, false, true); err != nil {
+				mlog.Error("Failed to post unarchive message", mlog.Err(err))
+			}
+		})
 	}

 	return channel, nil
@@ -1297,6 +1338,28 @@ func (a *App) DeleteChannel(c *request.Context, channel *model.Channel, userID s
 		if _, err := a.CreatePost(c, post, channel, false, true); err != nil {
 			mlog.Warn("Failed to post archive message", mlog.Err(err))
 		}
+	} else {
+		a.Srv().Go(func() {
+			systemBot, err := a.GetSystemBot()
+			if err != nil {
+				mlog.Error("Failed to post archive message", mlog.Err(err))
+				return
+			}
+
+			post := &model.Post{
+				ChannelId: channel.Id,
+				Message:   fmt.Sprintf(i18n.T("api.channel.delete_channel.archived"), systemBot.Username),
+				Type:      model.POST_CHANNEL_DELETED,
+				UserId:    systemBot.UserId,
+				Props: model.StringInterface{
+					"username": systemBot.Username,
+				},
+			}
+
+			if _, err := a.CreatePost(c, post, channel, false, true); err != nil {
+				mlog.Error("Failed to post archive message", mlog.Err(err))
+			}
+		})
 	}

 	now := model.GetMillis()
@@ -1468,7 +1531,9 @@ func (a *App) AddChannelMember(c *request.Context, userID string, channel *model
 	}

 	if opts.UserRequestorID == "" || userID == opts.UserRequestorID {
-		a.postJoinChannelMessage(c, user, channel)
+		if err := a.postJoinChannelMessage(c, user, channel); err != nil {
+			mlog.Error("Failed to post join channel message", mlog.Err(err))
+		}
 	} else {
 		a.Srv().Go(func() {
 			a.PostAddToChannelMessage(c, userRequestor, user, channel, opts.PostRootID)
@@ -2179,6 +2244,16 @@ func (a *App) postAddToTeamMessage(c *request.Context, user *model.User, addedUs
 }

 func (a *App) postRemoveFromChannelMessage(c *request.Context, removerUserId string, removedUser *model.User, channel *model.Channel) *model.AppError {
+	messageUserId := removerUserId
+	if messageUserId == "" {
+		systemBot, err := a.GetSystemBot()
+		if err != nil {
+			return model.NewAppError("postRemoveFromChannelMessage", "api.channel.post_user_add_remove_message_and_forget.error", nil, err.Error(), http.StatusInternalServerError)
+		}
+
+		messageUserId = systemBot.UserId
+	}
+
 	post := &model.Post{
 		ChannelId: channel.Id,
 		// Message here embeds `@username`, not just `username`, to ensure that mentions
@@ -2186,7 +2261,7 @@ func (a *App) postRemoveFromChannelMessage(c *request.Context, removerUserId str
 		// The client renders its own system message, ignoring this value altogether.
 		Message: fmt.Sprintf(i18n.T("api.channel.remove_member.removed"), fmt.Sprintf("@%s", removedUser.Username)),
 		Type:    model.POST_REMOVE_FROM_CHANNEL,
-		UserId:  removerUserId,
+		UserId:  messageUserId,
 		Props: model.StringInterface{
 			"removedUserId":   removedUser.Id,
 			"removedUsername": removedUser.Username,
@@ -2308,7 +2383,9 @@ func (a *App) RemoveUserFromChannel(c *request.Context, userIDToRemove string, r
 		}
 	} else {
 		a.Srv().Go(func() {
-			a.postRemoveFromChannelMessage(c, removerUserId, user, channel)
+			if err := a.postRemoveFromChannelMessage(c, removerUserId, user, channel); err != nil {
+				mlog.Error("Failed to post user removal message", mlog.Err(err))
+			}
 		})
 	}

--- a/app/opentracing/opentracing_layer.go
+++ b/app/opentracing/opentracing_layer.go
@@ -8809,6 +8809,28 @@ func (a *OpenTracingAppLayer) GetSuggestions(c *request.Context, commandArgs *mo
 	return resultVar0
 }

+func (a *OpenTracingAppLayer) GetSystemBot() (*model.Bot, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetSystemBot")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := a.app.GetSystemBot()
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (a *OpenTracingAppLayer) GetTeam(teamID string) (*model.Team, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetTeam")
@@ -10136,6 +10158,28 @@ func (a *OpenTracingAppLayer) GetViewUsersRestrictions(userID string) (*model.Vi
 	return resultVar0, resultVar1
 }

+func (a *OpenTracingAppLayer) GetWarnMetricsBot() (*model.Bot, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetWarnMetricsBot")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := a.app.GetWarnMetricsBot()
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (a *OpenTracingAppLayer) GetWarnMetricsStatus() (map[string]*model.WarnMetricStatus, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetWarnMetricsStatus")