MM-28835 Configure/update notices setting for TeamAdmin (#15632)

Makefile

@@ -49,11 +49,6 @@ else
 	BUILD_CLIENT = false
 endif
 
-# these variables are used by QA to override location of InProduct Notices
-NOTICES_JSON_URL ?= https://notices.mattermost.com/
-NOTICES_FETCH_SECS ?= 3600
-NOTICES_SKIP_CACHE ?= false
-
 # Go Flags
 GOFLAGS ?= $(GOFLAGS:)
 # We need to export GOBIN to allow it to be set
@@ -66,9 +61,6 @@ LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/model.BuildDate=$(BUIL
 LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/model.BuildHash=$(BUILD_HASH)"
 LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/model.BuildHashEnterprise=$(BUILD_HASH_ENTERPRISE)"
 LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/model.BuildEnterpriseReady=$(BUILD_ENTERPRISE_READY)"
-LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/app.NOTICES_JSON_URL=$(NOTICES_JSON_URL)"
-LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/app.NOTICES_JSON_FETCH_FREQUENCY_SECONDS=$(NOTICES_FETCH_SECS)"
-LDFLAGS += -X "github.com/mattermost/mattermost-server/v5/app.NOTICES_SKIP_CACHE=$(NOTICES_SKIP_CACHE)"
 
 GO_MAJOR_VERSION = $(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'.' -f1)
 GO_MINOR_VERSION = $(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'.' -f2)

app/product_notices.go

@@ -6,33 +6,23 @@ package app
 import (
 	"net/http"
 	"reflect"
-	"strconv"
 	"strings"
 	"time"
 
-	"github.com/mattermost/mattermost-server/v5/store"
-
-	"github.com/Masterminds/semver/v3"
 	"github.com/mattermost/mattermost-server/v5/config"
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 	"github.com/mattermost/mattermost-server/v5/utils"
 	"github.com/pkg/errors"
+
+	"github.com/Masterminds/semver/v3"
 	date_constraints "github.com/reflog/dateconstraints"
 )
 
 const MAX_REPEAT_VIEWINGS = 3
 const MIN_SECONDS_BETWEEN_REPEAT_VIEWINGS = 60 * 60
 
-// where to fetch notices from. setting as var to allow overriding during build/test
-var NOTICES_JSON_URL = "https://notices.mattermost.com/"
-
-// notice.json fetch frequency in seconds. setting as var to allow overriding during build/test
-var NOTICES_JSON_FETCH_FREQUENCY_SECONDS = "3600" // one hour by default
-
-// this variable can be set during build time for QA to skip caching JSON responses (to avoid CDN delay)
-var NOTICES_SKIP_CACHE = "false"
-
 // http request cache
 var noticesCache = utils.RequestCache{}
 
@@ -65,9 +55,9 @@ func noticeMatchesConditions(config *model.Config, preferences store.PreferenceS
 	}
 
 	for _, v := range clientVersions {
-		c, err := semver.NewConstraint(v)
+		c, err2 := semver.NewConstraint(v)
-		if err != nil {
+		if err2 != nil {
-			return false, errors.Wrapf(err, "Cannot parse version range %s", v)
+			return false, errors.Wrapf(err2, "Cannot parse version range %s", v)
 		}
 		if !c.Check(clientVersionParsed) {
 			return false, nil
@@ -77,9 +67,9 @@ func noticeMatchesConditions(config *model.Config, preferences store.PreferenceS
 	// check if notice date range matches current
 	if cnd.DisplayDate != nil {
 		now := time.Now().UTC()
-		c, err := date_constraints.NewConstraint(*cnd.DisplayDate)
+		c, err2 := date_constraints.NewConstraint(*cnd.DisplayDate)
-		if err != nil {
+		if err2 != nil {
-			return false, errors.Wrapf(err, "Cannot parse date range %s", *cnd.DisplayDate)
+			return false, errors.Wrapf(err2, "Cannot parse date range %s", *cnd.DisplayDate)
 		}
 		if !c.Check(&now) {
 			return false, nil
@@ -87,14 +77,18 @@ func noticeMatchesConditions(config *model.Config, preferences store.PreferenceS
 	}
 
 	// check if current server version is notice range
-	serverVersion, _ := semver.NewVersion(model.BuildNumber)
+	serverVersion, err := semver.NewVersion(model.BuildNumber)
-	for _, v := range cnd.ServerVersion {
+	if err != nil {
-		c, err := semver.NewConstraint(v)
+		mlog.Warn("Skipping server version check, build number is not in semver format", mlog.String("build_number", model.BuildNumber))
-		if err != nil {
+	} else {
-			return false, errors.Wrapf(err, "Cannot parse version range %s", v)
+		for _, v := range cnd.ServerVersion {
-		}
+			c, err := semver.NewConstraint(v)
-		if !c.Check(serverVersion) {
+			if err != nil {
-			return false, nil
+				return false, errors.Wrapf(err, "Cannot parse version range %s", v)
+			}
+			if !c.Check(serverVersion) {
+				return false, nil
+			}
 		}
 	}
 
@@ -191,7 +185,7 @@ func (a *App) GetProductNotices(userId, teamId string, client model.NoticeClient
 	isTeamAdmin := a.SessionHasPermissionToTeam(*a.Session(), teamId, model.PERMISSION_MANAGE_TEAM)
 
 	// check if notices for regular users are disabled
-	if !*a.Srv().Config().AnnouncementSettings.UserNoticesEnabled && !isTeamAdmin && !isSystemAdmin {
+	if !*a.Srv().Config().AnnouncementSettings.UserNoticesEnabled && !isSystemAdmin {
 		return []model.NoticeMessage{}, nil
 	}
 
@@ -269,12 +263,11 @@ func (a *App) UpdateViewedProductNotices(userId string, noticeIds []string) *mod
 }
 
 func (a *App) UpdateProductNotices() *model.AppError {
-	skip, err := strconv.ParseBool(NOTICES_SKIP_CACHE)
+	url := *a.Srv().Config().AnnouncementSettings.NoticesURL
-	if err != nil {
+	skip := *a.Srv().Config().AnnouncementSettings.NoticesSkipCache
-		skip = false
+	mlog.Debug("Will fetch notices from", mlog.String("url", url), mlog.Bool("skip_cache", skip))
-	}
-	mlog.Debug("Will fetch notices from", mlog.String("url", NOTICES_JSON_URL), mlog.Bool("skip_cache", skip))
 	var appErr *model.AppError
+	var err error
 	cachedPostCount, err = a.Srv().Store.Post().AnalyticsPostCount("", false, false)
 	if err != nil {
 		mlog.Error("Failed to fetch post count", mlog.String("error", err.Error()))
@@ -285,7 +278,7 @@ func (a *App) UpdateProductNotices() *model.AppError {
 		mlog.Error("Failed to fetch user count", mlog.String("error", appErr.Error()))
 	}
 
-	data, err := utils.GetUrlWithCache(NOTICES_JSON_URL, &noticesCache, skip)
+	data, err := utils.GetUrlWithCache(url, &noticesCache, skip)
 	if err != nil {
 		return model.NewAppError("UpdateProductNotices", "api.system.update_notices.fetch_failed", nil, err.Error(), http.StatusBadRequest)
 	}

app/product_notices_test.go

@@ -449,10 +449,6 @@ func TestNoticeValidation(t *testing.T) {
 func TestNoticeFetch(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
-	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.AnnouncementSettings.AdminNoticesEnabled = true
-		*cfg.AnnouncementSettings.UserNoticesEnabled = true
-	})
 
 	notices := model.ProductNotices{model.ProductNotice{
 		Conditions: model.Conditions{},
@@ -491,8 +487,11 @@ func TestNoticeFetch(t *testing.T) {
 		}
 	}))
 	defer server1.Close()
-
+	th.App.UpdateConfig(func(cfg *model.Config) {
-	NOTICES_JSON_URL = fmt.Sprintf("http://%s/notices.json", server1.Listener.Addr().String())
+		*cfg.AnnouncementSettings.AdminNoticesEnabled = true
+		*cfg.AnnouncementSettings.UserNoticesEnabled = true
+		*cfg.AnnouncementSettings.NoticesURL = fmt.Sprintf("http://%s/notices.json", server1.Listener.Addr().String())
+	})
 
 	// fetch fake notices
 	appErr = th.App.UpdateProductNotices()
@@ -518,7 +517,9 @@ func TestNoticeFetch(t *testing.T) {
 	require.Len(t, views, 1)
 
 	// fetch another set
-	NOTICES_JSON_URL = fmt.Sprintf("http://%s/notices2.json", server1.Listener.Addr().String())
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.AnnouncementSettings.NoticesURL = fmt.Sprintf("http://%s/notices2.json", server1.Listener.Addr().String())
+	})
 
 	// fetch fake notices
 	appErr = th.App.UpdateProductNotices()

jobs/product_notices/scheduler.go

@@ -4,8 +4,6 @@
 package product_notices
 
 import (
-	"github.com/mattermost/mattermost-server/v5/mlog"
-	"strconv"
 	"time"
 
 	"github.com/mattermost/mattermost-server/v5/app"
@@ -34,12 +32,7 @@ func (scheduler *Scheduler) Enabled(cfg *model.Config) bool {
 }
 
 func (scheduler *Scheduler) NextScheduleTime(cfg *model.Config, now time.Time, pendingJobs bool, lastSuccessfulJob *model.Job) *time.Time {
-	freq, err := strconv.ParseInt(app.NOTICES_JSON_FETCH_FREQUENCY_SECONDS, 10, 32)
+	nextTime := time.Now().Add(time.Duration(*cfg.AnnouncementSettings.NoticesFetchFrequency) * time.Second)
-	if err != nil {
-		mlog.Debug("Invalid NOTICES_JSON_FETCH_FREQUENCY_SECONDS variable provided!", mlog.String("value", app.NOTICES_JSON_FETCH_FREQUENCY_SECONDS))
-		freq = 3600
-	}
-	nextTime := time.Now().Add(time.Duration(freq) * time.Second)
 	return &nextTime
 }
 

model/config.go

@@ -161,8 +161,10 @@ const (
 
 	ANALYTICS_SETTINGS_DEFAULT_MAX_USERS_FOR_STATISTICS = 2500
 
-	ANNOUNCEMENT_SETTINGS_DEFAULT_BANNER_COLOR      = "#f2a93b"
+	ANNOUNCEMENT_SETTINGS_DEFAULT_BANNER_COLOR                    = "#f2a93b"
-	ANNOUNCEMENT_SETTINGS_DEFAULT_BANNER_TEXT_COLOR = "#333333"
+	ANNOUNCEMENT_SETTINGS_DEFAULT_BANNER_TEXT_COLOR               = "#333333"
+	ANNOUNCEMENT_SETTINGS_DEFAULT_NOTICES_JSON_URL                = "https://notices.mattermost.com/"
+	ANNOUNCEMENT_SETTINGS_DEFAULT_NOTICES_FETCH_FREQUENCY_SECONDS = 3600
 
 	TEAM_SETTINGS_DEFAULT_TEAM_TEXT = "default"
 
@@ -1667,13 +1669,16 @@ func (s *SupportSettings) SetDefaults() {
 }
 
 type AnnouncementSettings struct {
-	EnableBanner         *bool   `access:"site"`
+	EnableBanner          *bool   `access:"site"`
-	BannerText           *string `access:"site"`
+	BannerText            *string `access:"site"`
-	BannerColor          *string `access:"site"`
+	BannerColor           *string `access:"site"`
-	BannerTextColor      *string `access:"site"`
+	BannerTextColor       *string `access:"site"`
-	AllowBannerDismissal *bool   `access:"site"`
+	AllowBannerDismissal  *bool   `access:"site"`
-	AdminNoticesEnabled  *bool   `access:"site"`
+	AdminNoticesEnabled   *bool   `access:"site"`
-	UserNoticesEnabled   *bool   `access:"site"`
+	UserNoticesEnabled    *bool   `access:"site"`
+	NoticesURL            *string `access:"site,write_restrictable"`
+	NoticesFetchFrequency *int    `access:"site,write_restrictable"`
+	NoticesSkipCache      *bool   `access:"site,write_restrictable"`
 }
 
 func (s *AnnouncementSettings) SetDefaults() {
@@ -1704,6 +1709,16 @@ func (s *AnnouncementSettings) SetDefaults() {
 	if s.UserNoticesEnabled == nil {
 		s.UserNoticesEnabled = NewBool(true)
 	}
+	if s.NoticesURL == nil {
+		s.NoticesURL = NewString(ANNOUNCEMENT_SETTINGS_DEFAULT_NOTICES_JSON_URL)
+	}
+	if s.NoticesSkipCache == nil {
+		s.NoticesSkipCache = NewBool(false)
+	}
+	if s.NoticesFetchFrequency == nil {
+		s.NoticesFetchFrequency = NewInt(ANNOUNCEMENT_SETTINGS_DEFAULT_NOTICES_FETCH_FREQUENCY_SECONDS)
+	}
+
 }
 
 type ThemeSettings struct {