diff --git a/Makefile b/Makefile index 02eec43722..bf65ba7421 100644 --- a/Makefile +++ b/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) diff --git a/app/product_notices.go b/app/product_notices.go index 4bd29491cd..8faee4f2e1 100644 --- a/app/product_notices.go +++ b/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) - if err != nil { - return false, errors.Wrapf(err, "Cannot parse version range %s", v) + c, err2 := semver.NewConstraint(v) + if err2 != nil { + 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) - if err != nil { - return false, errors.Wrapf(err, "Cannot parse date range %s", *cnd.DisplayDate) + c, err2 := date_constraints.NewConstraint(*cnd.DisplayDate) + if err2 != nil { + 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) - for _, v := range cnd.ServerVersion { - c, err := semver.NewConstraint(v) - if err != nil { - return false, errors.Wrapf(err, "Cannot parse version range %s", v) - } - if !c.Check(serverVersion) { - return false, nil + serverVersion, err := semver.NewVersion(model.BuildNumber) + if err != nil { + mlog.Warn("Skipping server version check, build number is not in semver format", mlog.String("build_number", model.BuildNumber)) + } else { + for _, v := range cnd.ServerVersion { + c, err := semver.NewConstraint(v) + if err != 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) - if err != nil { - skip = false - } - mlog.Debug("Will fetch notices from", mlog.String("url", NOTICES_JSON_URL), mlog.Bool("skip_cache", skip)) + url := *a.Srv().Config().AnnouncementSettings.NoticesURL + skip := *a.Srv().Config().AnnouncementSettings.NoticesSkipCache + mlog.Debug("Will fetch notices from", mlog.String("url", 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, ¬icesCache, skip) + data, err := utils.GetUrlWithCache(url, ¬icesCache, skip) if err != nil { return model.NewAppError("UpdateProductNotices", "api.system.update_notices.fetch_failed", nil, err.Error(), http.StatusBadRequest) } diff --git a/app/product_notices_test.go b/app/product_notices_test.go index a7a2fb3e8b..aa4ecfb42d 100644 --- a/app/product_notices_test.go +++ b/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() - - NOTICES_JSON_URL = fmt.Sprintf("http://%s/notices.json", server1.Listener.Addr().String()) + th.App.UpdateConfig(func(cfg *model.Config) { + *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() diff --git a/jobs/product_notices/scheduler.go b/jobs/product_notices/scheduler.go index caac705de4..73ddeef900 100644 --- a/jobs/product_notices/scheduler.go +++ b/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) - 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) + nextTime := time.Now().Add(time.Duration(*cfg.AnnouncementSettings.NoticesFetchFrequency) * time.Second) return &nextTime } diff --git a/model/config.go b/model/config.go index 90c407f0ee..02830b67e8 100644 --- a/model/config.go +++ b/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_TEXT_COLOR = "#333333" + ANNOUNCEMENT_SETTINGS_DEFAULT_BANNER_COLOR = "#f2a93b" + 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"` - BannerText *string `access:"site"` - BannerColor *string `access:"site"` - BannerTextColor *string `access:"site"` - AllowBannerDismissal *bool `access:"site"` - AdminNoticesEnabled *bool `access:"site"` - UserNoticesEnabled *bool `access:"site"` + EnableBanner *bool `access:"site"` + BannerText *string `access:"site"` + BannerColor *string `access:"site"` + BannerTextColor *string `access:"site"` + AllowBannerDismissal *bool `access:"site"` + AdminNoticesEnabled *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 {