From d4378a6b42a83eb321af29e6f1abee2ad7e307a0 Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Fri, 18 Feb 2022 15:04:18 -0700 Subject: [PATCH] [MM-41668] Remove "add support email" admin advisor notifications (#19567) Automatic Merge --- app/admin_advisor.go | 126 ------------------------------------------- app/server.go | 17 ------ app/server_test.go | 89 ------------------------------ i18n/en.json | 36 ------------- model/system.go | 8 --- 5 files changed, 276 deletions(-) diff --git a/app/admin_advisor.go b/app/admin_advisor.go index aa8e98087e..a6b58f3bc7 100644 --- a/app/admin_advisor.go +++ b/app/admin_advisor.go @@ -4,7 +4,6 @@ package app import ( - "fmt" "net/http" "strings" @@ -137,9 +136,6 @@ func (a *App) getWarnMetricStatusAndDisplayTextsForId(warnMetricId string, T i18 warnMetricDisplayTexts.EmailBody = T("api.server.warn_metric.number_of_posts_2M.contact_us.email_body") warnMetricDisplayTexts.BotMessageBody = T("api.server.warn_metric.number_of_posts_2M.notification_body") } - case model.SystemMetricSupportEmailNotConfigured: - warnMetricDisplayTexts.BotTitle = T("api.server.warn_metric.support_email_not_configured.notification_title") - warnMetricDisplayTexts.BotMessageBody = T("api.server.warn_metric.support_email_not_configured.start_trial.notification_body") default: mlog.Debug("Invalid metric id", mlog.String("id", warnMetricId)) return nil, nil @@ -150,128 +146,6 @@ func (a *App) getWarnMetricStatusAndDisplayTextsForId(warnMetricId string, T i18 return nil, nil } -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, - PerPage: perPage, - Role: model.SystemAdminRoleId, - Inactive: false, - } - - // get sysadmins - var sysAdmins []*model.User - for { - sysAdminsList, err := a.GetUsers(userOptions) - if err != nil { - return err - } - - if len(sysAdmins) == 0 && len(sysAdminsList) == 0 { - return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.empty_admin_list.app_error", nil, "", http.StatusInternalServerError) - } - sysAdmins = append(sysAdmins, sysAdminsList...) - - if len(sysAdminsList) < perPage { - mlog.Debug("Number of system admins is less than page limit", mlog.Int("count", len(sysAdminsList))) - break - } - - userOptions.Page++ - } - - for _, sysAdmin := range sysAdmins { - T := i18n.GetUserTranslations(sysAdmin.Locale) - warnMetricsBot.DisplayName = T("app.system.warn_metric.bot_displayname") - warnMetricsBot.Description = T("app.system.warn_metric.bot_description") - - channel, appErr := a.GetOrCreateDirectChannel(c, warnMetricsBot.UserId, sysAdmin.Id) - if appErr != nil { - return appErr - } - - warnMetricStatus, warnMetricDisplayTexts := a.getWarnMetricStatusAndDisplayTextsForId(warnMetricId, T, isE0Edition) - if warnMetricStatus == nil { - return model.NewAppError("NotifyAdminsOfWarnMetricStatus", "app.system.warn_metric.notification.invalid_metric.app_error", nil, "", http.StatusInternalServerError) - } - - botPost := &model.Post{ - UserId: warnMetricsBot.UserId, - ChannelId: channel.Id, - Type: model.PostTypeSystemWarnMetricStatus, - Message: "", - } - - actionId := "contactUs" - actionName := T("api.server.warn_metric.contact_us") - postActionValue := T("api.server.warn_metric.contacting_us") - postActionURL := fmt.Sprintf("/warn_metrics/ack/%s", warnMetricId) - - if isE0Edition { - actionId = "startTrial" - actionName = T("api.server.warn_metric.start_trial") - postActionValue = T("api.server.warn_metric.starting_trial") - postActionURL = fmt.Sprintf("/warn_metrics/trial-license-ack/%s", warnMetricId) - } - - actions := []*model.PostAction{} - actions = append(actions, - &model.PostAction{ - Id: actionId, - Name: actionName, - Type: model.PostActionTypeButton, - Options: []*model.PostActionOptions{ - { - Text: "TrackEventId", - Value: warnMetricId, - }, - { - Text: "ActionExecutingMessage", - Value: postActionValue, - }, - }, - Integration: &model.PostActionIntegration{ - Context: model.StringInterface{ - "bot_user_id": warnMetricsBot.UserId, - "force_ack": false, - }, - URL: postActionURL, - }, - }, - ) - - attachments := []*model.SlackAttachment{{ - AuthorName: "", - Title: warnMetricDisplayTexts.BotTitle, - Text: warnMetricDisplayTexts.BotMessageBody, - }} - - if !warnMetric.SkipAction { - attachments[0].Actions = actions - } - - model.ParseSlackAttachment(botPost, attachments) - - mlog.Debug("Post admin advisory for metric", mlog.String("warnMetricId", warnMetricId), mlog.String("userid", botPost.UserId)) - if _, err := a.CreatePostAsUser(c, botPost, c.Session().Id, true); err != nil { - return err - } - } - - return nil -} - func (a *App) NotifyAndSetWarnMetricAck(warnMetricId string, sender *model.User, forceAck bool, isBot bool) *model.AppError { if warnMetric, ok := model.WarnMetricsTable[warnMetricId]; ok { data, nErr := a.Srv().Store.System().GetByName(warnMetric.Id) diff --git a/app/server.go b/app/server.go index a9b8ec8ea3..4979e08220 100644 --- a/app/server.go +++ b/app/server.go @@ -624,7 +624,6 @@ func NewServer(options ...Option) (*Server, error) { s.Go(func() { appInstance := New(ServerConnector(s.Channels())) s.runLicenseExpirationCheckJob() - runCheckAdminSupportStatusJob(appInstance, request.EmptyContext()) runDNDStatusExpireJob(appInstance) }) s.runJobs() @@ -1495,12 +1494,6 @@ func doReportUsageToAWSMeteringService(s *Server) { awsMeter.ReportUserCategoryUsage(reports) } -func runCheckAdminSupportStatusJob(a *App, c *request.Context) { - model.CreateRecurringTask("Check Admin Support Status Job", func() { - doCheckAdminSupportStatus(a, c) - }, time.Hour*model.WarnMetricJobInterval) -} - func doSecurity(s *Server) { s.DoSecurityUpdateCheck() } @@ -1544,16 +1537,6 @@ func doJobsCleanup(s *Server) { } } -func doCheckAdminSupportStatus(a *App, c *request.Context) { - isE0Edition := model.BuildEnterpriseReady == "true" - - if strings.TrimSpace(*a.Config().SupportSettings.SupportEmail) == model.SupportSettingsDefaultSupportEmail { - if err := a.notifyAdminsOfWarnMetricStatus(c, model.SystemMetricSupportEmailNotConfigured, isE0Edition); err != nil { - mlog.Error("Failed to send notifications to admin users.", mlog.Err(err)) - } - } -} - func (s *Server) StopMetricsServer() { s.metricsLock.Lock() defer s.metricsLock.Unlock() diff --git a/app/server_test.go b/app/server_test.go index a9bfe660c3..4f9ca6536c 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/mattermost/mattermost-server/v6/app/users" "github.com/mattermost/mattermost-server/v6/config" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/shared/filestore" @@ -706,91 +705,3 @@ func TestSentry(t *testing.T) { } }) } - -func TestAdminAdvisor(t *testing.T) { - th := Setup(t) - defer th.TearDown() - - // creating a system user to whole admin advisor will send post - user := model.User{ - Email: strings.ToLower(model.NewId()) + "success+test@example.com", - Nickname: "Darth Vader", - Username: "vader" + model.NewId(), - Password: "passwd1", - AuthService: "", - Roles: model.SystemAdminRoleId, - } - ruser, err := th.App.CreateUser(th.Context, &user) - assert.Nil(t, err, "User should be created") - defer th.App.PermanentDeleteUser(th.Context, &user) - - t.Run("Should notify admin of un-configured support email", func(t *testing.T) { - doCheckAdminSupportStatus(th.App, th.Context) - - bot, err := th.App.GetUserByUsername(model.BotWarnMetricBotUsername) - assert.NotNil(t, bot, "Bot should have been created now") - assert.Nil(t, err, "No error should be generated") - - channel, err := th.App.getDirectChannel(bot.Id, ruser.Id) - assert.NotNil(t, channel, "DM channel should exist between Admin Advisor and system admin") - assert.Nil(t, err, "No error should be generated") - }) - - t.Run("Should NOT notify admin when support email is configured", func(t *testing.T) { - th.App.UpdateConfig(func(m *model.Config) { - email := "success+test@example.com" - m.SupportSettings.SupportEmail = &email - }) - - bot, err := th.App.GetUserByUsername(model.BotWarnMetricBotUsername) - assert.NotNil(t, bot, "Bot should be already created") - assert.Nil(t, err, "No error should be generated") - - channel, err := th.App.getDirectChannel(bot.Id, ruser.Id) - assert.NotNil(t, channel, "DM channel should already exist") - assert.Nil(t, err, "No error should be generated") - - err = th.App.PermanentDeleteChannel(channel) - assert.Nil(t, err, "No error should be generated") - - doCheckAdminSupportStatus(th.App, th.Context) - - channel, err = th.App.getDirectChannel(bot.Id, ruser.Id) - assert.NotNil(t, channel, "DM channel should exist between Admin Advisor and system admin") - assert.Nil(t, err, "No error should be generated") - - posts, err := th.App.GetPosts(channel.Id, 0, 100) - assert.Nil(t, err, "No error should be generated") - assert.Equal(t, 0, len(posts.Posts)) - }) - - t.Run("Should not break in case of many sysadmins", func(t *testing.T) { - var userList []*model.User - for i := 0; i < 50; i++ { - user := model.User{ - Email: strings.ToLower(NewTestId()) + "success+test@example.com", - Nickname: "Admin", - Username: "admin" + NewTestId(), - Password: "password", - AuthService: "", - Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, - } - ruser, err := th.App.ch.srv.userService.CreateUser(&user, users.UserCreateOptions{FromImport: true}) - assert.NoError(t, err, "User should be created") - userList = append(userList, ruser) - defer th.App.PermanentDeleteUser(th.Context, ruser) - } - - th.App.notifyAdminsOfWarnMetricStatus(th.Context, model.SystemMetricSupportEmailNotConfigured, true) - - bot, err := th.App.GetUserByUsername(model.BotWarnMetricBotUsername) - assert.NotNil(t, bot, "Bot should have been created now") - assert.Nil(t, err, "No error should be generated") - - for _, user := range userList { - channel, err := th.App.getDirectChannel(bot.Id, user.Id) - assert.NotNil(t, channel, "DM channel should exist between Admin Advisor and system admin") - assert.Nil(t, err, "No error should be generated") - } - }) -} diff --git a/i18n/en.json b/i18n/en.json index e7ba44fa22..ffb73c3543 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2535,14 +2535,6 @@ "id": "api.server.warn_metric.bot_response.start_trial_failure.message", "translation": "Trial license could not be retrieved. Visit https://mattermost.com/trial/ to request a license." }, - { - "id": "api.server.warn_metric.contact_us", - "translation": "Contact Us" - }, - { - "id": "api.server.warn_metric.contacting_us", - "translation": "Contacting Us" - }, { "id": "api.server.warn_metric.email_domain.contact_us.email_body", "translation": "Mattermost contact us request. I'm interested in learning more about using Guest Accounts.\r\n" @@ -2727,22 +2719,6 @@ "id": "api.server.warn_metric.number_of_teams_5.start_trial_notification_success.message", "translation": "Your Enterprise trial is now active. Go to **System Console > User Management > Permissions** to enable Advanced Permissions." }, - { - "id": "api.server.warn_metric.start_trial", - "translation": "Start Trial" - }, - { - "id": "api.server.warn_metric.starting_trial", - "translation": "Getting Trial" - }, - { - "id": "api.server.warn_metric.support_email_not_configured.notification_title", - "translation": "Set Your Support Email Address" - }, - { - "id": "api.server.warn_metric.support_email_not_configured.start_trial.notification_body", - "translation": "Please go to **System Console > Site Configuration > Customization** to set the [Support Email](https://docs.mattermost.com/administration/config-settings.html#support-email) as your organization's internal IT Support Desk email address for end user feedback, email notifications, and support requests." - }, { "id": "api.slackimport.slack_add_bot_user.email_pwd", "translation": "The Integration/Slack Bot user with email {{.Email}} and password {{.Password}} has been imported.\r\n" @@ -6151,22 +6127,10 @@ "id": "app.system.system_bot.bot_displayname", "translation": "System" }, - { - "id": "app.system.warn_metric.bot_description", - "translation": "[Learn more about the Mattermost Advisor](https://about.mattermost.com/default-channel-handle-documentation)" - }, { "id": "app.system.warn_metric.bot_displayname", "translation": "Mattermost Advisor" }, - { - "id": "app.system.warn_metric.notification.empty_admin_list.app_error", - "translation": "List of admins is empty." - }, - { - "id": "app.system.warn_metric.notification.invalid_metric.app_error", - "translation": "Could not find metric." - }, { "id": "app.system.warn_metric.store.app_error", "translation": "Failed to store value for {{.WarnMetricName}}" diff --git a/model/system.go b/model/system.go index c8a4210118..88b58d6e2b 100644 --- a/model/system.go +++ b/model/system.go @@ -30,7 +30,6 @@ const ( SystemWarnMetricNumberOfActiveUsers500 = "warn_metric_number_of_active_users_500" SystemWarnMetricNumberOfPosts2m = "warn_metric_number_of_posts_2M" SystemWarnMetricLastRunTimestampKey = "LastWarnMetricRunTimestamp" - SystemMetricSupportEmailNotConfigured = "warn_metric_support_email_not_configured" SystemFirstAdminVisitMarketplace = "FirstAdminVisitMarketplace" SystemFirstAdminCompleteSetup = "FirstAdminCompleteSetup" AwsMeteringReportInterval = 1 @@ -148,13 +147,6 @@ var WarnMetricsTable = map[string]WarnMetric{ IsBotOnly: false, IsRunOnce: true, }, - SystemMetricSupportEmailNotConfigured: { - Id: SystemMetricSupportEmailNotConfigured, - Limit: -1, - IsBotOnly: true, - IsRunOnce: false, - SkipAction: true, - }, } type WarnMetric struct {