From 4d3a2bbee6b3ec012f16594558a5d3e6c269d372 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:07:46 +0200 Subject: [PATCH 01/41] nil control --- server/store/setting_store.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/store/setting_store.go b/server/store/setting_store.go index d58a081d..5870902a 100644 --- a/server/store/setting_store.go +++ b/server/store/setting_store.go @@ -26,7 +26,9 @@ func (s *pluginStore) SetSetting(userID, settingID string, value interface{}) er return fmt.Errorf("cannot read value %v for setting %s (expecting bool)", value, settingID) } user.Settings.UpdateStatus = storableValue - s.Tracker.TrackAutomaticStatusUpdate(userID, storableValue, "settings") + if s.Tracker != nil { + s.Tracker.TrackAutomaticStatusUpdate(userID, storableValue, "settings") + } case GetConfirmationSettingID: storableValue, ok := value.(bool) if !ok { From fe6fe7593fe0f9b48ce808d01723b81498ef1182 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:08:11 +0200 Subject: [PATCH 02/41] use subscriptions default ttl --- server/remote/gcal/subscription.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/remote/gcal/subscription.go b/server/remote/gcal/subscription.go index 215f6f97..323bfef7 100644 --- a/server/remote/gcal/subscription.go +++ b/server/remote/gcal/subscription.go @@ -18,7 +18,7 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot" ) -const subscribeTTL = 48 * time.Hour +const subscribeTTL = 7 * 24 * time.Hour // 7 days const defaultCalendarName = "primary" const googleSubscriptionType = "webhook" From 7634f2ca51e65c72f6c12d24151e919bbf26a865 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:09:14 +0200 Subject: [PATCH 03/41] return specific error when no superuser token can be used --- server/remote/gcal/remote.go | 22 +++------------------- server/remote/remote.go | 3 +++ 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/server/remote/gcal/remote.go b/server/remote/gcal/remote.go index 13fba695..02d85487 100644 --- a/server/remote/gcal/remote.go +++ b/server/remote/gcal/remote.go @@ -51,25 +51,9 @@ func (r *impl) MakeClient(ctx context.Context, token *oauth2.Token) remote.Clien } // MakeSuperuserClient creates a new client used for app-only permissions. -func (r *impl) MakeSuperuserClient(ctx context.Context) (remote.Client, error) { - httpClient := &http.Client{} - c := &client{ - conf: r.conf, - ctx: ctx, - httpClient: httpClient, - Logger: r.logger, - // rbuilder: msgraph.NewClient(httpClient), - } - token, err := c.GetSuperuserToken() - if err != nil { - return nil, err - } - - o := &oauth2.Token{ - AccessToken: token, - TokenType: "Bearer", - } - return r.MakeClient(ctx, o), nil +// Super user tokens are not available on google calendar, so we instantiate a normal client +func (r *impl) MakeSuperuserClient(_ context.Context) (remote.Client, error) { + return nil, remote.ErrSuperUserClientNotSupported } func (r *impl) NewOAuth2Config() *oauth2.Config { diff --git a/server/remote/remote.go b/server/remote/remote.go index 1d77e7ba..66c2479f 100644 --- a/server/remote/remote.go +++ b/server/remote/remote.go @@ -5,6 +5,7 @@ package remote import ( "context" + "errors" "net/http" "golang.org/x/oauth2" @@ -13,6 +14,8 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot" ) +var ErrSuperUserClientNotSupported = errors.New("superuser client is not supported") + type Remote interface { MakeClient(context.Context, *oauth2.Token) Client MakeSuperuserClient(ctx context.Context) (Client, error) From 9dc21a28ed0f3c0c3b11bb1ebfecd5bd341d07af Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:09:48 +0200 Subject: [PATCH 04/41] notification formatting logic to other file --- server/mscalendar/notification.go | 238 ---------------------- server/mscalendar/notification_format.go | 246 +++++++++++++++++++++++ 2 files changed, 246 insertions(+), 238 deletions(-) create mode 100644 server/mscalendar/notification_format.go diff --git a/server/mscalendar/notification.go b/server/mscalendar/notification.go index 11904d36..4e8d442f 100644 --- a/server/mscalendar/notification.go +++ b/server/mscalendar/notification.go @@ -6,18 +6,13 @@ package mscalendar import ( "context" "fmt" - "strings" - "time" "github.com/mattermost/mattermost-server/v6/model" "github.com/pkg/errors" - "github.com/mattermost/mattermost-plugin-mscalendar/server/config" - "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/bot" - "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/fields" ) const maxQueueSize = 1024 @@ -221,236 +216,3 @@ func (processor *notificationProcessor) processNotification(n *remote.Notificati return nil } - -func (processor *notificationProcessor) newSlackAttachment(n *remote.Notification) *model.SlackAttachment { - title := views.EnsureSubject(n.Event.Subject) - titleLink := n.Event.Weblink - text := n.Event.BodyPreview - return &model.SlackAttachment{ - AuthorName: n.Event.Organizer.EmailAddress.Name, - AuthorLink: "mailto:" + n.Event.Organizer.EmailAddress.Address, - TitleLink: titleLink, - Title: title, - Text: text, - Fallback: fmt.Sprintf("[%s](%s): %s", title, titleLink, text), - } -} - -func (processor *notificationProcessor) newEventSlackAttachment(n *remote.Notification, timezone string) *model.SlackAttachment { - sa := processor.newSlackAttachment(n) - sa.Title = "(new) " + sa.Title - - fields := eventToFields(n.Event, timezone) - for _, k := range notificationFieldOrder { - v := fields[k] - - sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ - Title: k, - Value: strings.Join(v.Strings(), ", "), - Short: true, - }) - } - - if n.Event.ResponseRequested && !n.Event.IsOrganizer { - sa.Actions = NewPostActionForEventResponse(n.Event.ID, n.Event.ResponseStatus.Response, processor.actionURL(config.PathRespond)) - } - return sa -} - -func (processor *notificationProcessor) updatedEventSlackAttachment(n *remote.Notification, prior *remote.Event, timezone string) (bool, *model.SlackAttachment) { - sa := processor.newSlackAttachment(n) - sa.Title = "(updated) " + sa.Title - - newFields := eventToFields(n.Event, timezone) - priorFields := eventToFields(prior, timezone) - changed, added, updated, deleted := fields.Diff(priorFields, newFields) - if !changed { - return false, nil - } - - var allChanges []string - allChanges = append(allChanges, added...) - allChanges = append(allChanges, updated...) - allChanges = append(allChanges, deleted...) - - hasImportantChanges := false - for _, k := range allChanges { - if isImportantChange(k) { - hasImportantChanges = true - break - } - } - - if !hasImportantChanges { - return false, nil - } - - for _, k := range added { - if !isImportantChange(k) { - continue - } - sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ - Title: k, - Value: strings.Join(newFields[k].Strings(), ", "), - Short: true, - }) - } - for _, k := range updated { - if !isImportantChange(k) { - continue - } - sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ - Title: k, - Value: fmt.Sprintf("~~%s~~ \u2192 %s", strings.Join(priorFields[k].Strings(), ", "), strings.Join(newFields[k].Strings(), ", ")), - Short: true, - }) - } - for _, k := range deleted { - if !isImportantChange(k) { - continue - } - sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ - Title: k, - Value: fmt.Sprintf("~~%s~~", strings.Join(priorFields[k].Strings(), ", ")), - Short: true, - }) - } - - if n.Event.ResponseRequested && !n.Event.IsOrganizer && !n.Event.IsCancelled { - sa.Actions = NewPostActionForEventResponse(n.Event.ID, n.Event.ResponseStatus.Response, processor.actionURL(config.PathRespond)) - } - return true, sa -} - -func isImportantChange(fieldName string) bool { - for _, ic := range importantNotificationChanges { - if ic == fieldName { - return true - } - } - return false -} - -func (processor *notificationProcessor) actionURL(action string) string { - return fmt.Sprintf("%s%s%s", processor.Config.PluginURLPath, config.PathPostAction, action) -} - -func NewPostActionForEventResponse(eventID, response, url string) []*model.PostAction { - context := map[string]interface{}{ - config.EventIDKey: eventID, - } - - pa := &model.PostAction{ - Name: "Response", - Type: model.PostActionTypeSelect, - Integration: &model.PostActionIntegration{ - URL: url, - Context: context, - }, - } - - for _, o := range []string{OptionNotResponded, OptionYes, OptionNo, OptionMaybe} { - pa.Options = append(pa.Options, &model.PostActionOptions{Text: o, Value: o}) - } - switch response { - case ResponseNone: - pa.DefaultOption = OptionNotResponded - case ResponseYes: - pa.DefaultOption = OptionYes - case ResponseNo: - pa.DefaultOption = OptionNo - case ResponseMaybe: - pa.DefaultOption = OptionMaybe - } - return []*model.PostAction{pa} -} - -func eventToFields(e *remote.Event, timezone string) fields.Fields { - date := func(dtStart, dtEnd *remote.DateTime) (time.Time, time.Time, string) { - if dtStart == nil || dtEnd == nil { - return time.Time{}, time.Time{}, "n/a" - } - - dtStart = dtStart.In(timezone) - dtEnd = dtEnd.In(timezone) - tStart := dtStart.Time() - tEnd := dtEnd.Time() - startFormat := "Monday, January 02" - if tStart.Year() != time.Now().Year() { - startFormat = "Monday, January 02, 2006" - } - startFormat += " · (" + time.Kitchen - endFormat := " - " + time.Kitchen + ")" - return tStart, tEnd, tStart.Format(startFormat) + tEnd.Format(endFormat) - } - - start, end, formattedDate := date(e.Start, e.End) - - minutes := int(end.Sub(start).Round(time.Minute).Minutes()) - hours := int(end.Sub(start).Hours()) - minutes -= hours * 60 - days := int(end.Sub(start).Hours()) / 24 - hours -= days * 24 - - dur := "" - switch { - case days > 0: - dur = fmt.Sprintf("%v days", days) - - case e.IsAllDay: - dur = "all-day" - - // REVIEW: would be good to extract some of this stuff out into separate functions. different file too - default: - switch hours { - case 0: - // ignore - case 1: - dur = "one hour" - default: - dur = fmt.Sprintf("%v hours", hours) - } - if minutes > 0 { - if dur != "" { - dur += ", " - } - dur += fmt.Sprintf("%v minutes", minutes) - } - } - - attendees := []fields.Value{} - for _, a := range e.Attendees { - attendees = append(attendees, fields.NewStringValue( - fmt.Sprintf("[%s](mailto:%s)", - a.EmailAddress.Name, a.EmailAddress.Address))) - } - - if len(attendees) == 0 { - attendees = append(attendees, fields.NewStringValue("None")) - } - - // REVIEW: some good stuff here. gotta make sure they are all filled in for gcal's case - ff := fields.Fields{ - FieldSubject: fields.NewStringValue(views.EnsureSubject(e.Subject)), - FieldBodyPreview: fields.NewStringValue(valueOrNotDefined(e.BodyPreview)), - FieldImportance: fields.NewStringValue(valueOrNotDefined(e.Importance)), - FieldWhen: fields.NewStringValue(valueOrNotDefined(formattedDate)), - FieldDuration: fields.NewStringValue(valueOrNotDefined(dur)), - FieldOrganizer: fields.NewStringValue( - fmt.Sprintf("[%s](mailto:%s)", - e.Organizer.EmailAddress.Name, e.Organizer.EmailAddress.Address)), - FieldLocation: fields.NewStringValue(valueOrNotDefined(e.Location.DisplayName)), - FieldResponseStatus: fields.NewStringValue(e.ResponseStatus.Response), - FieldAttendees: fields.NewMultiValue(attendees...), - } - - return ff -} - -func valueOrNotDefined(s string) string { - if s == "" { - return "Not defined" - } - - return s -} diff --git a/server/mscalendar/notification_format.go b/server/mscalendar/notification_format.go new file mode 100644 index 00000000..baa65840 --- /dev/null +++ b/server/mscalendar/notification_format.go @@ -0,0 +1,246 @@ +package mscalendar + +import ( + "fmt" + "strings" + "time" + + "github.com/mattermost/mattermost-plugin-mscalendar/server/config" + "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views" + "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" + "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/fields" + "github.com/mattermost/mattermost-server/v6/model" +) + +func (processor *notificationProcessor) newSlackAttachment(n *remote.Notification) *model.SlackAttachment { + title := views.EnsureSubject(n.Event.Subject) + titleLink := n.Event.Weblink + text := n.Event.BodyPreview + return &model.SlackAttachment{ + AuthorName: n.Event.Organizer.EmailAddress.Name, + AuthorLink: "mailto:" + n.Event.Organizer.EmailAddress.Address, + TitleLink: titleLink, + Title: title, + Text: text, + Fallback: fmt.Sprintf("[%s](%s): %s", title, titleLink, text), + } +} + +func (processor *notificationProcessor) newEventSlackAttachment(n *remote.Notification, timezone string) *model.SlackAttachment { + sa := processor.newSlackAttachment(n) + sa.Title = "(new) " + sa.Title + + fields := eventToFields(n.Event, timezone) + for _, k := range notificationFieldOrder { + v := fields[k] + + sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ + Title: k, + Value: strings.Join(v.Strings(), ", "), + Short: true, + }) + } + + if n.Event.ResponseRequested && !n.Event.IsOrganizer { + sa.Actions = NewPostActionForEventResponse(n.Event.ID, n.Event.ResponseStatus.Response, processor.actionURL(config.PathRespond)) + } + return sa +} + +func (processor *notificationProcessor) updatedEventSlackAttachment(n *remote.Notification, prior *remote.Event, timezone string) (bool, *model.SlackAttachment) { + sa := processor.newSlackAttachment(n) + sa.Title = "(updated) " + sa.Title + + newFields := eventToFields(n.Event, timezone) + priorFields := eventToFields(prior, timezone) + changed, added, updated, deleted := fields.Diff(priorFields, newFields) + if !changed { + return false, nil + } + + var allChanges []string + allChanges = append(allChanges, added...) + allChanges = append(allChanges, updated...) + allChanges = append(allChanges, deleted...) + + hasImportantChanges := false + for _, k := range allChanges { + if isImportantChange(k) { + hasImportantChanges = true + break + } + } + + if !hasImportantChanges { + return false, nil + } + + for _, k := range added { + if !isImportantChange(k) { + continue + } + sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ + Title: k, + Value: strings.Join(newFields[k].Strings(), ", "), + Short: true, + }) + } + for _, k := range updated { + if !isImportantChange(k) { + continue + } + sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ + Title: k, + Value: fmt.Sprintf("~~%s~~ \u2192 %s", strings.Join(priorFields[k].Strings(), ", "), strings.Join(newFields[k].Strings(), ", ")), + Short: true, + }) + } + for _, k := range deleted { + if !isImportantChange(k) { + continue + } + sa.Fields = append(sa.Fields, &model.SlackAttachmentField{ + Title: k, + Value: fmt.Sprintf("~~%s~~", strings.Join(priorFields[k].Strings(), ", ")), + Short: true, + }) + } + + if n.Event.ResponseRequested && !n.Event.IsOrganizer && !n.Event.IsCancelled { + sa.Actions = NewPostActionForEventResponse(n.Event.ID, n.Event.ResponseStatus.Response, processor.actionURL(config.PathRespond)) + } + return true, sa +} + +func isImportantChange(fieldName string) bool { + for _, ic := range importantNotificationChanges { + if ic == fieldName { + return true + } + } + return false +} + +func (processor *notificationProcessor) actionURL(action string) string { + return fmt.Sprintf("%s%s%s", processor.Config.PluginURLPath, config.PathPostAction, action) +} + +func NewPostActionForEventResponse(eventID, response, url string) []*model.PostAction { + context := map[string]interface{}{ + config.EventIDKey: eventID, + } + + pa := &model.PostAction{ + Name: "Response", + Type: model.PostActionTypeSelect, + Integration: &model.PostActionIntegration{ + URL: url, + Context: context, + }, + } + + for _, o := range []string{OptionNotResponded, OptionYes, OptionNo, OptionMaybe} { + pa.Options = append(pa.Options, &model.PostActionOptions{Text: o, Value: o}) + } + switch response { + case ResponseNone: + pa.DefaultOption = OptionNotResponded + case ResponseYes: + pa.DefaultOption = OptionYes + case ResponseNo: + pa.DefaultOption = OptionNo + case ResponseMaybe: + pa.DefaultOption = OptionMaybe + } + return []*model.PostAction{pa} +} + +func eventToFields(e *remote.Event, timezone string) fields.Fields { + date := func(dtStart, dtEnd *remote.DateTime) (time.Time, time.Time, string) { + if dtStart == nil || dtEnd == nil { + return time.Time{}, time.Time{}, "n/a" + } + + dtStart = dtStart.In(timezone) + dtEnd = dtEnd.In(timezone) + tStart := dtStart.Time() + tEnd := dtEnd.Time() + startFormat := "Monday, January 02" + if tStart.Year() != time.Now().Year() { + startFormat = "Monday, January 02, 2006" + } + startFormat += " · (" + time.Kitchen + endFormat := " - " + time.Kitchen + ")" + return tStart, tEnd, tStart.Format(startFormat) + tEnd.Format(endFormat) + } + + start, end, formattedDate := date(e.Start, e.End) + + minutes := int(end.Sub(start).Round(time.Minute).Minutes()) + hours := int(end.Sub(start).Hours()) + minutes -= hours * 60 + days := int(end.Sub(start).Hours()) / 24 + hours -= days * 24 + + dur := "" + switch { + case days > 0: + dur = fmt.Sprintf("%v days", days) + + case e.IsAllDay: + dur = "all-day" + + // REVIEW: would be good to extract some of this stuff out into separate functions. different file too + default: + switch hours { + case 0: + // ignore + case 1: + dur = "one hour" + default: + dur = fmt.Sprintf("%v hours", hours) + } + if minutes > 0 { + if dur != "" { + dur += ", " + } + dur += fmt.Sprintf("%v minutes", minutes) + } + } + + attendees := []fields.Value{} + for _, a := range e.Attendees { + attendees = append(attendees, fields.NewStringValue( + fmt.Sprintf("[%s](mailto:%s)", + a.EmailAddress.Name, a.EmailAddress.Address))) + } + + if len(attendees) == 0 { + attendees = append(attendees, fields.NewStringValue("None")) + } + + // REVIEW: some good stuff here. gotta make sure they are all filled in for gcal's case + ff := fields.Fields{ + FieldSubject: fields.NewStringValue(views.EnsureSubject(e.Subject)), + FieldBodyPreview: fields.NewStringValue(valueOrNotDefined(e.BodyPreview)), + FieldImportance: fields.NewStringValue(valueOrNotDefined(e.Importance)), + FieldWhen: fields.NewStringValue(valueOrNotDefined(formattedDate)), + FieldDuration: fields.NewStringValue(valueOrNotDefined(dur)), + FieldOrganizer: fields.NewStringValue( + fmt.Sprintf("[%s](mailto:%s)", + e.Organizer.EmailAddress.Name, e.Organizer.EmailAddress.Address)), + FieldLocation: fields.NewStringValue(valueOrNotDefined(e.Location.DisplayName)), + FieldResponseStatus: fields.NewStringValue(e.ResponseStatus.Response), + FieldAttendees: fields.NewMultiValue(attendees...), + } + + return ff +} + +func valueOrNotDefined(s string) string { + if s == "" { + return "Not defined" + } + + return s +} From a436769b46af726d73b2f156e78ebd1abeb86df6 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:10:51 +0200 Subject: [PATCH 05/41] Client.GetEventsBetweenDates --- server/remote/client.go | 1 + server/remote/gcal/event.go | 28 ++++++++++++++++++++++++ server/remote/mock_remote/mock_client.go | 15 +++++++++++++ server/remote/msgraph/event.go | 5 +++++ 4 files changed, 49 insertions(+) diff --git a/server/remote/client.go b/server/remote/client.go index 78b5c40a..ce40b6a9 100644 --- a/server/remote/client.go +++ b/server/remote/client.go @@ -34,6 +34,7 @@ type Events interface { AcceptEvent(remoteUserID, eventID string) error DeclineEvent(remoteUserID, eventID string) error TentativelyAcceptEvent(remoteUserID, eventID string) error + GetEventsBetweenDates(remoteUserID string, start, end time.Time) ([]*Event, error) } type Subscriptions interface { diff --git a/server/remote/gcal/event.go b/server/remote/gcal/event.go index a08a33c8..07d15e90 100644 --- a/server/remote/gcal/event.go +++ b/server/remote/gcal/event.go @@ -6,6 +6,7 @@ package gcal import ( "context" "net/http" + "time" "github.com/pkg/errors" msgraph "github.com/yaegashi/msgraph.go/v1.0" @@ -86,6 +87,33 @@ func (c *client) TentativelyAcceptEvent(remoteUserID, eventID string) error { return nil } +func (c *client) GetEventsBetweenDates(_ string, start, end time.Time) (events []*remote.Event, err error) { + ctx := context.Background() + service, err := calendar.NewService(ctx, option.WithHTTPClient(c.httpClient)) + if err != nil { + return nil, errors.Wrap(err, "gcal CreateEvent, error creating service") + } + + result, err := service.Events. + List(defaultCalendarName). + TimeMin(start.Format(time.RFC3339)). + TimeMax(end.Format(time.RFC3339)). + SingleEvents(true). + OrderBy("startTime"). + ShowHiddenInvitations(false). + Context(ctx). + Do() + if err != nil { + return nil, errors.Wrap(err, "error getting list of events") + } + + for _, evt := range result.Items { + events = append(events, convertGCalEventToRemoteEvent(evt)) + } + + return events, nil +} + func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { out := &calendar.Event{} out.Summary = in.Subject diff --git a/server/remote/mock_remote/mock_client.go b/server/remote/mock_remote/mock_client.go index 55b9c355..a6dfdc72 100644 --- a/server/remote/mock_remote/mock_client.go +++ b/server/remote/mock_remote/mock_client.go @@ -242,6 +242,21 @@ func (mr *MockClientMockRecorder) GetEvent(arg0, arg1 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEvent", reflect.TypeOf((*MockClient)(nil).GetEvent), arg0, arg1) } +// GetEventsBetweenDates mocks base method. +func (m *MockClient) GetEventsBetweenDates(arg0 string, arg1, arg2 time.Time) ([]*remote.Event, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEventsBetweenDates", arg0, arg1, arg2) + ret0, _ := ret[0].([]*remote.Event) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetEventsBetweenDates indicates an expected call of GetEventsBetweenDates. +func (mr *MockClientMockRecorder) GetEventsBetweenDates(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEventsBetweenDates", reflect.TypeOf((*MockClient)(nil).GetEventsBetweenDates), arg0, arg1, arg2) +} + // GetMailboxSettings mocks base method. func (m *MockClient) GetMailboxSettings(arg0 string) (*remote.MailboxSettings, error) { m.ctrl.T.Helper() diff --git a/server/remote/msgraph/event.go b/server/remote/msgraph/event.go index beab2469..6d154ee6 100644 --- a/server/remote/msgraph/event.go +++ b/server/remote/msgraph/event.go @@ -5,6 +5,7 @@ package msgraph import ( "net/http" + "time" "github.com/pkg/errors" msgraph "github.com/yaegashi/msgraph.go/v1.0" @@ -49,3 +50,7 @@ func (c *client) TentativelyAcceptEvent(remoteUserID, eventID string) error { } return nil } + +func (c *client) GetEventsBetweenDates(_ string, start, end time.Time) ([]*remote.Event, error) { + return nil, errors.New("not implemented") +} From 18767a370366aaef0f21b65ab783fbc6d7a7ac3b Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:11:07 +0200 Subject: [PATCH 06/41] typo: import --- server/remote/gcal/remote.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/remote/gcal/remote.go b/server/remote/gcal/remote.go index 02d85487..14b66167 100644 --- a/server/remote/gcal/remote.go +++ b/server/remote/gcal/remote.go @@ -5,7 +5,6 @@ package gcal import ( "context" - "net/http" "golang.org/x/oauth2" "google.golang.org/api/calendar/v3" From 8de422316ce3114dab0906430aa1ffbbef3ad90b Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:38:23 +0200 Subject: [PATCH 07/41] shorter command name :) --- server/command/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/command/command.go b/server/command/command.go index 4ff03ec6..cb7069f8 100644 --- a/server/command/command.go +++ b/server/command/command.go @@ -107,7 +107,7 @@ func (c *Command) Handle() (string, bool, error) { handler = c.requireConnectedUser(c.findMeetings) case "showcals": handler = c.requireConnectedUser(c.showCalendars) - case "availability": + case "avail": handler = c.requireConnectedUser(c.requireAdminUser(c.debugAvailability)) case "settings": handler = c.requireConnectedUser(c.settings) From af611a92eae06f66aaf46da71e64295383381eb3 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:38:50 +0200 Subject: [PATCH 08/41] availability without super user token --- server/mscalendar/availability.go | 106 ++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 6 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index a52267d4..1eda5577 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -15,6 +15,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/mattermost/mattermost-plugin-mscalendar/server/utils/bot" ) const ( @@ -46,15 +47,10 @@ func (m *mscalendar) Sync(mattermostUserID string) (string, *StatusSyncJobSummar return "", nil, err } - return m.syncUsers(store.UserIndex{user}) + return m.syncUsersIndividually(store.UserIndex{user}) } func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { - err := m.Filter(withSuperuserClient) - if err != nil { - return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") - } - userIndex, err := m.Store.LoadUserIndex() if err != nil { if err.Error() == "not found" { @@ -63,9 +59,85 @@ func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to load the users from user index") } + err = m.Filter(withSuperuserClient) + // Allow processing the daily summary using individual credentials for remotes that doesn't allow + // "superuser" access + if errors.Is(err, remote.ErrSuperUserClientNotSupported) { + return m.syncUsersIndividually(userIndex) + } + if err != nil { + return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") + } + return m.syncUsers(userIndex) } +func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, *StatusSyncJobSummary, error) { + syncJobSummary := &StatusSyncJobSummary{} + if len(userIndex) == 0 { + return "No connected users found", syncJobSummary, nil + } + syncJobSummary.NumberOfUsersProcessed = len(userIndex) + + numberOfLogs := 0 + users := []*store.User{} + calendarViews := []*remote.ViewCalendarResponse{} + for _, u := range userIndex { + // TODO fetch users from kvstore in batches, and process in batches instead of all at once + user, err := m.Store.LoadUser(u.MattermostUserID) + if err != nil { + syncJobSummary.NumberOfUsersFailedStatusChanged++ + if numberOfLogs < logTruncateLimit { + m.Logger.Warnf("Not able to load user %s from user index. err=%v", u.MattermostUserID, err) + } else if numberOfLogs == logTruncateLimit { + m.Logger.Warnf(logTruncateMsg) + } + numberOfLogs++ + + // In case of error in loading, skip this user and continue with the next user + continue + } + if user.Settings.UpdateStatus || user.Settings.ReceiveReminders { + users = append(users, user) + } + + start := time.Now().UTC() + end := time.Now().UTC().Add(calendarViewTimeWindowSize) + + calendarUser := NewUser(u.MattermostUserID) + + calendar, err := m.GetCalendarEvents(calendarUser, start, end) + if err != nil { + syncJobSummary.NumberOfUsersFailedStatusChanged++ + m.Logger.With(bot.LogContext{ + "user": u.MattermostUserID, + "err": err, + }).Errorf("error getting calendar events") + continue + } + + calendarViews = append(calendarViews, calendar) + } + if len(users) == 0 { + return "No users need to be synced", syncJobSummary, nil + } + + if len(calendarViews) == 0 { + return "No calendar views found", syncJobSummary, nil + } + + m.deliverReminders(users, calendarViews) + out, numberOfUsersStatusChanged, numberOfUsersFailedStatusChanged, err := m.setUserStatuses(users, calendarViews) + if err != nil { + return "", syncJobSummary, errors.Wrap(err, "error setting the user statuses") + } + + syncJobSummary.NumberOfUsersFailedStatusChanged += numberOfUsersFailedStatusChanged + syncJobSummary.NumberOfUsersStatusChanged = numberOfUsersStatusChanged + + return out, syncJobSummary, nil +} + func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, *StatusSyncJobSummary, error) { syncJobSummary := &StatusSyncJobSummary{} if len(userIndex) == 0 { @@ -385,6 +457,28 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S return nil } +func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { + err := m.Filter(withRemoteUser(user)) + if err != nil { + return nil, err + } + + err = m.Filter(withClient) + if err != nil { + return nil, err + } + + events, err := m.client.GetEventsBetweenDates(user.Remote.ID, start, end) + if err != nil { + return nil, errors.Wrapf(err, "error getting events for user %s", user.MattermostUserID) + } + + return &remote.ViewCalendarResponse{ + RemoteUserID: user.Remote.ID, + Events: events, + }, nil +} + func (m *mscalendar) GetCalendarViews(users []*store.User) ([]*remote.ViewCalendarResponse, error) { err := m.Filter(withClient) if err != nil { From 24c2c6417f92e9636d936caf79b67457327018aa Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:51:31 +0200 Subject: [PATCH 09/41] delete store subs only if we successfuly do on remote --- server/mscalendar/subscription.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/mscalendar/subscription.go b/server/mscalendar/subscription.go index 01881505..3ded2e8d 100644 --- a/server/mscalendar/subscription.go +++ b/server/mscalendar/subscription.go @@ -115,15 +115,16 @@ func (m *mscalendar) DeleteMyEventSubscription() error { subscriptionID := m.actingUser.Settings.EventSubscriptionID - err = m.Store.DeleteUserSubscription(m.actingUser.User, subscriptionID) + err = m.DeleteOrphanedSubscription(subscriptionID) if err != nil { - return errors.WithMessagef(err, "failed to delete subscription %s", subscriptionID) + return err } - err = m.DeleteOrphanedSubscription(subscriptionID) + err = m.Store.DeleteUserSubscription(m.actingUser.User, subscriptionID) if err != nil { - return err + return errors.WithMessagef(err, "failed to delete subscription %s", subscriptionID) } + return nil } From 0ecad3fac3688d743a27179a42fc43644c8d5f3f Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:53:08 +0200 Subject: [PATCH 10/41] comment --- server/mscalendar/subscription.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/mscalendar/subscription.go b/server/mscalendar/subscription.go index 3ded2e8d..e5aed5a7 100644 --- a/server/mscalendar/subscription.go +++ b/server/mscalendar/subscription.go @@ -51,6 +51,9 @@ func (m *mscalendar) LoadMyEventSubscription() (*store.Subscription, error) { if err != nil { return nil, err } + + // TODO: if m.actingUser.Settings.EventSubscriptionID is empty, there's no sub + storedSub, err := m.Store.LoadSubscription(m.actingUser.Settings.EventSubscriptionID) if err != nil { return nil, err From 165948ce7dd8cc624475fd8702f84b4d943559a1 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 15:06:53 +0200 Subject: [PATCH 11/41] ignore missing subs --- server/mscalendar/notification.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/mscalendar/notification.go b/server/mscalendar/notification.go index 4e8d442f..550e1caa 100644 --- a/server/mscalendar/notification.go +++ b/server/mscalendar/notification.go @@ -119,7 +119,9 @@ func (processor *notificationProcessor) work() { func (processor *notificationProcessor) processNotification(n *remote.Notification) error { sub, err := processor.Store.LoadSubscription(n.SubscriptionID) if err != nil { - return err + // TODO: Only happening when the subs were removed locally but not on remote, I kept getting + // notifications even after removing the app manually from my google account page. + return nil } creator, err := processor.Store.LoadUser(sub.MattermostCreatorID) if err != nil { From 2e6c571a53759e91724e26ec9769e07296f8973a Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 15:07:13 +0200 Subject: [PATCH 12/41] update sync method --- server/mscalendar/availability.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 1eda5577..9414d4af 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -47,7 +47,19 @@ func (m *mscalendar) Sync(mattermostUserID string) (string, *StatusSyncJobSummar return "", nil, err } - return m.syncUsersIndividually(store.UserIndex{user}) + userIndex := store.UserIndex{user} + + err = m.Filter(withSuperuserClient) + // Allow processing the daily summary using individual credentials for remotes that doesn't allow + // "superuser" access + if errors.Is(err, remote.ErrSuperUserClientNotSupported) { + return m.syncUsersIndividually(userIndex) + } + if err != nil { + return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") + } + + return m.syncUsers(userIndex) } func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { @@ -460,12 +472,12 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { err := m.Filter(withRemoteUser(user)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "error withRemoteUser") } err = m.Filter(withClient) if err != nil { - return nil, err + return nil, errors.Wrap(err, "errror withClient") } events, err := m.client.GetEventsBetweenDates(user.Remote.ID, start, end) From cc88b2e86ebc20edf221fb0100d1379769bfa1ca Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 15:24:48 +0200 Subject: [PATCH 13/41] allow switching acting user on the fly --- server/mscalendar/availability.go | 5 ++--- server/mscalendar/filters.go | 7 +++++++ server/mscalendar/user.go | 7 +++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 9414d4af..65526d90 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -116,8 +116,7 @@ func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, * start := time.Now().UTC() end := time.Now().UTC().Add(calendarViewTimeWindowSize) - calendarUser := NewUser(u.MattermostUserID) - + calendarUser := newUserFromStoredUser(user) calendar, err := m.GetCalendarEvents(calendarUser, start, end) if err != nil { syncJobSummary.NumberOfUsersFailedStatusChanged++ @@ -470,7 +469,7 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S } func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { - err := m.Filter(withRemoteUser(user)) + err := m.Filter(withActingUser(user.MattermostUserID)) if err != nil { return nil, errors.Wrap(err, "error withRemoteUser") } diff --git a/server/mscalendar/filters.go b/server/mscalendar/filters.go index 7074c171..14d8792f 100644 --- a/server/mscalendar/filters.go +++ b/server/mscalendar/filters.go @@ -31,6 +31,13 @@ func withRemoteUser(user *User) func(m *mscalendar) error { } } +func withActingUser(mattermostUserID string) func(m *mscalendar) error { + return func(m *mscalendar) error { + m.actingUser = NewUser(mattermostUserID) + return nil + } +} + func withClient(m *mscalendar) error { if m.client != nil { return nil diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index b1331072..b8e0351b 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -35,6 +35,13 @@ func NewUser(mattermostUserID string) *User { } } +func newUserFromStoredUser(u *store.User) *User { + return &User{ + User: u, + MattermostUserID: u.MattermostUserID, + } +} + func (user *User) Clone() *User { clone := *user clone.User = user.User.Clone() From 0def44ad75e6b5b424678f50866236f87d6745cf Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 15:30:35 +0200 Subject: [PATCH 14/41] daily summary without super user token --- server/mscalendar/daily_summary.go | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index ad57b1a0..10be7277 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -12,6 +12,7 @@ import ( "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/bot" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/tz" ) @@ -92,6 +93,65 @@ func (m *mscalendar) SetDailySummaryEnabled(user *User, enable bool) (*store.Dai return dsum, nil } +func (m *mscalendar) processAllDailySummaryByUser(now time.Time, userIndex store.UserIndex) error { + for _, user := range userIndex { + storeUser, storeErr := m.Store.LoadUser(user.MattermostUserID) + if storeErr != nil { + m.Logger.Warnf("Error loading user %s for daily summary. err=%v", user.MattermostUserID, storeErr) + continue + } + + dsum := storeUser.Settings.DailySummary + if dsum == nil { + continue + } + + shouldPost, shouldPostErr := shouldPostDailySummary(dsum, now) + if shouldPostErr != nil { + m.Logger.Warnf("Error checking should post daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + continue + } + if !shouldPost { + continue + } + + u := NewUser(user.MattermostUserID) + if err := m.ExpandUser(u); err != nil { + m.Logger.With(bot.LogContext{ + "mattermost_id": storeUser.MattermostUserID, + "remote_id": storeUser.Remote.ID, + "err": err, + }).Errorf("error getting user information") + continue + } + + m.Filter(withActingUser(u.MattermostUserID)) + + postStr, err := m.GetDailySummaryForUser(u) + if err != nil { + m.Logger.With(bot.LogContext{ + "mattermost_id": storeUser.MattermostUserID, + "remote_id": storeUser.Remote.ID, + "err": err, + }).Errorf("error getting daily summary for user") + continue + } + + if _, err := m.Poster.DM(user.MattermostUserID, postStr); err != nil { + m.Logger.With(bot.LogContext{ + "user": storeUser.MattermostUserID, + "err": err, + }).Errorf("error posting daily summary for user") + continue + } + + // REVIEW: Seems kind of pointless to track a passive event like this + m.Dependencies.Tracker.TrackDailySummarySent(user.MattermostUserID) + } + + return nil +} + func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { userIndex, err := m.Store.LoadUserIndex() if err != nil { @@ -102,6 +162,11 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { } err = m.Filter(withSuperuserClient) + // Allow processing the daily summary using individual credentials for remotes that doesn't allow + // "superuser" access + if errors.Is(err, remote.ErrSuperUserClientNotSupported) { + return m.processAllDailySummaryByUser(now, userIndex) + } if err != nil { return err } From c5f0e1ccae4e2b02690352424831ea639abf2a1f Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 15:57:56 +0200 Subject: [PATCH 15/41] formatted imports --- server/mscalendar/notification_format.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/mscalendar/notification_format.go b/server/mscalendar/notification_format.go index baa65840..d7b61ad2 100644 --- a/server/mscalendar/notification_format.go +++ b/server/mscalendar/notification_format.go @@ -9,6 +9,7 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views" "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/fields" + "github.com/mattermost/mattermost-server/v6/model" ) From 0b7bce2d0d9fde9a8e7886d3d91336cafac55a06 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 19:45:14 +0200 Subject: [PATCH 16/41] calendar -> calendarEvents --- server/mscalendar/availability.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 65526d90..716036dc 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -117,7 +117,7 @@ func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, * end := time.Now().UTC().Add(calendarViewTimeWindowSize) calendarUser := newUserFromStoredUser(user) - calendar, err := m.GetCalendarEvents(calendarUser, start, end) + calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) if err != nil { syncJobSummary.NumberOfUsersFailedStatusChanged++ m.Logger.With(bot.LogContext{ @@ -127,7 +127,7 @@ func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, * continue } - calendarViews = append(calendarViews, calendar) + calendarViews = append(calendarViews, calendarEvents) } if len(users) == 0 { return "No users need to be synced", syncJobSummary, nil From 100497a7d1e99eb76b0b0d0580c0d49935c2d167 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 19:45:47 +0200 Subject: [PATCH 17/41] processAllDailySummaryByUser -> processAllDailySummaryWithIndividualCredentials --- server/mscalendar/daily_summary.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index 10be7277..1570d1a9 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -93,7 +93,7 @@ func (m *mscalendar) SetDailySummaryEnabled(user *User, enable bool) (*store.Dai return dsum, nil } -func (m *mscalendar) processAllDailySummaryByUser(now time.Time, userIndex store.UserIndex) error { +func (m *mscalendar) processAllDailySummaryWithIndividualCredentials(now time.Time, userIndex store.UserIndex) error { for _, user := range userIndex { storeUser, storeErr := m.Store.LoadUser(user.MattermostUserID) if storeErr != nil { @@ -165,7 +165,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { // Allow processing the daily summary using individual credentials for remotes that doesn't allow // "superuser" access if errors.Is(err, remote.ErrSuperUserClientNotSupported) { - return m.processAllDailySummaryByUser(now, userIndex) + return m.processAllDailySummaryWithIndividualCredentials(now, userIndex) } if err != nil { return err From d90fb5e9fafeddef11583e84dd16ac58925a795b Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 19:49:16 +0200 Subject: [PATCH 18/41] revert silenced error under notifications --- server/mscalendar/notification.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/mscalendar/notification.go b/server/mscalendar/notification.go index 550e1caa..4e8d442f 100644 --- a/server/mscalendar/notification.go +++ b/server/mscalendar/notification.go @@ -119,9 +119,7 @@ func (processor *notificationProcessor) work() { func (processor *notificationProcessor) processNotification(n *remote.Notification) error { sub, err := processor.Store.LoadSubscription(n.SubscriptionID) if err != nil { - // TODO: Only happening when the subs were removed locally but not on remote, I kept getting - // notifications even after removing the app manually from my google account page. - return nil + return err } creator, err := processor.Store.LoadUser(sub.MattermostCreatorID) if err != nil { From 4345953f08c7aebf99fb7fcdfd228583206af088 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 19:55:43 +0200 Subject: [PATCH 19/41] msgraph GetEventsBetweenDates --- server/remote/msgraph/event.go | 12 ++++++++++-- server/remote/msgraph/get_default_calendar_view.go | 11 +---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/server/remote/msgraph/event.go b/server/remote/msgraph/event.go index 6d154ee6..c182fabe 100644 --- a/server/remote/msgraph/event.go +++ b/server/remote/msgraph/event.go @@ -51,6 +51,14 @@ func (c *client) TentativelyAcceptEvent(remoteUserID, eventID string) error { return nil } -func (c *client) GetEventsBetweenDates(_ string, start, end time.Time) ([]*remote.Event, error) { - return nil, errors.New("not implemented") +func (c *client) GetEventsBetweenDates(remoteUserID string, start, end time.Time) ([]*remote.Event, error) { + paramStr := getQueryParamStringForCalendarView(start, end) + res := &calendarViewResponse{} + err := c.rbuilder.Users().ID(remoteUserID).CalendarView().Request().JSONRequest( + c.ctx, http.MethodGet, paramStr, nil, res) + if err != nil { + return nil, errors.Wrap(err, "msgraph GetEventsBetweenDates") + } + + return res.Value, nil } diff --git a/server/remote/msgraph/get_default_calendar_view.go b/server/remote/msgraph/get_default_calendar_view.go index 3faa83a4..a8f51fd4 100644 --- a/server/remote/msgraph/get_default_calendar_view.go +++ b/server/remote/msgraph/get_default_calendar_view.go @@ -30,16 +30,7 @@ type calendarViewBatchResponse struct { } func (c *client) GetDefaultCalendarView(remoteUserID string, start, end time.Time) ([]*remote.Event, error) { - paramStr := getQueryParamStringForCalendarView(start, end) - - res := &calendarViewResponse{} - err := c.rbuilder.Users().ID(remoteUserID).CalendarView().Request().JSONRequest( - c.ctx, http.MethodGet, paramStr, nil, res) - if err != nil { - return nil, errors.Wrap(err, "msgraph GetDefaultCalendarView") - } - - return res.Value, nil + return c.GetEventsBetweenDates(remoteUserID, start, end) } func (c *client) DoBatchViewCalendarRequests(allParams []*remote.ViewCalendarParams) ([]*remote.ViewCalendarResponse, error) { From 57b30f982d11d8ac060acdc4e59bf991bebe1718 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Wed, 26 Jul 2023 09:49:12 +0200 Subject: [PATCH 20/41] Update server/mscalendar/daily_summary.go Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com> --- server/mscalendar/daily_summary.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index 1570d1a9..ea0ac14e 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -162,7 +162,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { } err = m.Filter(withSuperuserClient) - // Allow processing the daily summary using individual credentials for remotes that doesn't allow + // Allow processing the daily summary using individual credentials for remotes that don't allow // "superuser" access if errors.Is(err, remote.ErrSuperUserClientNotSupported) { return m.processAllDailySummaryWithIndividualCredentials(now, userIndex) From 92de7367445014c8fadea3df5f9980c5caaf3a96 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Tue, 25 Jul 2023 16:54:27 +0200 Subject: [PATCH 21/41] [GCAL] Create event logic (#269) * remote logic * allow datetimes without timezone * add calendar settings readonly scope to get tz * event conversion * scopes * revert datetime.time --- server/remote/gcal/event.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/server/remote/gcal/event.go b/server/remote/gcal/event.go index 07d15e90..6da18a5e 100644 --- a/server/remote/gcal/event.go +++ b/server/remote/gcal/event.go @@ -112,6 +112,39 @@ func (c *client) GetEventsBetweenDates(_ string, start, end time.Time) (events [ } return events, nil + +} +func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { + out := &calendar.Event{} + out.Summary = in.Subject + out.Start = convertRemoteDateTimeToGcalEventDateTime(in.Start) + out.End = convertRemoteDateTimeToGcalEventDateTime(in.End) + out.Description = in.Body.Content + if in.Location != nil { + out.Location = in.Location.DisplayName + } + + for _, attendee := range in.Attendees { + out.Attendees = append(out.Attendees, &calendar.EventAttendee{ + Email: attendee.EmailAddress.Address, + }) + } + + return out +} + +func convertRemoteDateTimeToGcalEventDateTime(in *remote.DateTime) *calendar.EventDateTime { + out := &calendar.EventDateTime{} + dt := in.String() + + // Avoid setting non RFC3339 strings + if dt != remote.UndefinedDatetime { + out.DateTime = dt + } + + out.TimeZone = in.TimeZone + + return out } func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { From 3e3f43d4525184e9ee1519b6a35adbb5211c11e1 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 25 Jul 2023 14:10:51 +0200 Subject: [PATCH 22/41] Client.GetEventsBetweenDates --- server/remote/gcal/event.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/remote/gcal/event.go b/server/remote/gcal/event.go index 6da18a5e..c8a6b66a 100644 --- a/server/remote/gcal/event.go +++ b/server/remote/gcal/event.go @@ -112,8 +112,8 @@ func (c *client) GetEventsBetweenDates(_ string, start, end time.Time) (events [ } return events, nil - } + func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { out := &calendar.Event{} out.Summary = in.Subject @@ -145,6 +145,7 @@ func convertRemoteDateTimeToGcalEventDateTime(in *remote.DateTime) *calendar.Eve out.TimeZone = in.TimeZone return out + } func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { From 5c05a7417b19a33c98b07f879fb409700967b135 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 10:06:41 +0200 Subject: [PATCH 23/41] fix: store last post time --- server/mscalendar/daily_summary.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index ea0ac14e..c0884506 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -147,6 +147,9 @@ func (m *mscalendar) processAllDailySummaryWithIndividualCredentials(now time.Ti // REVIEW: Seems kind of pointless to track a passive event like this m.Dependencies.Tracker.TrackDailySummarySent(user.MattermostUserID) + + dsum.LastPostTime = time.Now().Format(time.RFC3339) + err = m.Store.StoreUser(storeUser) } return nil From 5dbc7015c3d9e160e937932abcd4d117b20475fe Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 11:09:24 +0200 Subject: [PATCH 24/41] refactored availability logic --- server/mscalendar/availability.go | 111 ++++++++++-------------------- 1 file changed, 35 insertions(+), 76 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 716036dc..e2d3b8c1 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -50,16 +50,11 @@ func (m *mscalendar) Sync(mattermostUserID string) (string, *StatusSyncJobSummar userIndex := store.UserIndex{user} err = m.Filter(withSuperuserClient) - // Allow processing the daily summary using individual credentials for remotes that doesn't allow - // "superuser" access - if errors.Is(err, remote.ErrSuperUserClientNotSupported) { - return m.syncUsersIndividually(userIndex) - } - if err != nil { + if err != nil && !errors.Is(err, remote.ErrSuperUserClientNotSupported) { return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") } - return m.syncUsers(userIndex) + return m.syncUsers(userIndex, errors.Is(err, remote.ErrSuperUserClientNotSupported)) } func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { @@ -72,24 +67,19 @@ func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { } err = m.Filter(withSuperuserClient) - // Allow processing the daily summary using individual credentials for remotes that doesn't allow - // "superuser" access - if errors.Is(err, remote.ErrSuperUserClientNotSupported) { - return m.syncUsersIndividually(userIndex) - } - if err != nil { + if err != nil && !errors.Is(err, remote.ErrSuperUserClientNotSupported) { return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") } - return m.syncUsers(userIndex) + return m.syncUsers(userIndex, errors.Is(err, remote.ErrSuperUserClientNotSupported)) } -func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, *StatusSyncJobSummary, error) { - syncJobSummary := &StatusSyncJobSummary{} - if len(userIndex) == 0 { - return "No connected users found", syncJobSummary, nil - } - syncJobSummary.NumberOfUsersProcessed = len(userIndex) +// retrieveUsersToSync retrieves the users and their calendar data to sync up and send notifications +// The parameter fetchIndividually determines if the calendar data should be fetched while we loop the +// users (using individual credentials) or on a batch after the loop. +func (m *mscalendar) retrieveUsersToSync(userIndex store.UserIndex, syncJobSummary *StatusSyncJobSummary, fetchIndividually bool) ([]*store.User, []*remote.ViewCalendarResponse, error) { + start := time.Now().UTC() + end := time.Now().UTC().Add(calendarViewTimeWindowSize) numberOfLogs := 0 users := []*store.User{} @@ -113,80 +103,49 @@ func (m *mscalendar) syncUsersIndividually(userIndex store.UserIndex) (string, * users = append(users, user) } - start := time.Now().UTC() - end := time.Now().UTC().Add(calendarViewTimeWindowSize) - - calendarUser := newUserFromStoredUser(user) - calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) - if err != nil { - syncJobSummary.NumberOfUsersFailedStatusChanged++ - m.Logger.With(bot.LogContext{ - "user": u.MattermostUserID, - "err": err, - }).Errorf("error getting calendar events") - continue + if fetchIndividually { + calendarUser := newUserFromStoredUser(user) + calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) + if err != nil { + syncJobSummary.NumberOfUsersFailedStatusChanged++ + m.Logger.With(bot.LogContext{ + "user": u.MattermostUserID, + "err": err, + }).Errorf("error getting calendar events") + continue + } + calendarViews = append(calendarViews, calendarEvents) } - - calendarViews = append(calendarViews, calendarEvents) } if len(users) == 0 { - return "No users need to be synced", syncJobSummary, nil + return users, calendarViews, fmt.Errorf("no users need to be synced") } - if len(calendarViews) == 0 { - return "No calendar views found", syncJobSummary, nil + if !fetchIndividually { + var err error + calendarViews, err = m.GetCalendarViews(users) + if err != nil { + return users, calendarViews, errors.Wrap(err, "not able to get calendar views for connected users") + } } - m.deliverReminders(users, calendarViews) - out, numberOfUsersStatusChanged, numberOfUsersFailedStatusChanged, err := m.setUserStatuses(users, calendarViews) - if err != nil { - return "", syncJobSummary, errors.Wrap(err, "error setting the user statuses") + if len(calendarViews) == 0 { + return users, calendarViews, fmt.Errorf("no calendar views found") } - syncJobSummary.NumberOfUsersFailedStatusChanged += numberOfUsersFailedStatusChanged - syncJobSummary.NumberOfUsersStatusChanged = numberOfUsersStatusChanged - - return out, syncJobSummary, nil + return users, calendarViews, nil } -func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, *StatusSyncJobSummary, error) { +func (m *mscalendar) syncUsers(userIndex store.UserIndex, fetchIndividually bool) (string, *StatusSyncJobSummary, error) { syncJobSummary := &StatusSyncJobSummary{} if len(userIndex) == 0 { return "No connected users found", syncJobSummary, nil } syncJobSummary.NumberOfUsersProcessed = len(userIndex) - numberOfLogs := 0 - users := []*store.User{} - for _, u := range userIndex { - // TODO fetch users from kvstore in batches, and process in batches instead of all at once - user, err := m.Store.LoadUser(u.MattermostUserID) - if err != nil { - syncJobSummary.NumberOfUsersFailedStatusChanged++ - if numberOfLogs < logTruncateLimit { - m.Logger.Warnf("Not able to load user %s from user index. err=%v", u.MattermostUserID, err) - } else if numberOfLogs == logTruncateLimit { - m.Logger.Warnf(logTruncateMsg) - } - numberOfLogs++ - - // In case of error in loading, skip this user and continue with the next user - continue - } - if user.Settings.UpdateStatus || user.Settings.ReceiveReminders { - users = append(users, user) - } - } - if len(users) == 0 { - return "No users need to be synced", syncJobSummary, nil - } - - calendarViews, err := m.GetCalendarViews(users) + users, calendarViews, err := m.retrieveUsersToSync(userIndex, syncJobSummary, fetchIndividually) if err != nil { - return "", syncJobSummary, errors.Wrap(err, "not able to get calendar views for connected users") - } - if len(calendarViews) == 0 { - return "No calendar views found", syncJobSummary, nil + return err.Error(), syncJobSummary, errors.Wrapf(err, "error retrieving users to sync (individually=%b)", fetchIndividually) } m.deliverReminders(users, calendarViews) From 19abc7011e4678fcf4a400311a2bcfc90d7411a5 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 13:53:25 +0200 Subject: [PATCH 25/41] availability test --- server/mscalendar/availability.go | 14 ++- server/mscalendar/availability_test.go | 116 ++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index e2d3b8c1..0d82a4a2 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -29,6 +29,10 @@ const ( logTruncateLimit = 5 ) +var ( + errNoUsersNeedToBeSynced = errors.New("no users need to be synced") +) + type StatusSyncJobSummary struct { NumberOfUsersFailedStatusChanged int NumberOfUsersStatusChanged int @@ -99,10 +103,14 @@ func (m *mscalendar) retrieveUsersToSync(userIndex store.UserIndex, syncJobSumma // In case of error in loading, skip this user and continue with the next user continue } - if user.Settings.UpdateStatus || user.Settings.ReceiveReminders { - users = append(users, user) + + // If user does not have the proper features enabled, just go to the next one + if !(user.Settings.UpdateStatus || user.Settings.ReceiveReminders) { + continue } + users = append(users, user) + if fetchIndividually { calendarUser := newUserFromStoredUser(user) calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) @@ -145,7 +153,7 @@ func (m *mscalendar) syncUsers(userIndex store.UserIndex, fetchIndividually bool users, calendarViews, err := m.retrieveUsersToSync(userIndex, syncJobSummary, fetchIndividually) if err != nil { - return err.Error(), syncJobSummary, errors.Wrapf(err, "error retrieving users to sync (individually=%b)", fetchIndividually) + return err.Error(), syncJobSummary, errors.Wrapf(err, "error retrieving users to sync (individually=%v)", fetchIndividually) } m.deliverReminders(users, calendarViews) diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 23ca928f..fc8feec4 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -116,7 +116,15 @@ func TestSyncStatusAll(t *testing.T) { env, client := makeStatusSyncTestEnv(ctrl) deps := env.Dependencies - c, papi, s, logger := client.(*mock_remote.MockClient), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Store.(*mock_store.MockStore), deps.Logger.(*mock_bot.MockLogger) + c, r, papi, s, logger := client.(*mock_remote.MockClient), env.Remote.(*mock_remote.MockRemote), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Store.(*mock_store.MockStore), deps.Logger.(*mock_bot.MockLogger) + s.EXPECT().LoadUserIndex().Return(store.UserIndex{ + &store.UserShort{ + MattermostUserID: "user_mm_id", + RemoteID: "user_remote_id", + Email: "user_email@example.com", + }, + }, nil).Times(1) + r.EXPECT().MakeSuperuserClient(context.Background()).Return(client, nil) mockUser := &store.User{ MattermostUserID: "user_mm_id", @@ -188,7 +196,15 @@ func TestSyncStatusUserConfig(t *testing.T) { GetConfirmation: true, }, runAssertions: func(deps *Dependencies, client remote.Client) { - c, papi, poster, s := client.(*mock_remote.MockClient), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Poster.(*mock_bot.MockPoster), deps.Store.(*mock_store.MockStore) + c, r, papi, poster, s := client.(*mock_remote.MockClient), deps.Remote.(*mock_remote.MockRemote), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Poster.(*mock_bot.MockPoster), deps.Store.(*mock_store.MockStore) + s.EXPECT().LoadUserIndex().Return(store.UserIndex{ + &store.UserShort{ + MattermostUserID: "user_mm_id", + RemoteID: "user_remote_id", + Email: "user_email@example.com", + }, + }, nil).Times(1) + r.EXPECT().MakeSuperuserClient(context.Background()).Return(client, nil) moment := time.Now().UTC() busyEvent := &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} @@ -291,7 +307,15 @@ func TestReminders(t *testing.T) { env, client := makeStatusSyncTestEnv(ctrl) deps := env.Dependencies - c, s, poster, logger := client.(*mock_remote.MockClient), deps.Store.(*mock_store.MockStore), deps.Poster.(*mock_bot.MockPoster), deps.Logger.(*mock_bot.MockLogger) + c, r, poster, s, logger := client.(*mock_remote.MockClient), env.Remote.(*mock_remote.MockRemote), deps.Poster.(*mock_bot.MockPoster), deps.Store.(*mock_store.MockStore), deps.Logger.(*mock_bot.MockLogger) + s.EXPECT().LoadUserIndex().Return(store.UserIndex{ + &store.UserShort{ + MattermostUserID: "user_mm_id", + RemoteID: "user_remote_id", + Email: "user_email@example.com", + }, + }, nil).Times(1) + r.EXPECT().MakeSuperuserClient(context.Background()).Return(client, nil) loadUser := s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ MattermostUserID: "user_mm_id", @@ -328,6 +352,82 @@ func TestReminders(t *testing.T) { } } +func TestRetrieveUsersToSyncIndividually(t *testing.T) { + t.Run("no users to sync", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + env, _ := makeStatusSyncTestEnv(ctrl) + + m := New(env, "").(*mscalendar) + jobSummary := &StatusSyncJobSummary{} + + _, _, err := m.retrieveUsersToSync([]*store.UserShort{}, jobSummary, true) + require.Error(t, err, errNoUsersNeedToBeSynced) + }) + + t.Run("user reminders and status disabled", func(t *testing.T) { + testUser := newTestUser() + testUser.Settings.UpdateStatus = false + testUser.Settings.ReceiveReminders = false + + userIndex := []*store.UserShort{ + { + MattermostUserID: testUser.MattermostUserID, + RemoteID: testUser.Remote.ID, + Email: testUser.Remote.Mail, + }, + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + e, _ := makeStatusSyncTestEnv(ctrl) + + _, s := e.Remote.(*mock_remote.MockRemote), e.Store.(*mock_store.MockStore) + s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil) + + m := New(e, "").(*mscalendar) + jobSummary := &StatusSyncJobSummary{} + + _, _, err := m.retrieveUsersToSync(userIndex, jobSummary, true) + require.Error(t, err, errNoUsersNeedToBeSynced) + }) + + t.Run("user should be reminded", func(t *testing.T) { + t.Skip() // TODO: #gcal got blocked a bit on here, will continue later + testUser := newTestUser() + testUser.Settings.UpdateStatus = true + testUser.Settings.ReceiveReminders = true + + userIndex := []*store.UserShort{ + { + MattermostUserID: testUser.MattermostUserID, + RemoteID: testUser.Remote.ID, + Email: testUser.Remote.Mail, + }, + } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + e, client := makeStatusSyncTestEnv(ctrl) + + c, r, _, s, papi := client.(*mock_remote.MockClient), e.Remote.(*mock_remote.MockRemote), e.Poster.(*mock_bot.MockPoster), e.Store.(*mock_store.MockStore), e.PluginAPI.(*mock_plugin_api.MockPluginAPI) + s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil).Times(2) + events := []*remote.Event{newTestEvent("", "test")} + papi.EXPECT().GetMattermostUser(testUser.MattermostUserID) + r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token) + + c.EXPECT().GetEventsBetweenDates(testUser.Remote.ID, gomock.Any(), gomock.Any()).Return(events, nil) + + m := New(e, "").(*mscalendar) + jobSummary := &StatusSyncJobSummary{} + + _, _, err := m.retrieveUsersToSync(userIndex, jobSummary, true) + require.Error(t, err, errNoUsersNeedToBeSynced) + }) +} + func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { s := mock_store.NewMockStore(ctrl) poster := mock_bot.NewMockPoster(ctrl) @@ -347,15 +447,5 @@ func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { }, } - s.EXPECT().LoadUserIndex().Return(store.UserIndex{ - &store.UserShort{ - MattermostUserID: "user_mm_id", - RemoteID: "user_remote_id", - Email: "user_email@example.com", - }, - }, nil).Times(1) - - mockRemote.EXPECT().MakeSuperuserClient(context.Background()).Return(mockClient, nil) - return env, mockClient } From c3336e9fbe8d3abd8104700b8c62083e16d001a0 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 15:14:33 +0200 Subject: [PATCH 26/41] refactored daily summary logic --- server/mscalendar/daily_summary.go | 129 +++++++++++------------------ 1 file changed, 50 insertions(+), 79 deletions(-) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index c0884506..f586fd4b 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -93,68 +93,6 @@ func (m *mscalendar) SetDailySummaryEnabled(user *User, enable bool) (*store.Dai return dsum, nil } -func (m *mscalendar) processAllDailySummaryWithIndividualCredentials(now time.Time, userIndex store.UserIndex) error { - for _, user := range userIndex { - storeUser, storeErr := m.Store.LoadUser(user.MattermostUserID) - if storeErr != nil { - m.Logger.Warnf("Error loading user %s for daily summary. err=%v", user.MattermostUserID, storeErr) - continue - } - - dsum := storeUser.Settings.DailySummary - if dsum == nil { - continue - } - - shouldPost, shouldPostErr := shouldPostDailySummary(dsum, now) - if shouldPostErr != nil { - m.Logger.Warnf("Error checking should post daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) - continue - } - if !shouldPost { - continue - } - - u := NewUser(user.MattermostUserID) - if err := m.ExpandUser(u); err != nil { - m.Logger.With(bot.LogContext{ - "mattermost_id": storeUser.MattermostUserID, - "remote_id": storeUser.Remote.ID, - "err": err, - }).Errorf("error getting user information") - continue - } - - m.Filter(withActingUser(u.MattermostUserID)) - - postStr, err := m.GetDailySummaryForUser(u) - if err != nil { - m.Logger.With(bot.LogContext{ - "mattermost_id": storeUser.MattermostUserID, - "remote_id": storeUser.Remote.ID, - "err": err, - }).Errorf("error getting daily summary for user") - continue - } - - if _, err := m.Poster.DM(user.MattermostUserID, postStr); err != nil { - m.Logger.With(bot.LogContext{ - "user": storeUser.MattermostUserID, - "err": err, - }).Errorf("error posting daily summary for user") - continue - } - - // REVIEW: Seems kind of pointless to track a passive event like this - m.Dependencies.Tracker.TrackDailySummarySent(user.MattermostUserID) - - dsum.LastPostTime = time.Now().Format(time.RFC3339) - err = m.Store.StoreUser(storeUser) - } - - return nil -} - func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { userIndex, err := m.Store.LoadUserIndex() if err != nil { @@ -165,15 +103,13 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { } err = m.Filter(withSuperuserClient) - // Allow processing the daily summary using individual credentials for remotes that don't allow - // "superuser" access - if errors.Is(err, remote.ErrSuperUserClientNotSupported) { - return m.processAllDailySummaryWithIndividualCredentials(now, userIndex) - } - if err != nil { + if err != nil && !errors.Is(err, remote.ErrSuperUserClientNotSupported) { return err } + fetchIndividually := errors.Is(err, remote.ErrSuperUserClientNotSupported) + + calendarViews := []*remote.ViewCalendarResponse{} requests := []*remote.ViewCalendarParams{} byRemoteID := map[string]*store.User{} for _, user := range userIndex { @@ -198,21 +134,56 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { continue } - start, end := getTodayHoursForTimezone(now, dsum.Timezone) - req := &remote.ViewCalendarParams{ - RemoteUserID: storeUser.Remote.ID, - StartTime: start, - EndTime: end, + if fetchIndividually { + u := NewUser(user.MattermostUserID) + if err := m.ExpandUser(u); err != nil { + m.Logger.With(bot.LogContext{ + "mattermost_id": storeUser.MattermostUserID, + "remote_id": storeUser.Remote.ID, + "err": err, + }).Errorf("error getting user information") + continue + } + + m.Filter(withActingUser(storeUser.MattermostUserID)) + + tz, err := m.GetTimezone(u) + if err != nil { + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + continue + } + + events, err := m.getTodayCalendarEvents(u, now, tz) + if err != nil { + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + continue + } + + calendarViews = append(calendarViews, &remote.ViewCalendarResponse{ + Error: nil, + RemoteUserID: storeUser.Remote.ID, + Events: events, + }) + } else { + start, end := getTodayHoursForTimezone(now, dsum.Timezone) + req := &remote.ViewCalendarParams{ + RemoteUserID: storeUser.Remote.ID, + StartTime: start, + EndTime: end, + } + requests = append(requests, req) } - requests = append(requests, req) } - responses, err := m.client.DoBatchViewCalendarRequests(requests) - if err != nil { - return err + if !fetchIndividually { + var err error + calendarViews, err = m.client.DoBatchViewCalendarRequests(requests) + if err != nil { + return err + } } - for _, res := range responses { + for _, res := range calendarViews { user := byRemoteID[res.RemoteUserID] if res.Error != nil { m.Logger.Warnf("Error rendering user %s calendar. err=%s %s", user.MattermostUserID, res.Error.Code, res.Error.Message) @@ -238,7 +209,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { } } - m.Logger.Infof("Processed daily summary for %d users", len(responses)) + m.Logger.Infof("Processed daily summary for %d users", len(calendarViews)) return nil } From e4f73fd6b74b22997250ec02478960f33a1e43cb Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 16:32:57 +0200 Subject: [PATCH 27/41] test: availability --- server/mscalendar/availability.go | 12 ++++++++- server/mscalendar/availability_test.go | 36 ++++++++++++++------------ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 0d82a4a2..9ed3be9d 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -5,6 +5,7 @@ package mscalendar import ( "fmt" + "log" "time" "github.com/mattermost/mattermost-server/v6/model" @@ -75,7 +76,12 @@ func (m *mscalendar) SyncAll() (string, *StatusSyncJobSummary, error) { return "", &StatusSyncJobSummary{}, errors.Wrap(err, "not able to filter the super user client") } - return m.syncUsers(userIndex, errors.Is(err, remote.ErrSuperUserClientNotSupported)) + result, jobSummary, err := m.syncUsers(userIndex, errors.Is(err, remote.ErrSuperUserClientNotSupported)) + if result != "" && err != nil { + return result, jobSummary, nil + } + + return result, jobSummary, err } // retrieveUsersToSync retrieves the users and their calendar data to sync up and send notifications @@ -446,6 +452,10 @@ func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remot return nil, errors.Wrap(err, "errror withClient") } + log.Println(m.client) + log.Println(user) + log.Println(user.Remote) + events, err := m.client.GetEventsBetweenDates(user.Remote.ID, start, end) if err != nil { return nil, errors.Wrapf(err, "error getting events for user %s", user.MattermostUserID) diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index fc8feec4..fea7b525 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -186,8 +186,9 @@ func TestSyncStatusUserConfig(t *testing.T) { UpdateStatus: false, }, runAssertions: func(deps *Dependencies, client remote.Client) { - c := client.(*mock_remote.MockClient) + c, r := client.(*mock_remote.MockClient), deps.Remote.(*mock_remote.MockRemote) c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Times(0) + r.EXPECT().MakeSuperuserClient(gomock.Any()) }, }, "UpdateStatus enabled and GetConfirmation enabled": { @@ -197,17 +198,9 @@ func TestSyncStatusUserConfig(t *testing.T) { }, runAssertions: func(deps *Dependencies, client remote.Client) { c, r, papi, poster, s := client.(*mock_remote.MockClient), deps.Remote.(*mock_remote.MockRemote), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Poster.(*mock_bot.MockPoster), deps.Store.(*mock_store.MockStore) - s.EXPECT().LoadUserIndex().Return(store.UserIndex{ - &store.UserShort{ - MattermostUserID: "user_mm_id", - RemoteID: "user_remote_id", - Email: "user_email@example.com", - }, - }, nil).Times(1) r.EXPECT().MakeSuperuserClient(context.Background()).Return(client, nil) moment := time.Now().UTC() busyEvent := &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} - c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Times(1).Return([]*remote.ViewCalendarResponse{ {Events: []*remote.Event{busyEvent}, RemoteUserID: "user_remote_id"}, }, nil) @@ -227,6 +220,13 @@ func TestSyncStatusUserConfig(t *testing.T) { env, client := makeStatusSyncTestEnv(ctrl) s := env.Dependencies.Store.(*mock_store.MockStore) + s.EXPECT().LoadUserIndex().Return(store.UserIndex{ + &store.UserShort{ + MattermostUserID: "user_mm_id", + RemoteID: "user_remote_id", + Email: "user_email@example.com", + }, + }, nil).Times(1) s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ MattermostUserID: "user_mm_id", Remote: &remote.User{ @@ -384,7 +384,7 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { e, _ := makeStatusSyncTestEnv(ctrl) - _, s := e.Remote.(*mock_remote.MockRemote), e.Store.(*mock_store.MockStore) + s := e.Store.(*mock_store.MockStore) s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil) m := New(e, "").(*mscalendar) @@ -394,8 +394,7 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { require.Error(t, err, errNoUsersNeedToBeSynced) }) - t.Run("user should be reminded", func(t *testing.T) { - t.Skip() // TODO: #gcal got blocked a bit on here, will continue later + t.Run("one user should be synced", func(t *testing.T) { testUser := newTestUser() testUser.Settings.UpdateStatus = true testUser.Settings.ReceiveReminders = true @@ -414,17 +413,22 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { c, r, _, s, papi := client.(*mock_remote.MockClient), e.Remote.(*mock_remote.MockRemote), e.Poster.(*mock_bot.MockPoster), e.Store.(*mock_store.MockStore), e.PluginAPI.(*mock_plugin_api.MockPluginAPI) s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil).Times(2) + events := []*remote.Event{newTestEvent("", "test")} papi.EXPECT().GetMattermostUser(testUser.MattermostUserID) - r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token) - + r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token).Return(client) c.EXPECT().GetEventsBetweenDates(testUser.Remote.ID, gomock.Any(), gomock.Any()).Return(events, nil) m := New(e, "").(*mscalendar) jobSummary := &StatusSyncJobSummary{} - _, _, err := m.retrieveUsersToSync(userIndex, jobSummary, true) - require.Error(t, err, errNoUsersNeedToBeSynced) + users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) + require.NoError(t, err) + require.Equal(t, users, []*store.User{testUser}) + require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + RemoteUserID: testUser.Remote.ID, + Events: events, + }}) }) } From 9951416104e9bb3a59cd04fcebb6c6b9ffa9000e Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 26 Jul 2023 17:26:18 +0200 Subject: [PATCH 28/41] daily summary tests (wip) --- server/mscalendar/daily_summary.go | 2 +- server/mscalendar/daily_summary_test.go | 117 ++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index f586fd4b..8127e789 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -135,7 +135,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { } if fetchIndividually { - u := NewUser(user.MattermostUserID) + u := NewUser(storeUser.MattermostUserID) if err := m.ExpandUser(u); err != nil { m.Logger.With(bot.LogContext{ "mattermost_id": storeUser.MattermostUserID, diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 92beb8c7..8cfc292d 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -150,6 +150,123 @@ func TestProcessAllDailySummary(t *testing.T) { mockPoster.EXPECT().DM("user2_mm_id", `Times are shown in Pacific Standard Time Wednesday February 12, 2020 +| Time | Subject | +| :--: | :-- | +| 9:00AM - 11:00AM | [The subject]() |`).Return("postID2", nil).Times(1), + ) + + s.EXPECT().StoreUser(gomock.Any()).Times(2).DoAndReturn(func(u *store.User) error { + require.NotEmpty(t, u.Settings.DailySummary.LastPostTime) + return nil + }) + + mockLogger := deps.Logger.(*mock_bot.MockLogger) + mockLogger.EXPECT().Infof("Processed daily summary for %d users", 2) + }, + }, + { + name: "User receives their daily summary (individual data call)", + err: "", + runAssertions: func(deps *Dependencies, client remote.Client) { + user1 := &store.User{ + MattermostUserID: "user1_mm_id", + Remote: &remote.User{ID: "user1_remote_id"}, + Settings: store.Settings{ + DailySummary: &store.DailySummaryUserSettings{ + Enable: true, + PostTime: "9:00AM", + Timezone: "Eastern Standard Time", + LastPostTime: "", + }, + }, + } + user2 := &store.User{ + MattermostUserID: "user2_mm_id", + Remote: &remote.User{ID: "user2_remote_id"}, + Settings: store.Settings{ + DailySummary: &store.DailySummaryUserSettings{ + Enable: true, + PostTime: "6:00AM", + Timezone: "Pacific Standard Time", + LastPostTime: "", + }, + }, + } + user3 := &store.User{ + MattermostUserID: "user3_mm_id", + Remote: &remote.User{ID: "user3_remote_id"}, + Settings: store.Settings{ + DailySummary: &store.DailySummaryUserSettings{ + Enable: false, + PostTime: "10:00AM", // should not receive summary + Timezone: "Pacific Standard Time", + LastPostTime: "", + }, + }, + } + + mockRemote := deps.Remote.(*mock_remote.MockRemote) + papi := deps.PluginAPI.(*mock_plugin_api.MockPluginAPI) + r := deps.Remote.(*mock_remote.MockRemote) + + s := deps.Store.(*mock_store.MockStore) + s.EXPECT().LoadUserIndex().Return(store.UserIndex{{ + MattermostUserID: user1.MattermostUserID, + RemoteID: user1.Remote.ID, + }, { + MattermostUserID: user2.MattermostUserID, + RemoteID: user2.Remote.ID, + }, { + MattermostUserID: user3.MattermostUserID, + RemoteID: user3.Remote.ID, + }}, nil) + + mockRemote.EXPECT().MakeSuperuserClient(context.Background()).Return(nil, remote.ErrSuperUserClientNotSupported).Times(1) + mockClient := client.(*mock_remote.MockClient) + + s.EXPECT().LoadUser(user1.MattermostUserID).Return(user1, nil).Times(2) + s.EXPECT().LoadUser(user2.MattermostUserID).Return(user2, nil).Times(2) + s.EXPECT().LoadUser(user3.MattermostUserID).Return(user3, nil).Times(2) + + papi.EXPECT().GetMattermostUser(user1.MattermostUserID).Times(2) + papi.EXPECT().GetMattermostUser(user2.MattermostUserID).Times(2) + papi.EXPECT().GetMattermostUser(user3.MattermostUserID).Times(2) + + r.EXPECT().MakeClient(context.TODO(), user1.OAuth2Token).Return(mockClient) + r.EXPECT().MakeClient(context.TODO(), user2.OAuth2Token).Return(mockClient) + r.EXPECT().MakeClient(context.TODO(), user3.OAuth2Token).Return(mockClient) + + mockClient.EXPECT().GetMailboxSettings(user1.Remote.ID).Return(&remote.MailboxSettings{ + TimeZone: user1.Settings.DailySummary.Timezone, + }, nil) + mockClient.EXPECT().GetMailboxSettings(user2.Remote.ID).Return(&remote.MailboxSettings{ + TimeZone: user2.Settings.DailySummary.Timezone, + }, nil) + mockClient.EXPECT().GetMailboxSettings(user3.Remote.ID).Return(&remote.MailboxSettings{ + TimeZone: user3.Settings.DailySummary.Timezone, + }, nil) + + loc, err := time.LoadLocation("MST") + require.Nil(t, err) + hour, minute := 10, 0 // Time is "10:00AM" + moment := makeTime(hour, minute, loc) + + mockClient.EXPECT().GetDefaultCalendarView(user1.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) + mockClient.EXPECT().GetDefaultCalendarView(user2.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{ + { + Subject: "The subject", + Start: remote.NewDateTime(moment, "Mountain Standard Time"), + End: remote.NewDateTime(moment.Add(2*time.Hour), "Mountain Standard Time"), + }, + }, nil) + // mockClient.EXPECT().GetDefaultCalendarView(user3.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) + + mockPoster := deps.Poster.(*mock_bot.MockPoster) + gomock.InOrder( + mockPoster.EXPECT().DM(user1.MattermostUserID, "You have no upcoming events.").Return("postID1", nil).Times(1), + mockPoster.EXPECT().DM(user2.MattermostUserID, `Times are shown in Pacific Standard Time +Wednesday February 12, 2020 + | Time | Subject | | :--: | :-- | | 9:00AM - 11:00AM | [The subject]() |`).Return("postID2", nil).Times(1), From a92d5459e51d25586da163baa84bbdfe8308dad9 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 27 Jul 2023 13:31:29 +0200 Subject: [PATCH 29/41] daily summary test --- server/mscalendar/daily_summary.go | 8 +-- server/mscalendar/daily_summary_test.go | 96 +++++++++++-------------- 2 files changed, 47 insertions(+), 57 deletions(-) diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index 8127e789..a55ef140 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -115,7 +115,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { for _, user := range userIndex { storeUser, storeErr := m.Store.LoadUser(user.MattermostUserID) if storeErr != nil { - m.Logger.Warnf("Error loading user %s for daily summary. err=%v", user.MattermostUserID, storeErr) + m.Logger.Warnf("Error loading user %s for daily summary. err=%v", storeUser.MattermostUserID, storeErr) continue } byRemoteID[storeUser.Remote.ID] = storeUser @@ -127,7 +127,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { shouldPost, shouldPostErr := shouldPostDailySummary(dsum, now) if shouldPostErr != nil { - m.Logger.Warnf("Error posting daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + m.Logger.Warnf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, shouldPostErr) continue } if !shouldPost { @@ -149,13 +149,13 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { tz, err := m.GetTimezone(u) if err != nil { - m.Logger.Errorf("Error posting daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, shouldPostErr) continue } events, err := m.getTodayCalendarEvents(u, now, tz) if err != nil { - m.Logger.Errorf("Error posting daily summary for user %s. err=%v", user.MattermostUserID, shouldPostErr) + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, shouldPostErr) continue } diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 8cfc292d..57f93b12 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -168,7 +168,30 @@ Wednesday February 12, 2020 name: "User receives their daily summary (individual data call)", err: "", runAssertions: func(deps *Dependencies, client remote.Client) { - user1 := &store.User{ + s := deps.Store.(*mock_store.MockStore) + mockRemote := deps.Remote.(*mock_remote.MockRemote) + mockClient := client.(*mock_remote.MockClient) + papi := deps.PluginAPI.(*mock_plugin_api.MockPluginAPI) + + loc, err := time.LoadLocation("MST") + require.Nil(t, err) + hour, minute := 10, 0 // Time is "10:00AM" + moment := makeTime(hour, minute, loc) + + s.EXPECT().LoadUserIndex().Return(store.UserIndex{{ + MattermostUserID: "user1_mm_id", + RemoteID: "user1_remote_id", + }, { + MattermostUserID: "user2_mm_id", + RemoteID: "user2_remote_id", + }, { + MattermostUserID: "user3_mm_id", + RemoteID: "user3_remote_id", + }}, nil) + + mockRemote.EXPECT().MakeSuperuserClient(context.Background()).Return(nil, remote.ErrSuperUserClientNotSupported).Times(1) + + s.EXPECT().LoadUser("user1_mm_id").Return(&store.User{ MattermostUserID: "user1_mm_id", Remote: &remote.User{ID: "user1_remote_id"}, Settings: store.Settings{ @@ -179,8 +202,9 @@ Wednesday February 12, 2020 LastPostTime: "", }, }, - } - user2 := &store.User{ + }, nil).Times(3) + + s.EXPECT().LoadUser("user2_mm_id").Return(&store.User{ MattermostUserID: "user2_mm_id", Remote: &remote.User{ID: "user2_remote_id"}, Settings: store.Settings{ @@ -191,80 +215,46 @@ Wednesday February 12, 2020 LastPostTime: "", }, }, - } - user3 := &store.User{ + }, nil).Times(2) + + s.EXPECT().LoadUser("user3_mm_id").Return(&store.User{ MattermostUserID: "user3_mm_id", Remote: &remote.User{ID: "user3_remote_id"}, Settings: store.Settings{ DailySummary: &store.DailySummaryUserSettings{ - Enable: false, + Enable: true, PostTime: "10:00AM", // should not receive summary Timezone: "Pacific Standard Time", LastPostTime: "", }, }, - } - - mockRemote := deps.Remote.(*mock_remote.MockRemote) - papi := deps.PluginAPI.(*mock_plugin_api.MockPluginAPI) - r := deps.Remote.(*mock_remote.MockRemote) - - s := deps.Store.(*mock_store.MockStore) - s.EXPECT().LoadUserIndex().Return(store.UserIndex{{ - MattermostUserID: user1.MattermostUserID, - RemoteID: user1.Remote.ID, - }, { - MattermostUserID: user2.MattermostUserID, - RemoteID: user2.Remote.ID, - }, { - MattermostUserID: user3.MattermostUserID, - RemoteID: user3.Remote.ID, - }}, nil) - - mockRemote.EXPECT().MakeSuperuserClient(context.Background()).Return(nil, remote.ErrSuperUserClientNotSupported).Times(1) - mockClient := client.(*mock_remote.MockClient) - - s.EXPECT().LoadUser(user1.MattermostUserID).Return(user1, nil).Times(2) - s.EXPECT().LoadUser(user2.MattermostUserID).Return(user2, nil).Times(2) - s.EXPECT().LoadUser(user3.MattermostUserID).Return(user3, nil).Times(2) - - papi.EXPECT().GetMattermostUser(user1.MattermostUserID).Times(2) - papi.EXPECT().GetMattermostUser(user2.MattermostUserID).Times(2) - papi.EXPECT().GetMattermostUser(user3.MattermostUserID).Times(2) + }, nil) - r.EXPECT().MakeClient(context.TODO(), user1.OAuth2Token).Return(mockClient) - r.EXPECT().MakeClient(context.TODO(), user2.OAuth2Token).Return(mockClient) - r.EXPECT().MakeClient(context.TODO(), user3.OAuth2Token).Return(mockClient) + papi.EXPECT().GetMattermostUser("user1_mm_id").Times(2) + papi.EXPECT().GetMattermostUser("user2_mm_id").Times(1) - mockClient.EXPECT().GetMailboxSettings(user1.Remote.ID).Return(&remote.MailboxSettings{ - TimeZone: user1.Settings.DailySummary.Timezone, - }, nil) - mockClient.EXPECT().GetMailboxSettings(user2.Remote.ID).Return(&remote.MailboxSettings{ - TimeZone: user2.Settings.DailySummary.Timezone, + mockClient.EXPECT().GetMailboxSettings("user1_remote_id").Return(&remote.MailboxSettings{ + TimeZone: "Eastern Standard Time", }, nil) - mockClient.EXPECT().GetMailboxSettings(user3.Remote.ID).Return(&remote.MailboxSettings{ - TimeZone: user3.Settings.DailySummary.Timezone, + mockClient.EXPECT().GetMailboxSettings("user2_remote_id").Return(&remote.MailboxSettings{ + TimeZone: "Pacific Standard Time", }, nil) - loc, err := time.LoadLocation("MST") - require.Nil(t, err) - hour, minute := 10, 0 // Time is "10:00AM" - moment := makeTime(hour, minute, loc) + mockRemote.EXPECT().MakeClient(context.TODO(), gomock.Any()).Return(mockClient) - mockClient.EXPECT().GetDefaultCalendarView(user1.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) - mockClient.EXPECT().GetDefaultCalendarView(user2.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{ + mockClient.EXPECT().GetDefaultCalendarView("user1_remote_id", gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) + mockClient.EXPECT().GetDefaultCalendarView("user2_remote_id", gomock.Any(), gomock.Any()).Return([]*remote.Event{ { Subject: "The subject", Start: remote.NewDateTime(moment, "Mountain Standard Time"), End: remote.NewDateTime(moment.Add(2*time.Hour), "Mountain Standard Time"), }, }, nil) - // mockClient.EXPECT().GetDefaultCalendarView(user3.Remote.ID, gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) mockPoster := deps.Poster.(*mock_bot.MockPoster) gomock.InOrder( - mockPoster.EXPECT().DM(user1.MattermostUserID, "You have no upcoming events.").Return("postID1", nil).Times(1), - mockPoster.EXPECT().DM(user2.MattermostUserID, `Times are shown in Pacific Standard Time + mockPoster.EXPECT().DM("user1_mm_id", "You have no upcoming events.").Return("postID1", nil).Times(1), + mockPoster.EXPECT().DM("user2_mm_id", `Times are shown in Pacific Standard Time Wednesday February 12, 2020 | Time | Subject | From c0904100b002712c1d50837d6708733d686ee52c Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 27 Jul 2023 13:45:25 +0200 Subject: [PATCH 30/41] fix: merge duplications --- server/remote/gcal/event.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/server/remote/gcal/event.go b/server/remote/gcal/event.go index c8a6b66a..07d15e90 100644 --- a/server/remote/gcal/event.go +++ b/server/remote/gcal/event.go @@ -133,40 +133,6 @@ func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { return out } -func convertRemoteDateTimeToGcalEventDateTime(in *remote.DateTime) *calendar.EventDateTime { - out := &calendar.EventDateTime{} - dt := in.String() - - // Avoid setting non RFC3339 strings - if dt != remote.UndefinedDatetime { - out.DateTime = dt - } - - out.TimeZone = in.TimeZone - - return out - -} - -func convertRemoteEventToGcalEvent(in *remote.Event) *calendar.Event { - out := &calendar.Event{} - out.Summary = in.Subject - out.Start = convertRemoteDateTimeToGcalEventDateTime(in.Start) - out.End = convertRemoteDateTimeToGcalEventDateTime(in.End) - out.Description = in.Body.Content - if in.Location != nil { - out.Location = in.Location.DisplayName - } - - for _, attendee := range in.Attendees { - out.Attendees = append(out.Attendees, &calendar.EventAttendee{ - Email: attendee.EmailAddress.Address, - }) - } - - return out -} - func convertRemoteDateTimeToGcalEventDateTime(in *remote.DateTime) *calendar.EventDateTime { out := &calendar.EventDateTime{} dt := in.String() From ecca7390a060831c1cce6d422578990422caa2a8 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 27 Jul 2023 14:08:28 +0200 Subject: [PATCH 31/41] slack attachment for notifications --- server/mscalendar/availability.go | 5 +++-- server/mscalendar/views/calendar.go | 34 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 9ed3be9d..bfca3632 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -509,12 +509,13 @@ func (m *mscalendar) notifyUpcomingEvents(mattermostUserID string, events []*rem } } - message, err := views.RenderUpcomingEvent(event, timezone) + _, attachment, err := views.RenderUpcomingEventAsAttachment(event, timezone) if err != nil { m.Logger.Warnf("notifyUpcomingEvent error rendering schedule item. err=%v", err) continue } - _, err = m.Poster.DM(mattermostUserID, message) + + _, err = m.Poster.DMWithAttachments(mattermostUserID, attachment) if err != nil { m.Logger.Warnf("notifyUpcomingEvents error creating DM. err=%v", err) continue diff --git a/server/mscalendar/views/calendar.go b/server/mscalendar/views/calendar.go index 8cbbebd1..0fccf4a4 100644 --- a/server/mscalendar/views/calendar.go +++ b/server/mscalendar/views/calendar.go @@ -7,6 +7,7 @@ import ( "time" "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" + "github.com/mattermost/mattermost-server/v6/model" ) func RenderCalendarView(events []*remote.Event, timeZone string) (string, error) { @@ -65,6 +66,33 @@ func renderEvent(event *remote.Event, asRow bool, timeZone string) (string, erro return fmt.Sprintf(format, start, end, subject, link), nil } +func renderEventAsAttachment(event *remote.Event, timezone string) (*model.SlackAttachment, error) { + var actions []*model.PostAction + fields := []*model.SlackAttachmentField{} + + if event.Location != nil && event.Location.DisplayName != "" { + fields = append(fields, &model.SlackAttachmentField{ + Title: "Location", + Value: event.Location.DisplayName, + Short: true, + }) + + // Add actions for known links + // Disable join meeting button for now, since we don't have a handler and + // the location url is shown parsed and clickable anyway. + // if joinMeetingAction := getActionForLocation(event.Location); joinMeetingAction != nil { + // actions = append(actions, joinMeetingAction) + // } + } + + return &model.SlackAttachment{ + Title: event.Subject, + Text: fmt.Sprintf("(%s - %s)", event.Start.In(timezone).Time().Format(time.Kitchen), event.End.In(timezone).Time().Format(time.Kitchen)), + Fields: fields, + Actions: actions, + }, nil +} + func groupEventsByDate(events []*remote.Event) [][]*remote.Event { groups := map[string][]*remote.Event{} @@ -109,3 +137,9 @@ func EnsureSubject(s string) string { return s } + +func RenderUpcomingEventAsAttachment(event *remote.Event, timeZone string) (message string, attachment *model.SlackAttachment, err error) { + message = "You have an upcoming event:\n" + attachment, err = renderEventAsAttachment(event, timeZone) + return message, attachment, nil +} From 6f2b1d7eb2bcaf2ff38a2a48379f49b2656a88e4 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 27 Jul 2023 14:38:14 +0200 Subject: [PATCH 32/41] lint and test --- server/command/view_calendar.go | 2 +- server/mscalendar/availability_test.go | 2 +- server/mscalendar/views/calendar.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/command/view_calendar.go b/server/command/view_calendar.go index d6d88ab7..0fe7d1c0 100644 --- a/server/command/view_calendar.go +++ b/server/command/view_calendar.go @@ -9,7 +9,7 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views" ) -func (c *Command) viewCalendar(parameters ...string) (string, bool, error) { +func (c *Command) viewCalendar(_ ...string) (string, bool, error) { tz, err := c.MSCalendar.GetTimezone(c.user()) if err != nil { return "Error: No timezone found", false, err diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index fea7b525..0e00f652 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -330,7 +330,7 @@ func TestReminders(t *testing.T) { }, nil) if tc.numReminders > 0 { - poster.EXPECT().DM("user_mm_id", gomock.Any()).Times(tc.numReminders) + poster.EXPECT().DMWithAttachments("user_mm_id", gomock.Any()).Times(tc.numReminders) loadUser.Times(2) c.EXPECT().GetMailboxSettings("user_remote_id").Times(1).Return(&remote.MailboxSettings{TimeZone: "UTC"}, nil) } else { diff --git a/server/mscalendar/views/calendar.go b/server/mscalendar/views/calendar.go index 0fccf4a4..6d7e2610 100644 --- a/server/mscalendar/views/calendar.go +++ b/server/mscalendar/views/calendar.go @@ -141,5 +141,5 @@ func EnsureSubject(s string) string { func RenderUpcomingEventAsAttachment(event *remote.Event, timeZone string) (message string, attachment *model.SlackAttachment, err error) { message = "You have an upcoming event:\n" attachment, err = renderEventAsAttachment(event, timeZone) - return message, attachment, nil + return message, attachment, err } From 351730c309a5376894ac2272646e89dee4b94851 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 27 Jul 2023 14:42:04 +0200 Subject: [PATCH 33/41] goimports --- server/mscalendar/views/calendar.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/mscalendar/views/calendar.go b/server/mscalendar/views/calendar.go index 6d7e2610..2729720a 100644 --- a/server/mscalendar/views/calendar.go +++ b/server/mscalendar/views/calendar.go @@ -6,8 +6,9 @@ import ( "sort" "time" - "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" "github.com/mattermost/mattermost-server/v6/model" + + "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" ) func RenderCalendarView(events []*remote.Event, timeZone string) (string, error) { From 7c3b63da89d4103109fc12751aee6c3478a389d5 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Mon, 31 Jul 2023 10:19:19 +0200 Subject: [PATCH 34/41] Update server/mscalendar/availability.go Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com> --- server/mscalendar/availability.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index bfca3632..7dafeb30 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -444,7 +444,7 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { err := m.Filter(withActingUser(user.MattermostUserID)) if err != nil { - return nil, errors.Wrap(err, "error withRemoteUser") + return nil, errors.Wrap(err, "error withActingUser") } err = m.Filter(withClient) From d8745f6fb078de481f6afe6fd303450334854cd8 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 10:20:38 +0200 Subject: [PATCH 35/41] remove logges --- server/mscalendar/availability.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 7dafeb30..5cf22f0c 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -5,7 +5,6 @@ package mscalendar import ( "fmt" - "log" "time" "github.com/mattermost/mattermost-server/v6/model" @@ -444,7 +443,7 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { err := m.Filter(withActingUser(user.MattermostUserID)) if err != nil { - return nil, errors.Wrap(err, "error withActingUser") + return nil, errors.Wrap(err, "error withRemoteUser") } err = m.Filter(withClient) @@ -452,10 +451,6 @@ func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remot return nil, errors.Wrap(err, "errror withClient") } - log.Println(m.client) - log.Println(user) - log.Println(user.Remote) - events, err := m.client.GetEventsBetweenDates(user.Remote.ID, start, end) if err != nil { return nil, errors.Wrapf(err, "error getting events for user %s", user.MattermostUserID) From 58e7b9ff4fd7a8f9bdacac74565b7079381e1543 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 12:11:19 +0200 Subject: [PATCH 36/41] nullify client when changing acting user --- server/mscalendar/filters.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/mscalendar/filters.go b/server/mscalendar/filters.go index 14d8792f..3fb4bde3 100644 --- a/server/mscalendar/filters.go +++ b/server/mscalendar/filters.go @@ -34,6 +34,7 @@ func withRemoteUser(user *User) func(m *mscalendar) error { func withActingUser(mattermostUserID string) func(m *mscalendar) error { return func(m *mscalendar) error { m.actingUser = NewUser(mattermostUserID) + m.client = nil return nil } } From 9aac20fa500f82244a3a1c944a0d99aedf0424b5 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 12:11:54 +0200 Subject: [PATCH 37/41] move withActiveUser filter to fenced code outside of general method --- server/mscalendar/availability.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 5cf22f0c..c4958b18 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -117,6 +117,12 @@ func (m *mscalendar) retrieveUsersToSync(userIndex store.UserIndex, syncJobSumma users = append(users, user) if fetchIndividually { + err = m.Filter(withActingUser(user.MattermostUserID)) + if err != nil { + m.Logger.Warnf("Not able to enable active user %s from user index. err=%v", user.MattermostUserID, err) + continue + } + calendarUser := newUserFromStoredUser(user) calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) if err != nil { @@ -441,12 +447,7 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, currentStatus *model.S } func (m *mscalendar) GetCalendarEvents(user *User, start, end time.Time) (*remote.ViewCalendarResponse, error) { - err := m.Filter(withActingUser(user.MattermostUserID)) - if err != nil { - return nil, errors.Wrap(err, "error withRemoteUser") - } - - err = m.Filter(withClient) + err := m.Filter(withClient) if err != nil { return nil, errors.Wrap(err, "errror withClient") } From c9378ceda2aea6fa78e7cad7ef0369b32fd13e0a Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 12:12:05 +0200 Subject: [PATCH 38/41] tests: handling more scenarios --- server/mscalendar/availability_test.go | 101 ++++++++++++++++++++++++- server/mscalendar/notification_test.go | 23 +++--- 2 files changed, 114 insertions(+), 10 deletions(-) diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 0e00f652..84cb5fa4 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -414,7 +414,7 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { c, r, _, s, papi := client.(*mock_remote.MockClient), e.Remote.(*mock_remote.MockRemote), e.Poster.(*mock_bot.MockPoster), e.Store.(*mock_store.MockStore), e.PluginAPI.(*mock_plugin_api.MockPluginAPI) s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil).Times(2) - events := []*remote.Event{newTestEvent("", "test")} + events := []*remote.Event{newTestEvent("1", "", "test")} papi.EXPECT().GetMattermostUser(testUser.MattermostUserID) r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token).Return(client) c.EXPECT().GetEventsBetweenDates(testUser.Remote.ID, gomock.Any(), gomock.Any()).Return(events, nil) @@ -430,6 +430,105 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { Events: events, }}) }) + + t.Run("one user should be synced, one user shouldn't", func(t *testing.T) { + testUser := newTestUser() + testUser.Settings.UpdateStatus = true + testUser.Settings.ReceiveReminders = true + + testUser2 := newTestUserNumbered(1) + + userIndex := []*store.UserShort{ + { + MattermostUserID: testUser.MattermostUserID, + RemoteID: testUser.Remote.ID, + Email: testUser.Remote.Mail, + }, + { + MattermostUserID: testUser2.MattermostUserID, + RemoteID: testUser2.Remote.ID, + Email: testUser2.Remote.Mail, + }, + } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + e, client := makeStatusSyncTestEnv(ctrl) + + c, r, _, s, papi := client.(*mock_remote.MockClient), e.Remote.(*mock_remote.MockRemote), e.Poster.(*mock_bot.MockPoster), e.Store.(*mock_store.MockStore), e.PluginAPI.(*mock_plugin_api.MockPluginAPI) + s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil).Times(2) + s.EXPECT().LoadUser(testUser2.MattermostUserID).Return(testUser2, nil) + + events := []*remote.Event{newTestEvent("1", "", "test")} + papi.EXPECT().GetMattermostUser(testUser.MattermostUserID) + r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token).Return(client) + c.EXPECT().GetEventsBetweenDates(testUser.Remote.ID, gomock.Any(), gomock.Any()).Return(events, nil) + + m := New(e, "").(*mscalendar) + jobSummary := &StatusSyncJobSummary{} + + users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) + require.NoError(t, err) + require.Equal(t, users, []*store.User{testUser}) + require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + RemoteUserID: testUser.Remote.ID, + Events: events, + }}) + }) + + t.Run("two users should be synced", func(t *testing.T) { + testUser := newTestUserNumbered(1) + testUser.Settings.UpdateStatus = true + testUser.Settings.ReceiveReminders = true + + testUser2 := newTestUserNumbered(2) + testUser2.Settings.UpdateStatus = true + testUser2.Settings.ReceiveReminders = true + + userIndex := []*store.UserShort{ + { + MattermostUserID: testUser.MattermostUserID, + RemoteID: testUser.Remote.ID, + Email: testUser.Remote.Mail, + }, + { + MattermostUserID: testUser2.MattermostUserID, + RemoteID: testUser2.Remote.ID, + Email: testUser2.Remote.Mail, + }, + } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + e, client := makeStatusSyncTestEnv(ctrl) + + c, r, _, s, papi := client.(*mock_remote.MockClient), e.Remote.(*mock_remote.MockRemote), e.Poster.(*mock_bot.MockPoster), e.Store.(*mock_store.MockStore), e.PluginAPI.(*mock_plugin_api.MockPluginAPI) + s.EXPECT().LoadUser(testUser.MattermostUserID).Return(testUser, nil).Times(2) + s.EXPECT().LoadUser(testUser2.MattermostUserID).Return(testUser2, nil).Times(2) + + eventsUser1 := []*remote.Event{newTestEvent("1", "", "test")} + eventsUser2 := []*remote.Event{newTestEvent("2", "", "test2")} + papi.EXPECT().GetMattermostUser(testUser.MattermostUserID) + papi.EXPECT().GetMattermostUser(testUser2.MattermostUserID) + r.EXPECT().MakeClient(gomock.Any(), testUser.OAuth2Token).Return(client) + r.EXPECT().MakeClient(gomock.Any(), testUser2.OAuth2Token).Return(client) + c.EXPECT().GetEventsBetweenDates(testUser.Remote.ID, gomock.Any(), gomock.Any()).Return(eventsUser1, nil) + c.EXPECT().GetEventsBetweenDates(testUser2.Remote.ID, gomock.Any(), gomock.Any()).Return(eventsUser2, nil) + + m := New(e, "").(*mscalendar) + jobSummary := &StatusSyncJobSummary{} + + users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) + require.NoError(t, err) + require.Equal(t, users, []*store.User{testUser, testUser2}) + require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + RemoteUserID: testUser.Remote.ID, + Events: eventsUser1, + }, { + RemoteUserID: testUser2.Remote.ID, + Events: eventsUser2, + }}) + }) } func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { diff --git a/server/mscalendar/notification_test.go b/server/mscalendar/notification_test.go index ee74149e..0ba83286 100644 --- a/server/mscalendar/notification_test.go +++ b/server/mscalendar/notification_test.go @@ -5,6 +5,7 @@ package mscalendar import ( "context" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -29,10 +30,10 @@ func newTestNotificationProcessor(env Env) NotificationProcessor { return processor } -func newTestEvent(locationDisplayName string, subjectDisplayName string) *remote.Event { +func newTestEvent(identifier, locationDisplayName string, subjectDisplayName string) *remote.Event { return &remote.Event{ - ID: "remote_event_id", - ICalUID: "remote_event_uid", + ID: fmt.Sprintf("remote_event_id_%s", identifier), + ICalUID: fmt.Sprintf("remote_event_uid_%s", identifier), Organizer: &remote.Attendee{ EmailAddress: &remote.EmailAddress{ Address: "event_organizer_email", @@ -65,15 +66,19 @@ func newTestSubscription() *store.Subscription { } func newTestUser() *store.User { + return newTestUserNumbered(1) +} + +func newTestUserNumbered(number int) *store.User { return &store.User{ Settings: store.Settings{ - EventSubscriptionID: "remote_subscription_id", + EventSubscriptionID: fmt.Sprintf("remote_subscription_id_%d", number), }, - Remote: &remote.User{ID: "remote_user_id"}, + Remote: &remote.User{ID: fmt.Sprintf("remote_user_id_%d", number)}, OAuth2Token: &oauth2.Token{ - AccessToken: "creator_oauth_token", + AccessToken: fmt.Sprintf("creator_oauth_token_%d", number), }, - MattermostUserID: "creator_mm_id", + MattermostUserID: fmt.Sprintf("creator_mm_id_%d", number), } } @@ -83,7 +88,7 @@ func newTestNotification(clientState string, recommendRenew bool) *remote.Notifi SubscriptionID: "remote_subscription_id", IsBare: true, SubscriptionCreator: &remote.User{}, - Event: newTestEvent("event_location_display_name", "event_subject"), + Event: newTestEvent("1", "event_location_display_name", "event_subject"), Subscription: &remote.Subscription{}, ClientState: clientState, RecommendRenew: recommendRenew, @@ -114,7 +119,7 @@ func TestProcessNotification(t *testing.T) { name: "prior event exists", expectedError: "", notification: newTestNotification("stored_client_state", false), - priorEvent: newTestEvent("prior_event_location_display_name", "other_event_subject"), + priorEvent: newTestEvent("1", "prior_event_location_display_name", "other_event_subject"), }, { name: "sub renewal recommended", expectedError: "", From b139ac5aff08da58f83d9d0d62e5bbfd0203f8ce Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 12:22:31 +0200 Subject: [PATCH 39/41] Fixed test after bugfix --- server/mscalendar/daily_summary_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 57f93b12..e1d7df22 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -215,7 +215,7 @@ Wednesday February 12, 2020 LastPostTime: "", }, }, - }, nil).Times(2) + }, nil).Times(3) s.EXPECT().LoadUser("user3_mm_id").Return(&store.User{ MattermostUserID: "user3_mm_id", @@ -231,7 +231,7 @@ Wednesday February 12, 2020 }, nil) papi.EXPECT().GetMattermostUser("user1_mm_id").Times(2) - papi.EXPECT().GetMattermostUser("user2_mm_id").Times(1) + papi.EXPECT().GetMattermostUser("user2_mm_id").Times(2) mockClient.EXPECT().GetMailboxSettings("user1_remote_id").Return(&remote.MailboxSettings{ TimeZone: "Eastern Standard Time", @@ -240,7 +240,7 @@ Wednesday February 12, 2020 TimeZone: "Pacific Standard Time", }, nil) - mockRemote.EXPECT().MakeClient(context.TODO(), gomock.Any()).Return(mockClient) + mockRemote.EXPECT().MakeClient(context.TODO(), gomock.Any()).Return(mockClient).Times(2) mockClient.EXPECT().GetDefaultCalendarView("user1_remote_id", gomock.Any(), gomock.Any()).Return([]*remote.Event{}, nil) mockClient.EXPECT().GetDefaultCalendarView("user2_remote_id", gomock.Any(), gomock.Any()).Return([]*remote.Event{ From 08b039ee9e8bf1e59be1a8e9b4cdc8ffe8677e23 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 13:04:17 +0200 Subject: [PATCH 40/41] fixed test assertions --- server/mscalendar/availability.go | 2 +- server/mscalendar/availability_test.go | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index c4958b18..5ed1cfa5 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -137,7 +137,7 @@ func (m *mscalendar) retrieveUsersToSync(userIndex store.UserIndex, syncJobSumma } } if len(users) == 0 { - return users, calendarViews, fmt.Errorf("no users need to be synced") + return users, calendarViews, errNoUsersNeedToBeSynced } if !fetchIndividually { diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 84cb5fa4..4405a173 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -363,7 +363,7 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { jobSummary := &StatusSyncJobSummary{} _, _, err := m.retrieveUsersToSync([]*store.UserShort{}, jobSummary, true) - require.Error(t, err, errNoUsersNeedToBeSynced) + require.ErrorIs(t, errNoUsersNeedToBeSynced, err) }) t.Run("user reminders and status disabled", func(t *testing.T) { @@ -391,7 +391,7 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { jobSummary := &StatusSyncJobSummary{} _, _, err := m.retrieveUsersToSync(userIndex, jobSummary, true) - require.Error(t, err, errNoUsersNeedToBeSynced) + require.ErrorIs(t, err, errNoUsersNeedToBeSynced) }) t.Run("one user should be synced", func(t *testing.T) { @@ -424,11 +424,11 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) require.NoError(t, err) - require.Equal(t, users, []*store.User{testUser}) - require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + require.Equal(t, []*store.User{testUser}, users) + require.Equal(t, []*remote.ViewCalendarResponse{{ RemoteUserID: testUser.Remote.ID, Events: events, - }}) + }}, responses) }) t.Run("one user should be synced, one user shouldn't", func(t *testing.T) { @@ -469,11 +469,11 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) require.NoError(t, err) - require.Equal(t, users, []*store.User{testUser}) - require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + require.Equal(t, []*store.User{testUser}, users) + require.Equal(t, []*remote.ViewCalendarResponse{{ RemoteUserID: testUser.Remote.ID, Events: events, - }}) + }}, responses) }) t.Run("two users should be synced", func(t *testing.T) { @@ -520,14 +520,14 @@ func TestRetrieveUsersToSyncIndividually(t *testing.T) { users, responses, err := m.retrieveUsersToSync(userIndex, jobSummary, true) require.NoError(t, err) - require.Equal(t, users, []*store.User{testUser, testUser2}) - require.Equal(t, responses, []*remote.ViewCalendarResponse{{ + require.Equal(t, []*store.User{testUser, testUser2}, users) + require.Equal(t, []*remote.ViewCalendarResponse{{ RemoteUserID: testUser.Remote.ID, Events: eventsUser1, }, { RemoteUserID: testUser2.Remote.ID, Events: eventsUser2, - }}) + }}, responses) }) } From b240b85ebc4bfca843811e3ff1a1146ddf9db65c Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 31 Jul 2023 15:19:41 +0200 Subject: [PATCH 41/41] allow engine copy to perform safe state mutations --- server/mscalendar/availability.go | 4 ++-- server/mscalendar/daily_summary.go | 14 +++++++++----- server/mscalendar/filters.go | 12 ++++++++++++ server/mscalendar/mscalendar.go | 11 +++++++++++ server/mscalendar/mscalendar_test.go | 23 +++++++++++++++++++++++ 5 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 server/mscalendar/mscalendar_test.go diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 5ed1cfa5..94afb359 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -117,14 +117,14 @@ func (m *mscalendar) retrieveUsersToSync(userIndex store.UserIndex, syncJobSumma users = append(users, user) if fetchIndividually { - err = m.Filter(withActingUser(user.MattermostUserID)) + engine, err := m.FilterCopy(withActingUser(user.MattermostUserID)) if err != nil { m.Logger.Warnf("Not able to enable active user %s from user index. err=%v", user.MattermostUserID, err) continue } calendarUser := newUserFromStoredUser(user) - calendarEvents, err := m.GetCalendarEvents(calendarUser, start, end) + calendarEvents, err := engine.GetCalendarEvents(calendarUser, start, end) if err != nil { syncJobSummary.NumberOfUsersFailedStatusChanged++ m.Logger.With(bot.LogContext{ diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index a55ef140..9fc1cdc3 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -145,17 +145,21 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { continue } - m.Filter(withActingUser(storeUser.MattermostUserID)) + engine, err := m.FilterCopy(withActingUser(storeUser.MattermostUserID)) + if err != nil { + m.Logger.Errorf("Error creating user engine %s. err=%v", storeUser.MattermostUserID, err) + continue + } - tz, err := m.GetTimezone(u) + tz, err := engine.GetTimezone(u) if err != nil { - m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, shouldPostErr) + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, err) continue } - events, err := m.getTodayCalendarEvents(u, now, tz) + events, err := engine.getTodayCalendarEvents(u, now, tz) if err != nil { - m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, shouldPostErr) + m.Logger.Errorf("Error posting daily summary for user %s. err=%v", storeUser.MattermostUserID, err) continue } diff --git a/server/mscalendar/filters.go b/server/mscalendar/filters.go index 3fb4bde3..3070524a 100644 --- a/server/mscalendar/filters.go +++ b/server/mscalendar/filters.go @@ -3,6 +3,8 @@ package mscalendar +import "github.com/pkg/errors" + type filterf func(*mscalendar) error func (m *mscalendar) Filter(filters ...filterf) error { @@ -15,6 +17,16 @@ func (m *mscalendar) Filter(filters ...filterf) error { return nil } +// FilterCopy creates a copy of the calendar engine and applies filters to it +func (m *mscalendar) FilterCopy(filters ...filterf) (*mscalendar, error) { + engine := m.copy() + if err := engine.Filter(filters...); err != nil { + return nil, errors.Wrap(err, "error filtering engine copy") + } + + return engine, nil +} + func withActingUserExpanded(m *mscalendar) error { return m.ExpandUser(m.actingUser) } diff --git a/server/mscalendar/mscalendar.go b/server/mscalendar/mscalendar.go index 5c80871a..28cc6592 100644 --- a/server/mscalendar/mscalendar.go +++ b/server/mscalendar/mscalendar.go @@ -60,6 +60,17 @@ type mscalendar struct { client remote.Client } +// copy returns a copy of the calendar engine +func (m mscalendar) copy() *mscalendar { + user := *m.actingUser + client := m.client + return &mscalendar{ + Env: m.Env, + actingUser: &user, + client: client, + } +} + func New(env Env, actingMattermostUserID string) MSCalendar { return &mscalendar{ Env: env, diff --git a/server/mscalendar/mscalendar_test.go b/server/mscalendar/mscalendar_test.go new file mode 100644 index 00000000..033a264f --- /dev/null +++ b/server/mscalendar/mscalendar_test.go @@ -0,0 +1,23 @@ +package mscalendar + +import ( + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" +) + +func TestCopy(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + user := newTestUserNumbered(1) + env, _ := makeStatusSyncTestEnv(ctrl) + + engine := New(env, user.MattermostUserID) + + engineCopy := engine.(*mscalendar).copy() + + assert.NotSame(t, engine, engineCopy) + assert.NotSame(t, engine.(*mscalendar).actingUser, engineCopy.actingUser) + assert.NotSame(t, engine.(*mscalendar).client, engineCopy.client) +}