diff --git a/api4/notify_admin.go b/api4/notify_admin.go index 36af8263a0..55048ea126 100644 --- a/api4/notify_admin.go +++ b/api4/notify_admin.go @@ -7,10 +7,13 @@ import ( "encoding/json" "net/http" + "github.com/mattermost/mattermost-server/v6/shared/mlog" + "github.com/mattermost/mattermost-server/v6/model" ) func handleNotifyAdmin(c *Context, w http.ResponseWriter, r *http.Request) { + mlog.Info("enter handleNotifyAdmin") var notifyAdminRequest *model.NotifyAdminToUpgradeRequest err := json.NewDecoder(r.Body).Decode(¬ifyAdminRequest) if err != nil { @@ -24,11 +27,14 @@ func handleNotifyAdmin(c *Context, w http.ResponseWriter, r *http.Request) { c.Err = appErr return } + mlog.Info("exit handleNotifyAdmin") ReturnStatusOK(w) } func handleTriggerNotifyAdminPosts(c *Context, w http.ResponseWriter, r *http.Request) { + mlog.Info("enter handleTriggerNotifyAdminPosts") + if !*c.App.Config().ServiceSettings.EnableAPITriggerAdminNotifications { c.Err = model.NewAppError("Api4.handleTriggerNotifyAdminPosts", "api.cloud.app_error", nil, "Manual triggering of notifications not allowed", http.StatusForbidden) return @@ -53,5 +59,7 @@ func handleTriggerNotifyAdminPosts(c *Context, w http.ResponseWriter, r *http.Re return } + mlog.Info("exit handleTriggerNotifyAdminPosts") + ReturnStatusOK(w) } diff --git a/app/app_iface.go b/app/app_iface.go index d7e3ad9c21..aa0fea139d 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -573,7 +573,7 @@ type AppIface interface { FillInChannelsProps(c request.CTX, channelList model.ChannelList) *model.AppError FilterUsersByVisible(viewer *model.User, otherUsers []*model.User) ([]*model.User, *model.AppError) FindTeamByName(name string) bool - FinishSendAdminNotifyPost(trial bool, now int64) + FinishSendAdminNotifyPost(trial bool, now int64, pluginBasedData map[string][]*model.NotifyAdminData) GenerateMfaSecret(userID string) (*model.MfaSecret, *model.AppError) GeneratePublicLink(siteURL string, info *model.FileInfo) string GenerateSupportPacket() []model.FileData @@ -1167,7 +1167,7 @@ type AppIface interface { UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) UpsertGroupMembers(groupID string, userIDs []string) ([]*model.GroupMember, *model.AppError) UpsertGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) - UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostPaidFeature) bool + UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostFeature) bool UserCanSeeOtherUser(userID string, otherUserId string) (bool, *model.AppError) UserIsFirstAdmin(user *model.User) bool VerifyEmailFromToken(c request.CTX, userSuppliedTokenString string) *model.AppError diff --git a/app/notify_admin.go b/app/notify_admin.go index b006bc5106..7f44c41054 100644 --- a/app/notify_admin.go +++ b/app/notify_admin.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "strconv" + "strings" "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/model" @@ -19,14 +20,16 @@ import ( const lastTrialNotificationTimeStamp = "LAST_TRIAL_NOTIFICATION_TIMESTAMP" const lastUpgradeNotificationTimeStamp = "LAST_UPGRADE_NOTIFICATION_TIMESTAMP" -const defaultNotifyAdminCoolOffDays = 14 +const defaultNotifyAdminCoolOffDays = 0.0104166667 // this is a temp change func (a *App) SaveAdminNotification(userId string, notifyData *model.NotifyAdminToUpgradeRequest) *model.AppError { requiredFeature := notifyData.RequiredFeature requiredPlan := notifyData.RequiredPlan trial := notifyData.TrialNotification - if a.UserAlreadyNotifiedOnRequiredFeature(userId, requiredFeature) { + isUserAlreadyNotified := a.UserAlreadyNotifiedOnRequiredFeature(userId, requiredFeature) + mlog.Info("SaveAdminNotification") + if isUserAlreadyNotified { return model.NewAppError("app.SaveAdminNotification", "api.cloud.notify_admin_to_upgrade_error.already_notified", nil, "", http.StatusForbidden) } @@ -36,6 +39,7 @@ func (a *App) SaveAdminNotification(userId string, notifyData *model.NotifyAdmin RequiredFeature: requiredFeature, Trial: trial, }) + if appErr != nil { return appErr } @@ -57,7 +61,9 @@ func (a *App) DoCheckForAdminNotifications(trial bool) *model.AppError { } func (a *App) SaveAdminNotifyData(data *model.NotifyAdminData) (*model.NotifyAdminData, *model.AppError) { + mlog.Info("Trying to save NotifyAdmin data") d, err := a.Srv().Store().NotifyAdmin().Save(data) + mlog.Info("NotifyAdmin data saved") if err != nil { var nfErr *store.ErrNotFound switch { @@ -67,6 +73,7 @@ func (a *App) SaveAdminNotifyData(data *model.NotifyAdminData) (*model.NotifyAdm return nil, model.NewAppError("SaveAdminNotifyData", "app.notify_admin.save.app_error", nil, err.Error(), http.StatusInternalServerError) } } + mlog.Info("return from SaveAdminNotifyData") return d, nil } @@ -81,6 +88,8 @@ func filterNotificationData(data []*model.NotifyAdminData, test func(*model.Noti } func (a *App) SendNotifyAdminPosts(c *request.Context, workspaceName string, currentSKU string, trial bool) *model.AppError { + mlog.Info("enter SendNotifyAdminPosts") + if !a.CanNotifyAdmin(trial) { return model.NewAppError("SendNotifyAdminPosts", "app.notify_admin.send_notification_post.app_error", nil, "Cannot notify yet", http.StatusForbidden) } @@ -103,6 +112,8 @@ func (a *App) SendNotifyAdminPosts(c *request.Context, workspaceName string, cur now := model.GetMillis() data, err := a.Srv().Store().NotifyAdmin().Get(trial) + mlog.Info("SendNotifyAdminPosts") + if err != nil { return model.NewAppError("SendNotifyAdminPosts", "app.notify_admin.send_notification_post.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -110,58 +121,99 @@ func (a *App) SendNotifyAdminPosts(c *request.Context, workspaceName string, cur data = filterNotificationData(data, func(nad *model.NotifyAdminData) bool { return nad.RequiredPlan != currentSKU }) if len(data) == 0 { - mlog.Warn("No notification data available") + a.Log().Warn("No notification data available") return nil } - userBasedData := a.groupNotifyAdminByUser(data) - featureBasedData := a.groupNotifyAdminByFeature(data) - props := make(model.StringInterface) + userBasedPaidFeatureData, userBasedPluginData := a.groupNotifyAdminByUser(data) + featureBasedData := a.groupNotifyAdminByPaidFeature(data) + pluginBasedData := a.groupNotifyAdminByPlugin(data) for _, admin := range sysadmins { - T := i18n.GetUserTranslations(admin.Locale) - message := T("app.cloud.upgrade_plan_bot_message", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) - if len(userBasedData) == 1 { - message = T("app.cloud.upgrade_plan_bot_message_single", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) // todo (allan): investigate if translations library can do this - } - if trial { - message = T("app.cloud.trial_plan_bot_message", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) - if len(userBasedData) == 1 { - message = T("app.cloud.trial_plan_bot_message_single", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) - } + if len(userBasedPaidFeatureData) > 0 && len(featureBasedData) > 0 { + a.upgradePlanAdminNotifyPost(c, workspaceName, userBasedPaidFeatureData, featureBasedData, systemBot, admin, trial) } - channel, appErr := a.GetOrCreateDirectChannel(c, systemBot.UserId, admin.Id) - if appErr != nil { - mlog.Warn("Error getting direct channel", mlog.Err(appErr)) - continue - } + if len(userBasedPluginData) > 0 { + mlog.Info("SendNotifyAdminPosts", mlog.String("length of user based plugin data", fmt.Sprint(len(userBasedPluginData)))) - post := &model.Post{ - Message: message, - UserId: systemBot.UserId, - ChannelId: channel.Id, - Type: fmt.Sprintf("%sup_notification", model.PostCustomTypePrefix), // webapp will have to create renderer for this custom post type - - } - - props["requested_features"] = featureBasedData - props["trial"] = trial - post.SetProps(props) - - _, appErr = a.CreatePost(c, post, channel, false, true) - if appErr != nil { - mlog.Warn("Error creating post", mlog.Err(appErr)) - continue + a.pluginInstallAdminNotifyPost(c, userBasedPluginData, pluginBasedData, systemBot, admin) } } - a.FinishSendAdminNotifyPost(trial, now) - + a.FinishSendAdminNotifyPost(trial, now, pluginBasedData) return nil } -func (a *App) UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostPaidFeature) bool { +func (a *App) pluginInstallAdminNotifyPost(c *request.Context, userBasedData map[string][]*model.NotifyAdminData, pluginBasedPluginData map[string][]*model.NotifyAdminData, systemBot *model.Bot, admin *model.User) { + props := make(model.StringInterface) + + channel, appErr := a.GetOrCreateDirectChannel(c, systemBot.UserId, admin.Id) + if appErr != nil { + a.Log().Warn("Error getting direct channel", mlog.Err(appErr)) + return + } + + post := &model.Post{ + UserId: systemBot.UserId, + ChannelId: channel.Id, + Type: fmt.Sprintf("%spl_notification", model.PostCustomTypePrefix), // webapp will have to create renderer for this custom post type + } + + props["requested_plugins_by_plugin_ids"] = pluginBasedPluginData + props["requested_plugins_by_user_ids"] = userBasedData + post.SetProps(props) + mlog.Info("pluginInstallAdminNotifyPost: send props") + + _, appErr = a.CreatePost(c, post, channel, false, true) + mlog.Info("pluginInstallAdminNotifyPost: post created") + + if appErr != nil { + a.Log().Warn("Error creating post", mlog.Err(appErr)) + } +} + +func (a *App) upgradePlanAdminNotifyPost(c *request.Context, workspaceName string, userBasedData map[string][]*model.NotifyAdminData, featureBasedData map[model.MattermostFeature][]*model.NotifyAdminData, systemBot *model.Bot, admin *model.User, trial bool) { + props := make(model.StringInterface) + T := i18n.GetUserTranslations(admin.Locale) + + message := T("app.cloud.upgrade_plan_bot_message", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) + if len(userBasedData) == 1 { + message = T("app.cloud.upgrade_plan_bot_message_single", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) // todo (allan): investigate if translations library can do this + } + if trial { + message = T("app.cloud.trial_plan_bot_message", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) + if len(userBasedData) == 1 { + message = T("app.cloud.trial_plan_bot_message_single", map[string]interface{}{"UsersNum": len(userBasedData), "WorkspaceName": workspaceName}) + } + } + + channel, appErr := a.GetOrCreateDirectChannel(c, systemBot.UserId, admin.Id) + if appErr != nil { + a.Log().Warn("Error getting direct channel", mlog.Err(appErr)) + return + } + + post := &model.Post{ + Message: message, + UserId: systemBot.UserId, + ChannelId: channel.Id, + Type: fmt.Sprintf("%sup_notification", model.PostCustomTypePrefix), // webapp will have to create renderer for this custom post type + + } + + props["requested_features"] = featureBasedData + props["trial"] = trial + post.SetProps(props) + + _, appErr = a.CreatePost(c, post, channel, false, true) + + if appErr != nil { + a.Log().Warn("Error creating post", mlog.Err(appErr)) + } +} + +func (a *App) UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostFeature) bool { data, err := a.Srv().Store().NotifyAdmin().GetDataByUserIdAndFeature(user, feature) if err != nil { return false @@ -185,13 +237,13 @@ func (a *App) CanNotifyAdmin(trial bool) bool { if errors.As(sysValErr, &nfErr) { // if no timestamps have been recorded before, system is free to notify return true } - mlog.Error("Cannot notify", mlog.Err(sysValErr)) + a.Log().Error("Cannot notify", mlog.Err(sysValErr)) return false } lastNotificationTimestamp, err := strconv.ParseFloat(sysVal.Value, 64) if err != nil { - mlog.Error("Cannot notify", mlog.Err(err)) + a.Log().Error("Cannot notify", mlog.Err(err)) return false } @@ -205,7 +257,9 @@ func (a *App) CanNotifyAdmin(trial bool) bool { return timeDiff >= int64(daysToMillis) } -func (a *App) FinishSendAdminNotifyPost(trial bool, now int64) { +func (a *App) FinishSendAdminNotifyPost(trial bool, now int64, pluginBasedData map[string][]*model.NotifyAdminData) { + mlog.Info("FinishSendAdminNotifyPost") + systemVarName := lastUpgradeNotificationTimeStamp if trial { systemVarName = lastTrialNotificationTimeStamp @@ -214,30 +268,64 @@ func (a *App) FinishSendAdminNotifyPost(trial bool, now int64) { val := strconv.FormatInt(model.GetMillis(), 10) sysVar := &model.System{Name: systemVarName, Value: val} if err := a.Srv().Store().System().SaveOrUpdate(sysVar); err != nil { - mlog.Error("Unable to finish send admin notify post job", mlog.Err(err)) + a.Log().Error("Unable to finish send admin notify post job", mlog.Err(err)) + } + + // All the requested features notifications are now sent in a post and can safely be removed except + // the plugin notify admin. We keep it as we do not want the same user to send the notification for the same plugin. + // We update the NotifyAdmin SentAt to keep track of it. + for pluginId := range pluginBasedData { + notifications := pluginBasedData[pluginId] + for _, notification := range notifications { + requiredFeature := notification.RequiredFeature + requiredPlan := notification.RequiredPlan + userId := notification.UserId + if err := a.Srv().Store().NotifyAdmin().Update(userId, requiredPlan, requiredFeature, now); err != nil { + a.Log().Error("Unable to update SentAt for work template feature", mlog.Err(err)) + } + } } - // all the notifications are now sent in a post and can safely be removed if err := a.Srv().Store().NotifyAdmin().DeleteBefore(trial, now); err != nil { - mlog.Error("Unable to finish send admin notify post job", mlog.Err(err)) + a.Log().Error("Unable to finish send admin notify post job", mlog.Err(err)) } + mlog.Info("exit FinishSendAdminNotifyPost") } -func (a *App) groupNotifyAdminByUser(data []*model.NotifyAdminData) map[string][]*model.NotifyAdminData { - myMap := make(map[string][]*model.NotifyAdminData) +func (a *App) groupNotifyAdminByUser(data []*model.NotifyAdminData) (map[string][]*model.NotifyAdminData, map[string][]*model.NotifyAdminData) { + userBasedPaidFeatureData := make(map[string][]*model.NotifyAdminData) + userBasedPluginData := make(map[string][]*model.NotifyAdminData) for _, d := range data { - myMap[d.UserId] = append(myMap[d.UserId], d) + if strings.HasPrefix(string(d.RequiredFeature), string(model.PluginFeature)) { + userBasedPluginData[d.UserId] = append(userBasedPluginData[d.UserId], d) + } else { + userBasedPaidFeatureData[d.UserId] = append(userBasedPaidFeatureData[d.UserId], d) + } } - - return myMap + return userBasedPaidFeatureData, userBasedPluginData } -func (a *App) groupNotifyAdminByFeature(data []*model.NotifyAdminData) map[model.MattermostPaidFeature][]*model.NotifyAdminData { - myMap := make(map[model.MattermostPaidFeature][]*model.NotifyAdminData) +func (a *App) groupNotifyAdminByPaidFeature(data []*model.NotifyAdminData) map[model.MattermostFeature][]*model.NotifyAdminData { + myMap := make(map[model.MattermostFeature][]*model.NotifyAdminData) for _, d := range data { + if strings.HasPrefix(string(d.RequiredFeature), string(model.PluginFeature)) { + continue + } myMap[d.RequiredFeature] = append(myMap[d.RequiredFeature], d) } - + return myMap +} + +func (a *App) groupNotifyAdminByPlugin(data []*model.NotifyAdminData) map[string][]*model.NotifyAdminData { + myMap := make(map[string][]*model.NotifyAdminData) + for _, d := range data { + if strings.HasPrefix(string(d.RequiredFeature), string(model.PluginFeature)) { + plugins := strings.Split(d.RequiredPlan, ",") + for _, plugin := range plugins { + myMap[plugin] = append(myMap[plugin], d) + } + } + } return myMap } diff --git a/app/notify_admin_test.go b/app/notify_admin_test.go index c784622f38..49c7dac6e2 100644 --- a/app/notify_admin_test.go +++ b/app/notify_admin_test.go @@ -15,9 +15,11 @@ import ( "github.com/stretchr/testify/require" ) +const PluginIdGithub = "github" + func Test_SendNotifyAdminPosts(t *testing.T) { - t.Run("no error sending upgrade post when no notifications are available", func(t *testing.T) { + t.Run("no error sending non trial upgrade post when no notifications are available", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -28,7 +30,7 @@ func Test_SendNotifyAdminPosts(t *testing.T) { require.Nil(t, err) }) - t.Run("no error sending trial post when do notifications are available", func(t *testing.T) { + t.Run("no error sending trial upgrade post when no notifications are available", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -45,7 +47,7 @@ func Test_SendNotifyAdminPosts(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicense("cloud")) - // some some notifications + // some notifications _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ UserId: th.BasicUser.Id, RequiredPlan: model.LicenseShortSkuProfessional, @@ -93,13 +95,13 @@ func Test_SendNotifyAdminPosts(t *testing.T) { require.Equal(t, "2 members of the test workspace have requested a workspace upgrade for: ", post.Message) }) - t.Run("successfully send trial notification", func(t *testing.T) { + t.Run("successfully send trial upgrade notification", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() th.App.Srv().SetLicense(model.NewTestLicense("cloud")) - // some some notifications + // some notifications _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ UserId: th.BasicUser.Id, RequiredPlan: model.LicenseShortSkuProfessional, @@ -141,13 +143,128 @@ func Test_SendNotifyAdminPosts(t *testing.T) { require.Equal(t, "1 member of the test workspace has requested starting the Enterprise trial for access to: ", post.Message) }) - t.Run("error when trying to send post before end of cool off period", func(t *testing.T) { + t.Run("successfully send install plugin notification", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + // some notifications + _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ + UserId: th.BasicUser.Id, + RequiredPlan: PluginIdGithub, + RequiredFeature: model.PluginFeature, + Trial: false, + }) + require.Nil(t, appErr) + + ctx := request.NewContext(context.Background(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.Session{}, nil) + appErr = th.App.SendNotifyAdminPosts(ctx, "", "", false) + require.Nil(t, appErr) + + bot, appErr := th.App.GetSystemBot() + require.Nil(t, appErr) + + var channel *model.Channel + var err error + var timeout = 5 * time.Second + begin := time.Now() + + for { + if time.Since(begin) > timeout { + break + } + channel, err = th.App.Srv().Store().Channel().GetByName("", model.GetDMNameFromIds(bot.UserId, th.SystemAdminUser.Id), false) + if err == nil && channel != nil { + break + } + time.Sleep(100 * time.Millisecond) + } + require.NoError(t, err, "Expected message to have been sent within %d seconds", timeout) + postList, err := th.App.Srv().Store().Post().GetPosts(model.GetPostsOptions{ChannelId: channel.Id, Page: 0, PerPage: 1}, false, map[string]bool{}) + require.NoError(t, err) + + post := postList.Posts[postList.Order[0]] + + require.Equal(t, fmt.Sprintf("%spl_notification", model.PostCustomTypePrefix), post.Type) + require.Equal(t, bot.UserId, post.UserId) + }) + + t.Run("persist notify admin data after sending the install plugin notification", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + // some notifications + _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ + UserId: th.BasicUser.Id, + RequiredPlan: PluginIdGithub, + RequiredFeature: model.PluginFeature, + Trial: false, + }) + require.Nil(t, appErr) + + ctx := request.NewContext(context.Background(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.Session{}, nil) + appErr = th.App.SendNotifyAdminPosts(ctx, "", "", false) + require.Nil(t, appErr) + + bot, appErr := th.App.GetSystemBot() + require.Nil(t, appErr) + + var channel *model.Channel + var err error + var timeout = 5 * time.Second + begin := time.Now() + + for { + if time.Since(begin) > timeout { + break + } + channel, err = th.App.Srv().Store().Channel().GetByName("", model.GetDMNameFromIds(bot.UserId, th.SystemAdminUser.Id), false) + if err == nil && channel != nil { + break + } + time.Sleep(100 * time.Millisecond) + } + require.NoError(t, err, "Expected message to have been sent within %d seconds", timeout) + postList, err := th.App.Srv().Store().Post().GetPosts(model.GetPostsOptions{ChannelId: channel.Id, Page: 0, PerPage: 1}, false, map[string]bool{}) + require.NoError(t, err) + + post := postList.Posts[postList.Order[0]] + + require.Equal(t, fmt.Sprintf("%spl_notification", model.PostCustomTypePrefix), post.Type) + require.Equal(t, bot.UserId, post.UserId) + + data, err := th.App.Srv().Store().NotifyAdmin().GetDataByUserIdAndFeature(th.BasicUser.Id, model.PluginFeature) + require.NoError(t, err) + require.Equal(t, len(data), 1) + }) + + t.Run("error sending more than one notification to the same user and for the same plugin", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + err := th.App.SaveAdminNotification(th.BasicUser.Id, &model.NotifyAdminToUpgradeRequest{ + RequiredPlan: PluginIdGithub, + RequiredFeature: model.PluginFeature, + TrialNotification: false, + }) + + require.Nil(t, err) + + err = th.App.SaveAdminNotification(th.BasicUser.Id, &model.NotifyAdminToUpgradeRequest{ + RequiredPlan: PluginIdGithub, + RequiredFeature: model.PluginFeature, + TrialNotification: false, + }) + + require.Equal(t, err.Error(), "app.SaveAdminNotification: Already notified admin") + }) + + t.Run("error when trying to send upgrade post before end of cool off period", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() th.App.Srv().SetLicense(model.NewTestLicense("cloud")) - // some some notifications + // some notifications _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ UserId: th.BasicUser.Id, RequiredPlan: model.LicenseShortSkuProfessional, @@ -173,7 +290,7 @@ func Test_SendNotifyAdminPosts(t *testing.T) { require.Equal(t, appErr.Error(), "SendNotifyAdminPosts: Unable to send notification post., Cannot notify yet") }) - t.Run("can send post at the end of cool off period", func(t *testing.T) { + t.Run("can send upgrade post at the end of cool off period", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -182,7 +299,7 @@ func Test_SendNotifyAdminPosts(t *testing.T) { os.Setenv("MM_NOTIFY_ADMIN_COOL_OFF_DAYS", "0.00003472222222") // set to 3 seconds defer os.Unsetenv("MM_NOTIFY_ADMIN_COOL_OFF_DAYS") - // some some notifications + // some notifications _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ UserId: th.BasicUser.Id, RequiredPlan: model.LicenseShortSkuProfessional, @@ -215,7 +332,7 @@ func Test_SendNotifyAdminPosts(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicense("cloud")) - // some some notifications + // some notifications _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ UserId: th.BasicUser.Id, RequiredPlan: model.LicenseShortSkuProfessional, @@ -264,4 +381,68 @@ func Test_SendNotifyAdminPosts(t *testing.T) { require.Equal(t, bot.UserId, post.UserId) require.Equal(t, "1 member of the test workspace has requested a workspace upgrade for: ", post.Message) // expect only one member's notification even though 2 were added }) + + t.Run("correctly send upgrade and install plugin post with the correct user request", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + os.Setenv("MM_NOTIFY_ADMIN_COOL_OFF_DAYS", "0") + defer os.Unsetenv("MM_NOTIFY_ADMIN_COOL_OFF_DAYS") + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + ctx := request.NewContext(context.Background(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.NewId(), model.Session{}, nil) + + // some notifications + _, appErr := th.App.SaveAdminNotifyData(&model.NotifyAdminData{ + UserId: th.BasicUser.Id, + RequiredPlan: model.LicenseShortSkuProfessional, + RequiredFeature: model.PaidFeatureGuestAccounts, + Trial: false, + }) + require.Nil(t, appErr) + appErr = th.App.SendNotifyAdminPosts(ctx, "test", "", false) + require.Nil(t, appErr) + + // some notifications + _, appErr = th.App.SaveAdminNotifyData(&model.NotifyAdminData{ + UserId: th.BasicUser.Id, + RequiredPlan: PluginIdGithub, + RequiredFeature: model.PluginFeature, + Trial: false, + }) + require.Nil(t, appErr) + appErr = th.App.SendNotifyAdminPosts(ctx, "test", "", false) + require.Nil(t, appErr) + + bot, appErr := th.App.GetSystemBot() + require.Nil(t, appErr) + + var channel *model.Channel + var err error + var timeout = 5 * time.Second + begin := time.Now() + + for { + if time.Since(begin) > timeout { + break + } + channel, err = th.App.Srv().Store().Channel().GetByName("", model.GetDMNameFromIds(bot.UserId, th.SystemAdminUser.Id), false) + if err == nil && channel != nil { + break + } + time.Sleep(100 * time.Millisecond) + } + require.NoError(t, err, "Expected message to have been sent within %d seconds", timeout) + postList, err := th.App.Srv().Store().Post().GetPosts(model.GetPostsOptions{ChannelId: channel.Id, Page: 0, PerPage: 2}, false, map[string]bool{}) + require.NoError(t, err) + + installPluginPost := postList.Posts[postList.Order[0]] + require.Equal(t, fmt.Sprintf("%spl_notification", model.PostCustomTypePrefix), installPluginPost.Type) + require.Equal(t, bot.UserId, installPluginPost.UserId) + + upgradePost := postList.Posts[postList.Order[1]] + require.Equal(t, fmt.Sprintf("%sup_notification", model.PostCustomTypePrefix), upgradePost.Type) + require.Equal(t, bot.UserId, upgradePost.UserId) + require.Equal(t, "1 member of the test workspace has requested a workspace upgrade for: ", upgradePost.Message) + }) } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 300e205a52..b93056bc89 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -4430,7 +4430,7 @@ func (a *OpenTracingAppLayer) FindTeamByName(name string) bool { return resultVar0 } -func (a *OpenTracingAppLayer) FinishSendAdminNotifyPost(trial bool, now int64) { +func (a *OpenTracingAppLayer) FinishSendAdminNotifyPost(trial bool, now int64, pluginBasedData map[string][]*model.NotifyAdminData) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FinishSendAdminNotifyPost") @@ -4442,7 +4442,7 @@ func (a *OpenTracingAppLayer) FinishSendAdminNotifyPost(trial bool, now int64) { }() defer span.Finish() - a.app.FinishSendAdminNotifyPost(trial, now) + a.app.FinishSendAdminNotifyPost(trial, now, pluginBasedData) } func (a *OpenTracingAppLayer) GenerateMfaSecret(userID string) (*model.MfaSecret, *model.AppError) { @@ -18521,7 +18521,7 @@ func (a *OpenTracingAppLayer) UpsertGroupSyncable(groupSyncable *model.GroupSync return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostPaidFeature) bool { +func (a *OpenTracingAppLayer) UserAlreadyNotifiedOnRequiredFeature(user string, feature model.MattermostFeature) bool { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UserAlreadyNotifiedOnRequiredFeature") diff --git a/app/server.go b/app/server.go index e35574108e..41e85604b3 100644 --- a/app/server.go +++ b/app/server.go @@ -1543,6 +1543,12 @@ func (s *Server) initJobs() { notify_admin.MakeScheduler(s.Jobs, s.License(), model.JobTypeTrialNotifyAdmin), ) + s.Jobs.RegisterJobType( + model.JobTypeInstallPluginNotifyAdmin, + notify_admin.MakeInstallPluginNotifyWorker(s.Jobs, New(ServerConnector(s.Channels()))), + notify_admin.MakeInstallPluginScheduler(s.Jobs, s.License(), model.JobTypeInstallPluginNotifyAdmin), + ) + s.Jobs.RegisterJobType( model.JobTypeHostedPurchaseScreening, hosted_purchase_screening.MakeWorker(s.Jobs, s.License(), s.Store().System()), diff --git a/db/migrations/migrations.list b/db/migrations/migrations.list index ef1ba5f3e2..9c06d5b88c 100644 --- a/db/migrations/migrations.list +++ b/db/migrations/migrations.list @@ -204,6 +204,10 @@ db/migrations/mysql/000101_create_true_up_review_history.down.sql db/migrations/mysql/000101_create_true_up_review_history.up.sql db/migrations/mysql/000102_posts_originalid_index.down.sql db/migrations/mysql/000102_posts_originalid_index.up.sql +db/migrations/mysql/000103_add_sentat_to_notifyadmin.down.sql +db/migrations/mysql/000103_add_sentat_to_notifyadmin.up.sql +db/migrations/mysql/000104_upgrade_notifyadmin.down.sql +db/migrations/mysql/000104_upgrade_notifyadmin.up.sql db/migrations/postgres/000001_create_teams.down.sql db/migrations/postgres/000001_create_teams.up.sql db/migrations/postgres/000002_create_team_members.down.sql @@ -408,3 +412,7 @@ db/migrations/postgres/000101_create_true_up_review_history.down.sql db/migrations/postgres/000101_create_true_up_review_history.up.sql db/migrations/postgres/000102_posts_originalid_index.down.sql db/migrations/postgres/000102_posts_originalid_index.up.sql +db/migrations/postgres/000103_add_sentat_to_notifyadmin.down.sql +db/migrations/postgres/000103_add_sentat_to_notifyadmin.up.sql +db/migrations/postgres/000104_upgrade_notifyadmin.down.sql +db/migrations/postgres/000104_upgrade_notifyadmin.up.sql diff --git a/db/migrations/mysql/000103_add_sentat_to_notifyadmin.down.sql b/db/migrations/mysql/000103_add_sentat_to_notifyadmin.down.sql new file mode 100644 index 0000000000..371c98478c --- /dev/null +++ b/db/migrations/mysql/000103_add_sentat_to_notifyadmin.down.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'SentAt' + ) > 0, + 'ALTER TABLE NotifyAdmin DROP COLUMN SentAt;', + 'SELECT 1;' +)); + +PREPARE removeColumnIfExists FROM @preparedStatement; +EXECUTE removeColumnIfExists; +DEALLOCATE PREPARE removeColumnIfExists; diff --git a/db/migrations/mysql/000103_add_sentat_to_notifyadmin.up.sql b/db/migrations/mysql/000103_add_sentat_to_notifyadmin.up.sql new file mode 100644 index 0000000000..b6bfdb2b4c --- /dev/null +++ b/db/migrations/mysql/000103_add_sentat_to_notifyadmin.up.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + NOT EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'SentAt' + ), + 'ALTER TABLE NotifyAdmin ADD COLUMN SentAt bigint DEFAULT NULL;', + 'SELECT 1;' +)); + +PREPARE addColumnIfNotExists FROM @preparedStatement; +EXECUTE addColumnIfNotExists; +DEALLOCATE PREPARE addColumnIfNotExists; diff --git a/db/migrations/mysql/000104_upgrade_notifyadmin.down.sql b/db/migrations/mysql/000104_upgrade_notifyadmin.down.sql new file mode 100644 index 0000000000..e9583db7b8 --- /dev/null +++ b/db/migrations/mysql/000104_upgrade_notifyadmin.down.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'RequiredFeature' + AND column_type != 'varchar(100)' + ) > 0, + 'ALTER TABLE NotifyAdmin MODIFY COLUMN RequiredFeature varchar(100);', + 'SELECT 1' +)); +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; + +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'RequiredPlan' + AND column_type != 'varchar(26)' + ) > 0, + 'ALTER TABLE NotifyAdmin MODIFY COLUMN RequiredPlan varchar(26);', + 'SELECT 1' +)); +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; diff --git a/db/migrations/mysql/000104_upgrade_notifyadmin.up.sql b/db/migrations/mysql/000104_upgrade_notifyadmin.up.sql new file mode 100644 index 0000000000..25cc1d598e --- /dev/null +++ b/db/migrations/mysql/000104_upgrade_notifyadmin.up.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'RequiredFeature' + AND column_type != 'varchar(255)' + ) > 0, + 'ALTER TABLE NotifyAdmin MODIFY COLUMN RequiredFeature varchar(255);', + 'SELECT 1' +)); +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; + +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'NotifyAdmin' + AND table_schema = DATABASE() + AND column_name = 'RequiredPlan' + AND column_type != 'varchar(100)' + ) > 0, + 'ALTER TABLE NotifyAdmin MODIFY COLUMN RequiredPlan varchar(100);', + 'SELECT 1' +)); +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; diff --git a/db/migrations/postgres/000103_add_sentat_to_notifyadmin.down.sql b/db/migrations/postgres/000103_add_sentat_to_notifyadmin.down.sql new file mode 100644 index 0000000000..1cb28a0ee3 --- /dev/null +++ b/db/migrations/postgres/000103_add_sentat_to_notifyadmin.down.sql @@ -0,0 +1 @@ +ALTER TABLE NotifyAdmin DROP COLUMN IF EXISTS SentAt; diff --git a/db/migrations/postgres/000103_add_sentat_to_notifyadmin.up.sql b/db/migrations/postgres/000103_add_sentat_to_notifyadmin.up.sql new file mode 100644 index 0000000000..2e5f7db83d --- /dev/null +++ b/db/migrations/postgres/000103_add_sentat_to_notifyadmin.up.sql @@ -0,0 +1 @@ +ALTER TABLE NotifyAdmin ADD COLUMN IF NOT EXISTS SentAt bigint DEFAULT NULL; diff --git a/db/migrations/postgres/000104_upgrade_notifyadmin.down.sql b/db/migrations/postgres/000104_upgrade_notifyadmin.down.sql new file mode 100644 index 0000000000..438d3eb8bf --- /dev/null +++ b/db/migrations/postgres/000104_upgrade_notifyadmin.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE NotifyAdmin ALTER COLUMN RequiredFeature TYPE VARCHAR(100); +ALTER TABLE NotifyAdmin ALTER COLUMN RequiredPlan TYPE VARCHAR(26); diff --git a/db/migrations/postgres/000104_upgrade_notifyadmin.up.sql b/db/migrations/postgres/000104_upgrade_notifyadmin.up.sql new file mode 100644 index 0000000000..8e281eacf4 --- /dev/null +++ b/db/migrations/postgres/000104_upgrade_notifyadmin.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE NotifyAdmin ALTER COLUMN RequiredFeature TYPE VARCHAR(255); +ALTER TABLE NotifyAdmin ALTER COLUMN RequiredPlan TYPE VARCHAR(100); diff --git a/jobs/notify_admin/install_plugin_scheduler.go b/jobs/notify_admin/install_plugin_scheduler.go new file mode 100644 index 0000000000..e1e8801a6d --- /dev/null +++ b/jobs/notify_admin/install_plugin_scheduler.go @@ -0,0 +1,25 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package notify_admin + +import ( + "strconv" + "time" + + "github.com/mattermost/mattermost-server/v6/jobs" + "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/shared/mlog" +) + +const installPluginSchedFreq = 1 * time.Minute + +func MakeInstallPluginScheduler(jobServer *jobs.JobServer, license *model.License, jobType string) model.Scheduler { + isEnabled := func(cfg *model.Config) bool { + enabled := jobType == model.JobTypeInstallPluginNotifyAdmin + mlog.Debug("Scheduler: isEnabled: "+strconv.FormatBool(enabled), mlog.String("scheduler", jobType)) + return enabled + } + return jobs.NewPeriodicScheduler(jobServer, jobType, installPluginSchedFreq, isEnabled) + +} diff --git a/jobs/notify_admin/worker.go b/jobs/notify_admin/worker.go index 363e7628aa..f9eb8dc4e9 100644 --- a/jobs/notify_admin/worker.go +++ b/jobs/notify_admin/worker.go @@ -11,6 +11,7 @@ import ( const ( UpgradeNotifyJobName = "UpgradeNotifyAdmin" TrialNotifyJobName = "TrialNotifyAdmin" + InstallNotifyJobName = "InstallNotifyAdmin" ) type AppIface interface { @@ -52,3 +53,21 @@ func MakeTrialNotifyWorker(jobServer *jobs.JobServer, license *model.License, ap worker := jobs.NewSimpleWorker(TrialNotifyJobName, jobServer, execute, isEnabled) return worker } + +func MakeInstallPluginNotifyWorker(jobServer *jobs.JobServer, app AppIface) model.Worker { + isEnabled := func(_ *model.Config) bool { + return true + } + execute := func(job *model.Job) error { + defer jobServer.HandleJobPanic(job) + + appErr := app.DoCheckForAdminNotifications(false) + if appErr != nil { + return appErr + } + + return nil + } + worker := jobs.NewSimpleWorker(InstallNotifyJobName, jobServer, execute, isEnabled) + return worker +} diff --git a/model/job.go b/model/job.go index c4dd9012f7..a9c4c6f4a0 100644 --- a/model/job.go +++ b/model/job.go @@ -31,6 +31,7 @@ const ( JobTypeLastAccessibleFile = "last_accessible_file" JobTypeUpgradeNotifyAdmin = "upgrade_notify_admin" JobTypeTrialNotifyAdmin = "trial_notify_admin" + JobTypeInstallPluginNotifyAdmin = "install_plugin_notify_admin" JobTypeHostedPurchaseScreening = "hosted_purchase_screening" JobStatusPending = "pending" diff --git a/model/notify_admin.go b/model/notify_admin.go index c4840ab24f..868385fe83 100644 --- a/model/notify_admin.go +++ b/model/notify_admin.go @@ -4,23 +4,26 @@ package model import ( + "database/sql" "fmt" "net/http" + "strings" ) -type MattermostPaidFeature string +type MattermostFeature string const ( - PaidFeatureGuestAccounts = MattermostPaidFeature("mattermost.feature.guest_accounts") - PaidFeatureCustomUsergroups = MattermostPaidFeature("mattermost.feature.custom_user_groups") - PaidFeatureCreateMultipleTeams = MattermostPaidFeature("mattermost.feature.create_multiple_teams") - PaidFeatureStartcall = MattermostPaidFeature("mattermost.feature.start_call") - PaidFeaturePlaybooksRetrospective = MattermostPaidFeature("mattermost.feature.playbooks_retro") - PaidFeatureUnlimitedMessages = MattermostPaidFeature("mattermost.feature.unlimited_messages") - PaidFeatureUnlimitedFileStorage = MattermostPaidFeature("mattermost.feature.unlimited_file_storage") - PaidFeatureAllProfessionalfeatures = MattermostPaidFeature("mattermost.feature.all_professional") - PaidFeatureAllEnterprisefeatures = MattermostPaidFeature("mattermost.feature.all_enterprise") - UpgradeDowngradedWorkspace = MattermostPaidFeature("mattermost.feature.upgrade_downgraded_workspace") + PaidFeatureGuestAccounts = MattermostFeature("mattermost.feature.guest_accounts") + PaidFeatureCustomUsergroups = MattermostFeature("mattermost.feature.custom_user_groups") + PaidFeatureCreateMultipleTeams = MattermostFeature("mattermost.feature.create_multiple_teams") + PaidFeatureStartcall = MattermostFeature("mattermost.feature.start_call") + PaidFeaturePlaybooksRetrospective = MattermostFeature("mattermost.feature.playbooks_retro") + PaidFeatureUnlimitedMessages = MattermostFeature("mattermost.feature.unlimited_messages") + PaidFeatureUnlimitedFileStorage = MattermostFeature("mattermost.feature.unlimited_file_storage") + PaidFeatureAllProfessionalfeatures = MattermostFeature("mattermost.feature.all_professional") + PaidFeatureAllEnterprisefeatures = MattermostFeature("mattermost.feature.all_enterprise") + UpgradeDowngradedWorkspace = MattermostFeature("mattermost.feature.upgrade_downgraded_workspace") + PluginFeature = MattermostFeature("mattermost.feature.plugin") ) var validSKUs map[string]struct{} = map[string]struct{}{ @@ -29,7 +32,7 @@ var validSKUs map[string]struct{} = map[string]struct{}{ } // These are the features a non admin would typically ping an admin about -var paidFeatures map[MattermostPaidFeature]struct{} = map[MattermostPaidFeature]struct{}{ +var paidFeatures map[MattermostFeature]struct{} = map[MattermostFeature]struct{}{ PaidFeatureGuestAccounts: {}, PaidFeatureCustomUsergroups: {}, PaidFeatureCreateMultipleTeams: {}, @@ -43,20 +46,24 @@ var paidFeatures map[MattermostPaidFeature]struct{} = map[MattermostPaidFeature] } type NotifyAdminToUpgradeRequest struct { - TrialNotification bool `json:"trial_notification"` - RequiredPlan string `json:"required_plan"` - RequiredFeature MattermostPaidFeature `json:"required_feature"` + TrialNotification bool `json:"trial_notification"` + RequiredPlan string `json:"required_plan"` + RequiredFeature MattermostFeature `json:"required_feature"` } type NotifyAdminData struct { - CreateAt int64 `json:"create_at,omitempty"` - UserId string `json:"user_id"` - RequiredPlan string `json:"required_plan"` - RequiredFeature MattermostPaidFeature `json:"required_feature"` - Trial bool `json:"trial"` + CreateAt int64 `json:"create_at,omitempty"` + UserId string `json:"user_id"` + RequiredPlan string `json:"required_plan"` + RequiredFeature MattermostFeature `json:"required_feature"` + Trial bool `json:"trial"` + SentAt sql.NullInt64 `json:"sent_at"` } func (nad *NotifyAdminData) IsValid() *AppError { + if strings.HasPrefix(string(nad.RequiredFeature), string(PluginFeature)) { + return nil + } if _, planOk := validSKUs[nad.RequiredPlan]; !planOk { return NewAppError("NotifyAdmin.IsValid", fmt.Sprintf("Invalid plan, %s provided", nad.RequiredPlan), nil, "", http.StatusBadRequest) } diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 5b6ce59fb0..0486a0e350 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -5249,7 +5249,7 @@ func (s *OpenTracingLayerNotifyAdminStore) Get(trial bool) ([]*model.NotifyAdmin return result, err } -func (s *OpenTracingLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) { +func (s *OpenTracingLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "NotifyAdminStore.GetDataByUserIdAndFeature") s.Root.Store.SetContext(newCtx) @@ -5285,6 +5285,24 @@ func (s *OpenTracingLayerNotifyAdminStore) Save(data *model.NotifyAdminData) (*m return result, err } +func (s *OpenTracingLayerNotifyAdminStore) Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "NotifyAdminStore.Update") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.NotifyAdminStore.Update(userId, requiredPlan, requiredFeature, now) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerOAuthStore) DeleteApp(id string) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "OAuthStore.DeleteApp") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 45e8bd1bbc..333caea8db 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -5941,7 +5941,7 @@ func (s *RetryLayerNotifyAdminStore) Get(trial bool) ([]*model.NotifyAdminData, } -func (s *RetryLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) { +func (s *RetryLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) { tries := 0 for { @@ -5983,6 +5983,27 @@ func (s *RetryLayerNotifyAdminStore) Save(data *model.NotifyAdminData) (*model.N } +func (s *RetryLayerNotifyAdminStore) Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error { + + tries := 0 + for { + err := s.NotifyAdminStore.Update(userId, requiredPlan, requiredFeature, now) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerOAuthStore) DeleteApp(id string) error { tries := 0 diff --git a/store/sqlstore/notify_admin_store.go b/store/sqlstore/notify_admin_store.go index e08337e746..e1046cfbc3 100644 --- a/store/sqlstore/notify_admin_store.go +++ b/store/sqlstore/notify_admin_store.go @@ -42,7 +42,7 @@ func (s SqlNotifyAdminStore) Save(data *model.NotifyAdminData) (*model.NotifyAdm return data, nil } -func (s SqlNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) { +func (s SqlNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) { data := []*model.NotifyAdminData{} query, args, err := s.getQueryBuilder(). Select("*"). @@ -50,7 +50,7 @@ func (s SqlNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature mo Where(sq.Eq{"UserId": userId, "RequiredFeature": feature}). ToSql() if err != nil { - return nil, errors.Wrap(err, "could not build sql query to get all notifcation data by user id and required feature") + return nil, errors.Wrap(err, "could not build sql query to get all notification data by user id and required feature") } if err := s.GetReplicaX().Select(&data, query, args...); err != nil { @@ -67,7 +67,8 @@ func (s SqlNotifyAdminStore) Get(trial bool) ([]*model.NotifyAdminData, error) { query, args, err := s.getQueryBuilder(). Select("*"). From("NotifyAdmin"). - Where(sq.Eq{"trial": trial}). + Where(sq.Eq{"Trial": trial}). + Where("(SentAt IS NULL)"). ToSql() if err != nil { return nil, errors.Wrap(err, "could not build sql query to get all notifcation data") @@ -80,8 +81,15 @@ func (s SqlNotifyAdminStore) Get(trial bool) ([]*model.NotifyAdminData, error) { } func (s SqlNotifyAdminStore) DeleteBefore(trial bool, now int64) error { - if _, err := s.GetMasterX().Exec("DELETE FROM NotifyAdmin WHERE trial = ? AND createat < ?", trial, now); err != nil { + if _, err := s.GetMasterX().Exec("DELETE FROM NotifyAdmin WHERE Trial = ? AND CreateAt < ? AND SentAt IS NULL", trial, now); err != nil { return errors.Wrapf(err, "failed to remove all notification data with trial=%t", trial) } return nil } + +func (s SqlNotifyAdminStore) Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error { + if _, err := s.GetMasterX().Exec("UPDATE NotifyAdmin SET SentAt = ? WHERE UserId = ? AND RequiredPlan = ? AND RequiredFeature = ?", now, userId, requiredPlan, requiredFeature); err != nil { + return errors.Wrapf(err, "failed to update SentAt for userId=%s and requiredPlan=%s", userId, requiredPlan) + } + return nil +} diff --git a/store/store.go b/store/store.go index 01a74ae943..bfebe49e19 100644 --- a/store/store.go +++ b/store/store.go @@ -942,9 +942,10 @@ type LinkMetadataStore interface { type NotifyAdminStore interface { Save(data *model.NotifyAdminData) (*model.NotifyAdminData, error) - GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) + GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) Get(trial bool) ([]*model.NotifyAdminData, error) DeleteBefore(trial bool, now int64) error + Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error } type SharedChannelStore interface { diff --git a/store/storetest/mocks/NotifyAdminStore.go b/store/storetest/mocks/NotifyAdminStore.go index 4a3529103a..dbada2f895 100644 --- a/store/storetest/mocks/NotifyAdminStore.go +++ b/store/storetest/mocks/NotifyAdminStore.go @@ -52,11 +52,11 @@ func (_m *NotifyAdminStore) Get(trial bool) ([]*model.NotifyAdminData, error) { } // GetDataByUserIdAndFeature provides a mock function with given fields: userId, feature -func (_m *NotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) { +func (_m *NotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) { ret := _m.Called(userId, feature) var r0 []*model.NotifyAdminData - if rf, ok := ret.Get(0).(func(string, model.MattermostPaidFeature) []*model.NotifyAdminData); ok { + if rf, ok := ret.Get(0).(func(string, model.MattermostFeature) []*model.NotifyAdminData); ok { r0 = rf(userId, feature) } else { if ret.Get(0) != nil { @@ -65,7 +65,7 @@ func (_m *NotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature mod } var r1 error - if rf, ok := ret.Get(1).(func(string, model.MattermostPaidFeature) error); ok { + if rf, ok := ret.Get(1).(func(string, model.MattermostFeature) error); ok { r1 = rf(userId, feature) } else { r1 = ret.Error(1) @@ -96,3 +96,17 @@ func (_m *NotifyAdminStore) Save(data *model.NotifyAdminData) (*model.NotifyAdmi return r0, r1 } + +// Update provides a mock function with given fields: userId, requiredPlan, requiredFeature, now +func (_m *NotifyAdminStore) Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error { + ret := _m.Called(userId, requiredPlan, requiredFeature, now) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string, model.MattermostFeature, int64) error); ok { + r0 = rf(userId, requiredPlan, requiredFeature, now) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/store/storetest/notify_admin_store.go b/store/storetest/notify_admin_store.go index 2c6d19d3e8..fe0157c9fc 100644 --- a/store/storetest/notify_admin_store.go +++ b/store/storetest/notify_admin_store.go @@ -4,6 +4,7 @@ package storetest import ( + "database/sql" "testing" "github.com/mattermost/mattermost-server/v6/model" @@ -11,11 +12,14 @@ import ( "github.com/stretchr/testify/require" ) +const PluginIdJenkins = "jenkins" + func TestNotifyAdminStore(t *testing.T, ss store.Store) { t.Run("Save", func(t *testing.T) { testNotifyAdminStoreSave(t, ss) }) t.Run("testGetDataByUserIdAndFeature", func(t *testing.T) { testGetDataByUserIdAndFeature(t, ss) }) t.Run("testGet", func(t *testing.T) { testGet(t, ss) }) t.Run("testDeleteBefore", func(t *testing.T) { testDeleteBefore(t, ss) }) + t.Run("testUpdate", func(t *testing.T) { testUpdate(t, ss) }) } func tearDown(t *testing.T, ss store.Store) { @@ -145,6 +149,28 @@ func testGetDataByUserIdAndFeature(t *testing.T, ss store.Store) { tearDown(t, ss) } +func testUpdate(t *testing.T, ss store.Store) { + userId1 := model.NewId() + d1 := &model.NotifyAdminData{ + UserId: userId1, + RequiredPlan: PluginIdJenkins, + RequiredFeature: model.PluginFeature, + } + _, err := ss.NotifyAdmin().Save(d1) + require.NoError(t, err) + + err = ss.NotifyAdmin().Update(d1.UserId, d1.RequiredPlan, d1.RequiredFeature, 100) + require.NoError(t, err) + + userRequest, err := ss.NotifyAdmin().GetDataByUserIdAndFeature(d1.UserId, d1.RequiredFeature) + require.NoError(t, err) + + require.Equal(t, len(userRequest), 1) + require.Equal(t, userRequest[0].SentAt, sql.NullInt64{Int64: 100, Valid: true}) + + tearDown(t, ss) +} + func testDeleteBefore(t *testing.T, ss store.Store) { userId1 := model.NewId() d1 := &model.NotifyAdminData{ diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index eb9c4d4bbf..df44340752 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -4764,7 +4764,7 @@ func (s *TimerLayerNotifyAdminStore) Get(trial bool) ([]*model.NotifyAdminData, return result, err } -func (s *TimerLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostPaidFeature) ([]*model.NotifyAdminData, error) { +func (s *TimerLayerNotifyAdminStore) GetDataByUserIdAndFeature(userId string, feature model.MattermostFeature) ([]*model.NotifyAdminData, error) { start := time.Now() result, err := s.NotifyAdminStore.GetDataByUserIdAndFeature(userId, feature) @@ -4796,6 +4796,22 @@ func (s *TimerLayerNotifyAdminStore) Save(data *model.NotifyAdminData) (*model.N return result, err } +func (s *TimerLayerNotifyAdminStore) Update(userId string, requiredPlan string, requiredFeature model.MattermostFeature, now int64) error { + start := time.Now() + + err := s.NotifyAdminStore.Update(userId, requiredPlan, requiredFeature, now) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("NotifyAdminStore.Update", success, elapsed) + } + return err +} + func (s *TimerLayerOAuthStore) DeleteApp(id string) error { start := time.Now()