Use batch requests for daily summary job (#72)

* Use batch requests for daily summary job

* PR feedback:

Remove '"summary all" command
remove one-liner function call
fix error message return

* lint

* go mod tidy

Co-authored-by: Lev <1187448+levb@users.noreply.github.com>

go.mod

@@ -3,7 +3,6 @@ module github.com/mattermost/mattermost-plugin-mscalendar
 go 1.13
 
 require (
-	github.com/benbjohnson/clock v1.0.0
 	github.com/golang/mock v1.2.0
 	github.com/gorilla/mux v1.7.3
 	github.com/jarcoal/httpmock v1.0.4

go.sum

@@ -38,8 +38,6 @@ github.com/armon/go-metrics v0.3.0/go.mod h1:zXjbSimjXTd7vOpY8B0/2LpvNvDoXBuplAD
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/avct/uasurfer v0.0.0-20191028135549-26b5daa857f1/go.mod h1:noBAuukeYOXa0aXGqxr24tADqkwDO2KRD15FsuaZ5a8=
 github.com/aws/aws-sdk-go v1.19.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
-github.com/benbjohnson/clock v1.0.0 h1:78Jk/r6m4wCi6sndMpty7A//t4dw/RW5fV4ZgDVfX1w=
-github.com/benbjohnson/clock v1.0.0/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM=
 github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=

server/command/daily_summary.go

@@ -22,7 +22,7 @@ func (c *Command) dailySummary(parameters ...string) (string, error) {
 
 	switch parameters[0] {
 	case "view":
-		postStr, err := c.MSCalendar.GetDailySummary(c.user())
+		postStr, err := c.MSCalendar.GetDailySummaryForUser(c.user())
 		if err != nil {
 			return err.Error(), err
 		}

server/jobs/daily_summary_job.go

@@ -4,7 +4,8 @@
 package jobs
 
 import (
-	"github.com/mattermost/mattermost-plugin-mscalendar/server/config"
+	"time"
+
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar"
 )
 
@@ -25,16 +26,9 @@ func NewDailySummaryJob() RegisteredJob {
 
 // runDailySummaryJob delivers the daily calendar summary to all users who have their settings configured to receive it now
 func runDailySummaryJob(env mscalendar.Env) {
-	mscal := mscalendar.New(env, "")
-	_, err := mscal.GetRemoteUser(env.BotUserID)
-	if err != nil {
-		env.Logger.Errorf("Please connect bot user using `/%s connect_bot`, in order to run the daily summary job.", config.CommandTrigger)
-		return
-	}
-
 	env.Logger.Debugf("Daily summary job beginning")
 
-	err = mscal.ProcessAllDailySummary()
+	err := mscalendar.New(env, "").ProcessAllDailySummary(time.Now())
 	if err != nil {
 		env.Logger.Errorf("Error during daily summary job", "error", err.Error())
 	}

server/mscalendar/availability.go

@@ -10,6 +10,7 @@ import (
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -32,6 +33,18 @@ func (m *mscalendar) SyncStatus(mattermostUserID string) (string, error) {
 }
 
 func (m *mscalendar) SyncStatusAll() (string, error) {
+	isAdmin, err := m.IsAuthorizedAdmin(m.actingUser.MattermostUserID)
+	if err != nil {
+		return "", err
+	}
+	if !isAdmin {
+		return "", errors.Errorf("Non-admin user attempting SyncStatusAll %s", m.actingUser.MattermostUserID)
+	}
+	err = m.Filter(withSuperuserClient)
+	if err != nil {
+		return "", err
+	}
+
 	userIndex, err := m.Store.LoadUserIndex()
 	if err != nil {
 		if err.Error() == "not found" {

server/mscalendar/availability_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
 
 	"github.com/mattermost/mattermost-server/v5/model"
@@ -76,11 +77,12 @@ func TestSyncStatusAll(t *testing.T) {
 			env := Env{
 				Config: conf,
 				Dependencies: &Dependencies{
-					Store:     s,
-					Logger:    logger,
-					Poster:    poster,
-					Remote:    mockRemote,
-					PluginAPI: mockPluginAPI,
+					Store:             s,
+					Logger:            logger,
+					Poster:            poster,
+					Remote:            mockRemote,
+					PluginAPI:         mockPluginAPI,
+					IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return true, nil },
 				},
 			}
 
@@ -90,7 +92,7 @@ func TestSyncStatusAll(t *testing.T) {
 					RemoteID:         "user_remote_id",
 					Email:            "user_email@example.com",
 				},
-			}, nil).AnyTimes()
+			}, nil).Times(1)
 
 			token := &oauth2.Token{
 				AccessToken: "bot_oauth_token",
@@ -121,7 +123,8 @@ func TestSyncStatusAll(t *testing.T) {
 			}
 
 			mscalendar := New(env, "")
-			mscalendar.SyncStatusAll()
+			_, err := mscalendar.SyncStatusAll()
+			require.Nil(t, err)
 		})
 	}
 }

server/mscalendar/calendar.go

@@ -29,7 +29,7 @@ func (m *mscalendar) ViewCalendar(user *User, from, to time.Time) ([]*remote.Eve
 	return m.client.GetDefaultCalendarView(user.Remote.ID, from, to)
 }
 
-func (m *mscalendar) getTodayCalendar(user *User, timezone string) ([]*remote.Event, error) {
+func (m *mscalendar) getTodayCalendarEvents(user *User, now time.Time, timezone string) ([]*remote.Event, error) {
 	err := m.Filter(
 		withClient,
 	)
@@ -42,11 +42,7 @@ func (m *mscalendar) getTodayCalendar(user *User, timezone string) ([]*remote.Ev
 		return nil, err
 	}
 
-	now := time.Now().UTC()
-	t := remote.NewDateTime(now, "UTC").In(timezone).Time()
-
-	from := time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, t.Location())
-	to := from.Add(24 * time.Hour)
+	from, to := getTodayHoursForTimezone(now, timezone)
 	return m.client.GetDefaultCalendarView(user.Remote.ID, from, to)
 }
 

server/mscalendar/daily_summary.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views"
+	"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/tz"
 	"github.com/pkg/errors"
@@ -20,11 +21,11 @@ const DailySummaryJobInterval = 15 * time.Minute
 var timeNowFunc = time.Now
 
 type DailySummary interface {
+	GetDailySummaryForUser(user *User) (string, error)
 	GetDailySummarySettingsForUser(user *User) (*store.DailySummaryUserSettings, error)
 	SetDailySummaryPostTime(user *User, timeStr string) (*store.DailySummaryUserSettings, error)
 	SetDailySummaryEnabled(user *User, enable bool) (*store.DailySummaryUserSettings, error)
-	ProcessAllDailySummary() error
-	GetDailySummary(user *User) (string, error)
+	ProcessAllDailySummary(now time.Time) error
 }
 
 func (m *mscalendar) GetDailySummarySettingsForUser(user *User) (*store.DailySummaryUserSettings, error) {
@@ -100,40 +101,72 @@ func (m *mscalendar) SetDailySummaryEnabled(user *User, enable bool) (*store.Dai
 	return dsum, nil
 }
 
-func (m *mscalendar) ProcessAllDailySummary() error {
-	err := m.Filter(
-		withSuperuserClient,
-	)
+func (m *mscalendar) ProcessAllDailySummary(now time.Time) error {
+	isAdmin, err := m.IsAuthorizedAdmin(m.actingUser.MattermostUserID)
 	if err != nil {
 		return err
 	}
+	if !isAdmin {
+		return errors.Errorf("Non-admin user attempting ProcessAllDailySummary %s", m.actingUser.MattermostUserID)
+	}
 
 	dsumIndex, err := m.Store.LoadDailySummaryIndex()
 	if err != nil {
 		return err
 	}
+	if len(dsumIndex) == 0 {
+		return nil
+	}
 
-	updatedPostTimes := map[string]string{}
+	err = m.Filter(withSuperuserClient)
+	if err != nil {
+		return err
+	}
+
+	requests := []*remote.ViewCalendarParams{}
 	for _, dsum := range dsumIndex {
-		shouldPost, err := shouldPostDailySummary(dsum)
-		if err != nil {
-			m.Logger.Errorf("Error posting daily summary for user %s: %s", dsum.MattermostUserID, err.Error())
+		shouldPost, shouldPostErr := shouldPostDailySummary(dsum, now)
+		if shouldPostErr != nil {
+			m.Logger.Errorf("Error posting daily summary for user %s: %s", dsum.MattermostUserID, shouldPostErr.Error())
 			continue
 		}
 		if !shouldPost {
 			continue
 		}
 
-		err = m.processDailySummary(dsum)
+		start, end := getTodayHoursForTimezone(now, dsum.Timezone)
+		req := &remote.ViewCalendarParams{
+			RemoteID:  dsum.RemoteID,
+			StartTime: start,
+			EndTime:   end,
+		}
+		requests = append(requests, req)
+	}
+
+	responses, err := m.client.DoBatchViewCalendarRequests(requests)
+	if err != nil {
+		return err
+	}
+
+	mappedPostTimes := map[string]string{}
+	byRemoteID := dsumIndex.ByRemoteID()
+	for _, res := range responses {
+		dsum := byRemoteID[res.RemoteID]
+		if res.Error != nil {
+			m.Logger.Errorf("Error rendering user %s calendar: %s %s", dsum.MattermostUserID, res.Error.Code, res.Error.Message)
+		}
+		postStr, err := views.RenderCalendarView(res.Events, dsum.Timezone)
 		if err != nil {
-			m.Logger.Errorf("Error posting daily summary for user %s: %s", dsum.MattermostUserID, err.Error())
+			m.Logger.Errorf("Error rendering user %s calendar: %s", dsum.MattermostUserID, err.Error())
 		}
-		updatedPostTimes[dsum.MattermostUserID] = timeNowFunc().Format(time.RFC3339)
+
+		m.Poster.DM(dsum.MattermostUserID, postStr)
+		mappedPostTimes[dsum.MattermostUserID] = time.Now().Format(time.RFC3339)
 	}
 
 	modErr := m.Store.ModifyDailySummaryIndex(func(dsumIndex store.DailySummaryIndex) (store.DailySummaryIndex, error) {
 		for _, setting := range dsumIndex {
-			postTime, ok := updatedPostTimes[setting.MattermostUserID]
+			postTime, ok := mappedPostTimes[setting.MattermostUserID]
 			if ok {
 				setting.LastPostTime = postTime
 			}
@@ -143,50 +176,26 @@ func (m *mscalendar) ProcessAllDailySummary() error {
 	if modErr != nil {
 		return modErr
 	}
+	m.Logger.Infof("Processed daily summary for %d users", len(mappedPostTimes))
 
 	return nil
 }
 
-func (m *mscalendar) GetDailySummary(user *User) (string, error) {
-	return m.getDailySummary(user)
-}
-
-func (m *mscalendar) processDailySummary(dsum *store.DailySummaryUserSettings) error {
-	user := NewUser(dsum.MattermostUserID)
-	postStr, err := m.getDailySummary(user)
-	if err != nil {
-		return err
-	}
-	m.Poster.DM(user.MattermostUserID, postStr)
-	return nil
-}
-
-func (m *mscalendar) getDailySummary(user *User) (string, error) {
+func (m *mscalendar) GetDailySummaryForUser(user *User) (string, error) {
 	tz, err := m.GetTimezone(user)
 	if err != nil {
 		return "", err
 	}
 
-	calendarData, err := m.getTodayCalendar(user, tz)
+	calendarData, err := m.getTodayCalendarEvents(user, time.Now(), tz)
 	if err != nil {
-		m.Poster.DM(user.MattermostUserID, "Failed to run daily summary job. %s", err.Error())
-		return "", err
+		return "Failed to get calendar events", err
 	}
 
-	if len(calendarData) == 0 {
-		m.Poster.DM(user.MattermostUserID, "You have no upcoming events today.")
-		return "", nil
-	}
-
-	postStr, err := views.RenderCalendarView(calendarData, tz)
-	if err != nil {
-		return "", err
-	}
-
-	return postStr, nil
+	return views.RenderCalendarView(calendarData, tz)
 }
 
-func shouldPostDailySummary(dsum *store.DailySummaryUserSettings) (bool, error) {
+func shouldPostDailySummary(dsum *store.DailySummaryUserSettings, now time.Time) (bool, error) {
 	if !dsum.Enable {
 		return false, nil
 	}
@@ -197,7 +206,7 @@ func shouldPostDailySummary(dsum *store.DailySummaryUserSettings) (bool, error)
 		if err != nil {
 			return false, errors.New("Failed to parse last post time: " + lastPostStr)
 		}
-		since := timeNowFunc().Sub(lastPost)
+		since := now.Sub(lastPost)
 		if since < dailySummaryTimeWindow {
 			return false, nil
 		}
@@ -216,7 +225,7 @@ func shouldPostDailySummary(dsum *store.DailySummaryUserSettings) (bool, error)
 		return false, err
 	}
 
-	now := timeNowFunc().In(loc)
+	now = now.In(loc)
 	if now.Weekday() == time.Saturday || now.Weekday() == time.Sunday {
 		return false, nil
 	}
@@ -228,3 +237,10 @@ func shouldPostDailySummary(dsum *store.DailySummaryUserSettings) (bool, error)
 	}
 	return -diff < dailySummaryTimeWindow, nil
 }
+
+func getTodayHoursForTimezone(now time.Time, timezone string) (start, end time.Time) {
+	t := remote.NewDateTime(now.UTC(), "UTC").In(timezone).Time()
+	start = time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, t.Location())
+	end = start.Add(24 * time.Hour)
+	return start, end
+}