From 6c76941c6530e2974e058a7c96b925738ee6d7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 3 Apr 2020 17:38:40 +0200 Subject: [PATCH 01/20] Add status change confirmation --- server/api/api.go | 1 + server/api/post_action.go | 47 +++++++++++++++++++++++ server/config/const.go | 21 ++++++----- server/mscalendar/availability.go | 62 +++++++++++++++++++++++++++++++ server/mscalendar/user.go | 26 +++++++++++++ server/store/user_store.go | 11 +++--- 6 files changed, 153 insertions(+), 15 deletions(-) diff --git a/server/api/api.go b/server/api/api.go index 137e113a..19fdcfbe 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -31,4 +31,5 @@ func Init(h *httputils.Handler, env mscalendar.Env, notificationProcessor mscale postActionRouter.HandleFunc(config.PathDecline, api.postActionDecline).Methods("POST") postActionRouter.HandleFunc(config.PathTentative, api.postActionTentative).Methods("POST") postActionRouter.HandleFunc(config.PathRespond, api.postActionRespond).Methods("POST") + postActionRouter.HandleFunc(config.PathConfirmStatusChange, api.postActionConfirmStatusChange).Methods("POST") } diff --git a/server/api/post_action.go b/server/api/post_action.go index e2e24c05..afeb564a 100644 --- a/server/api/post_action.go +++ b/server/api/post_action.go @@ -4,6 +4,7 @@ package api import ( + "fmt" "net/http" "github.com/mattermost/mattermost-server/v5/model" @@ -81,3 +82,49 @@ func (api *api) postActionRespond(w http.ResponseWriter, req *http.Request) { return } } + +func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.Request) { + mattermostUserID := req.Header.Get("Mattermost-User-ID") + if mattermostUserID == "" { + http.Error(w, "Not authorized", http.StatusUnauthorized) + return + } + + response := model.PostActionIntegrationResponse{} + post := &model.Post{} + + request := model.PostActionIntegrationRequestFromJson(req.Body) + if request == nil { + http.Error(w, "invalid request", http.StatusBadRequest) + return + } + + value, ok := request.Context["value"] + if !ok { + http.Error(w, "No value", http.StatusBadRequest) + return + } + + returnText := "The status has not been changed." + if value.(bool) { + changeTo, ok := request.Context["change_to"] + if !ok { + http.Error(w, "No state to change", http.StatusBadRequest) + return + } + stringChangeTo := changeTo.(string) + api.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, stringChangeTo) + returnText = fmt.Sprintf("The status has been changed to %s.", stringChangeTo) + } + + sa := &model.SlackAttachment{ + Title: "Status Change", + Text: returnText, + } + + model.ParseSlackAttachment(post, []*model.SlackAttachment{sa}) + + response.Update = post + w.Header().Set("Content-Type", "application/json") + w.Write(response.ToJson()) +} diff --git a/server/config/const.go b/server/config/const.go index 2192f3ad..145a4e44 100644 --- a/server/config/const.go +++ b/server/config/const.go @@ -12,16 +12,17 @@ const ( Repository = "mattermost-plugin-mscalendar" CommandTrigger = "mscalendar" - PathOAuth2 = "/oauth2" - PathComplete = "/complete" - PathAPI = "/api/v1" - PathPostAction = "/action" - PathRespond = "/respond" - PathAccept = "/accept" - PathDecline = "/decline" - PathTentative = "/tentative" - PathNotification = "/notification/v1" - PathEvent = "/event" + PathOAuth2 = "/oauth2" + PathComplete = "/complete" + PathAPI = "/api/v1" + PathPostAction = "/action" + PathRespond = "/respond" + PathAccept = "/accept" + PathDecline = "/decline" + PathTentative = "/tentative" + PathConfirmStatusChange = "/confirm" + PathNotification = "/notification/v1" + PathEvent = "/event" FullPathEventNotification = PathNotification + PathEvent FullPathOAuth2Redirect = PathOAuth2 + PathComplete diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 020bc33d..8d197159 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -7,9 +7,11 @@ import ( "fmt" "time" + "github.com/mattermost/mattermost-plugin-mscalendar/server/config" "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-server/v5/model" ) const ( @@ -138,10 +140,29 @@ func (m *mscalendar) GetAvailabilities(remoteUserID string, scheduleIDs []string func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus string, av remote.AvailabilityView) string { currentAvailability := av[0] + u := NewUser(mattermostUserID) + + if !m.ShouldChangeStatus(u) { + return "Status not changed by user configuration." + } + + if !m.HasAvailabilityChanged(u, currentAvailability) { + return "Status not changed because there is no update since last status change." + } + + u.LastStatusUpdateAvailability = currentAvailability + m.Store.StoreUser(u.User) + + url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) switch currentAvailability { case remote.AvailabilityViewFree: if currentStatus == "dnd" { + if m.ShouldNotifyStatusChange(u) { + userQuestion := fmt.Sprintf("Shall we change your status from %s to online?", currentStatus) + m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "online", url)) + return fmt.Sprintf("User asked for status change from %s to online", currentStatus) + } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "online") return fmt.Sprintf("User is free. Setting user from %s to online.", currentStatus) } else { @@ -149,6 +170,11 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } case remote.AvailabilityViewTentative, remote.AvailabilityViewBusy: if currentStatus != "dnd" { + if m.ShouldNotifyStatusChange(u) { + userQuestion := fmt.Sprintf("Shall we change your status from %s to dnd?", currentStatus) + m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "dnd", url)) + return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) + } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "dnd") return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) } else { @@ -156,6 +182,11 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } case remote.AvailabilityViewOutOfOffice: if currentStatus != "offline" { + if m.ShouldNotifyStatusChange(u) { + userQuestion := fmt.Sprintf("Shall we change your status from %s to offline?", currentStatus) + m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "offline", url)) + return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) + } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "offline") return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) } else { @@ -167,3 +198,34 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s return fmt.Sprintf("Availability view doesn't match %d", currentAvailability) } + +func statusChangeAttachments(message, to, url string) *model.SlackAttachment { + actionYes := &model.PostAction{ + Name: "Yes", + Integration: &model.PostActionIntegration{ + URL: url, + Context: map[string]interface{}{ + "value": true, + "change_to": to, + }, + }, + } + + actionNo := &model.PostAction{ + Name: "No", + Integration: &model.PostActionIntegration{ + URL: url, + Context: map[string]interface{}{ + "value": false, + }, + }, + } + + sa := &model.SlackAttachment{ + Title: "Status change", + Text: message, + Actions: []*model.PostAction{actionYes, actionNo}, + } + + return sa +} diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index 866eb8a7..bfcb0020 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -111,6 +111,32 @@ func (user *User) Markdown() string { } } +func (m *mscalendar) ShouldNotifyStatusChange(u *User) bool { + err := m.ExpandRemoteUser(u) + if err != nil { + return false + } + // TODO: finish when related PRs are merged + return true +} + +func (m *mscalendar) ShouldChangeStatus(u *User) bool { + err := m.ExpandRemoteUser(u) + if err != nil { + return false + } + // TODO: finish when related PRs are merged + return true +} + +func (m *mscalendar) HasAvailabilityChanged(u *User, currentAvailability byte) bool { + err := m.ExpandRemoteUser(u) + if err != nil { + return false + } + return u.LastStatusUpdateAvailability != currentAvailability +} + func (m *mscalendar) DisconnectUser(mattermostUserID string) error { err := m.Filter( withClient, diff --git a/server/store/user_store.go b/server/store/user_store.go index e23eefad..ce403847 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -28,11 +28,12 @@ type UserShort struct { } type User struct { - PluginVersion string - Remote *remote.User - MattermostUserID string - OAuth2Token *oauth2.Token - Settings Settings `json:"mattermostSettings,omitempty"` + PluginVersion string + Remote *remote.User + MattermostUserID string + OAuth2Token *oauth2.Token + Settings Settings `json:"mattermostSettings,omitempty"` + LastStatusUpdateAvailability byte } type Settings struct { From 308905657fe93600a334f322729b9ed9af282120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 6 Apr 2020 13:43:07 +0200 Subject: [PATCH 02/20] Improve error handling and messages --- server/api/post_action.go | 6 +++--- server/mscalendar/availability.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/api/post_action.go b/server/api/post_action.go index afeb564a..c3c5920c 100644 --- a/server/api/post_action.go +++ b/server/api/post_action.go @@ -99,14 +99,14 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R return } - value, ok := request.Context["value"] + value, ok := request.Context["value"].(bool) if !ok { - http.Error(w, "No value", http.StatusBadRequest) + http.Error(w, `No recognizable value for property "value"`, http.StatusBadRequest) return } returnText := "The status has not been changed." - if value.(bool) { + if value { changeTo, ok := request.Context["change_to"] if !ok { http.Error(w, "No state to change", http.StatusBadRequest) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 8d197159..09e7517a 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -173,7 +173,7 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s if m.ShouldNotifyStatusChange(u) { userQuestion := fmt.Sprintf("Shall we change your status from %s to dnd?", currentStatus) m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "dnd", url)) - return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) + return fmt.Sprintf("User asked for status change from %s to dnd.", currentStatus) } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "dnd") return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) @@ -185,7 +185,7 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s if m.ShouldNotifyStatusChange(u) { userQuestion := fmt.Sprintf("Shall we change your status from %s to offline?", currentStatus) m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "offline", url)) - return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) + return fmt.Sprintf("User asked for status change fom %s to offline", currentStatus) } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "offline") return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) From 6e12f49634554ea9657bc3dcba878ae71812ae3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 7 Apr 2020 19:29:23 +0200 Subject: [PATCH 03/20] Improve detection of meetings by checking the start time --- server/mscalendar/availability.go | 21 +++++++++++---------- server/mscalendar/availability_test.go | 11 ++++++++++- server/mscalendar/user.go | 9 +++++++++ server/remote/schedule.go | 20 +++++++++++++++++++- server/store/user_store.go | 1 + 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 09e7517a..a23bcdfe 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -117,7 +117,7 @@ func (m *mscalendar) setUserStatuses(filteredUsers store.UserIndex, schedules [] continue } - res = m.setStatusFromAvailability(mattermostUserID, status, s.AvailabilityView) + res = m.setStatusFromAvailability(mattermostUserID, status, s) } if res != "" { return res, nil @@ -138,8 +138,8 @@ func (m *mscalendar) GetAvailabilities(remoteUserID string, scheduleIDs []string return client.GetSchedule(remoteUserID, scheduleIDs, start, end, availabilityTimeWindowSize) } -func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus string, av remote.AvailabilityView) string { - currentAvailability := av[0] +func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus string, s *remote.ScheduleInformation) string { + currentAvailability := s.AvailabilityView[0] u := NewUser(mattermostUserID) if !m.ShouldChangeStatus(u) { @@ -147,7 +147,11 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } if !m.HasAvailabilityChanged(u, currentAvailability) { - return "Status not changed because there is no update since last status change." + ok, newEventTime := m.HasNewEventStarted(u, s) + if !ok { + return "Status not changed because there is no update since last status change." + } + u.LastStatusUpdateEventTime = *newEventTime } u.LastStatusUpdateAvailability = currentAvailability @@ -165,9 +169,8 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "online") return fmt.Sprintf("User is free. Setting user from %s to online.", currentStatus) - } else { - return fmt.Sprintf("User is free, and is already set to %s.", currentStatus) } + return fmt.Sprintf("User is free, and is already set to %s.", currentStatus) case remote.AvailabilityViewTentative, remote.AvailabilityViewBusy: if currentStatus != "dnd" { if m.ShouldNotifyStatusChange(u) { @@ -177,9 +180,8 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "dnd") return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) - } else { - return fmt.Sprintf("User is busy, and is already set to %s.", currentStatus) } + return fmt.Sprintf("User is busy, and is already set to %s.", currentStatus) case remote.AvailabilityViewOutOfOffice: if currentStatus != "offline" { if m.ShouldNotifyStatusChange(u) { @@ -189,9 +191,8 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "offline") return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) - } else { - return fmt.Sprintf("User is out of office, and is already set to %s.", currentStatus) } + return fmt.Sprintf("User is out of office, and is already set to %s.", currentStatus) case remote.AvailabilityViewWorkingElsewhere: return fmt.Sprintf("User is working elsewhere. Pending implementation.") } diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index c23928f4..663bfa83 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -108,6 +108,15 @@ func TestSyncStatusAll(t *testing.T) { Mail: "bot_email@example.com", }, }, nil).Times(1) + s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ + MattermostUserID: "user_mm_id", + OAuth2Token: token, + Remote: &remote.User{ + ID: "user_remote_id", + Mail: "user_email@example.com", + }, + }, nil).Times(1) + s.EXPECT().StoreUser(gomock.Any()).Return(nil).Times(1) mockPluginAPI.EXPECT().GetMattermostUser("bot_mm_id").Return(&model.User{}, nil).Times(1) @@ -122,7 +131,7 @@ func TestSyncStatusAll(t *testing.T) { if tc.newStatus == "" { mockPluginAPI.EXPECT().UpdateMattermostUserStatus("user_mm_id", gomock.Any()).Times(0) } else { - mockPluginAPI.EXPECT().UpdateMattermostUserStatus("user_mm_id", tc.newStatus).Times(1) + poster.EXPECT().DMWithAttachments("user_mm_id", gomock.Any()).Return(nil) } mscalendar := New(env, "") diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index bfcb0020..a73544ea 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -137,6 +137,15 @@ func (m *mscalendar) HasAvailabilityChanged(u *User, currentAvailability byte) b return u.LastStatusUpdateAvailability != currentAvailability } +func (m *mscalendar) HasNewEventStarted(u *User, schedule *remote.ScheduleInformation) (bool, *remote.DateTime) { + for _, e := range schedule.ScheduleItems { + if e.Start.Time().After(u.LastStatusUpdateEventTime.Time()) { + return true, &e.Start + } + } + return false, nil +} + func (m *mscalendar) DisconnectUser(mattermostUserID string) error { err := m.Filter( withClient, diff --git a/server/remote/schedule.go b/server/remote/schedule.go index 7ca65d8e..c06c3f6b 100644 --- a/server/remote/schedule.go +++ b/server/remote/schedule.go @@ -11,6 +11,15 @@ const ( AvailabilityViewWorkingElsewhere = '4' ) +const ( + ScheduleStatusFree = "free" + ScheduleStatusTentative = "tentative" + ScheduleStatusBusy = "busy" + ScheduleStatusOof = "oof" + ScheduleStatusWorkingElsewhere = "workingElsewhere" + ScheduleStatusUnknown = "unknown" +) + type ScheduleInformationError struct { Message string `json:"message"` ResponseCode string `json:"responseCode"` @@ -29,7 +38,16 @@ type ScheduleInformation struct { Error *ScheduleInformationError `json:"error"` - // ScheduleItems []interface{} `json:"scheduleItems,omitempty"` + ScheduleItems []*ScheduleItem `json:"scheduleItems,omitempty"` // WorkingHours interface{} `json:"workingHours,omitempty"` // Error *FreeBusyError `json:"error,omitempty"` } + +type ScheduleItem struct { + IsPrivate bool + Status string + Subject string + Location string + Start DateTime + End DateTime +} diff --git a/server/store/user_store.go b/server/store/user_store.go index ce403847..3c6c36b4 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -34,6 +34,7 @@ type User struct { OAuth2Token *oauth2.Token Settings Settings `json:"mattermostSettings,omitempty"` LastStatusUpdateAvailability byte + LastStatusUpdateEventTime remote.DateTime } type Settings struct { From 4fbe8005805fbea69bd131e113739cf62d68e196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 8 Apr 2020 12:35:47 +0200 Subject: [PATCH 04/20] Add event information to status change notification --- server/mscalendar/availability.go | 42 ++----------- .../views/status_change_notification.go | 63 +++++++++++++++++++ 2 files changed, 67 insertions(+), 38 deletions(-) create mode 100644 server/mscalendar/views/status_change_notification.go diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index a23bcdfe..0a35815e 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -8,10 +8,10 @@ import ( "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/store" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils" - "github.com/mattermost/mattermost-server/v5/model" ) const ( @@ -163,8 +163,7 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s case remote.AvailabilityViewFree: if currentStatus == "dnd" { if m.ShouldNotifyStatusChange(u) { - userQuestion := fmt.Sprintf("Shall we change your status from %s to online?", currentStatus) - m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "online", url)) + m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Online", url)) return fmt.Sprintf("User asked for status change from %s to online", currentStatus) } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "online") @@ -174,8 +173,7 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s case remote.AvailabilityViewTentative, remote.AvailabilityViewBusy: if currentStatus != "dnd" { if m.ShouldNotifyStatusChange(u) { - userQuestion := fmt.Sprintf("Shall we change your status from %s to dnd?", currentStatus) - m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "dnd", url)) + m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Do Not Disturb", url)) return fmt.Sprintf("User asked for status change from %s to dnd.", currentStatus) } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "dnd") @@ -185,8 +183,7 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s case remote.AvailabilityViewOutOfOffice: if currentStatus != "offline" { if m.ShouldNotifyStatusChange(u) { - userQuestion := fmt.Sprintf("Shall we change your status from %s to offline?", currentStatus) - m.Poster.DMWithAttachments(mattermostUserID, statusChangeAttachments(userQuestion, "offline", url)) + m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Offline", url)) return fmt.Sprintf("User asked for status change fom %s to offline", currentStatus) } m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "offline") @@ -199,34 +196,3 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s return fmt.Sprintf("Availability view doesn't match %d", currentAvailability) } - -func statusChangeAttachments(message, to, url string) *model.SlackAttachment { - actionYes := &model.PostAction{ - Name: "Yes", - Integration: &model.PostActionIntegration{ - URL: url, - Context: map[string]interface{}{ - "value": true, - "change_to": to, - }, - }, - } - - actionNo := &model.PostAction{ - Name: "No", - Integration: &model.PostActionIntegration{ - URL: url, - Context: map[string]interface{}{ - "value": false, - }, - }, - } - - sa := &model.SlackAttachment{ - Title: "Status change", - Text: message, - Actions: []*model.PostAction{actionYes, actionNo}, - } - - return sa -} diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go new file mode 100644 index 00000000..f1507737 --- /dev/null +++ b/server/mscalendar/views/status_change_notification.go @@ -0,0 +1,63 @@ +package views + +import ( + "fmt" + "time" + + "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" + "github.com/mattermost/mattermost-server/v5/model" +) + +func RenderStatusChangeNotificationView(events []*remote.ScheduleItem, status, url string) *model.SlackAttachment { + for _, e := range events { + if e.Start.Time().After(time.Now()) { + return statusChangeAttachments(e, status, url) + } + } + return statusChangeAttachments(nil, status, url) +} + +func renderScheduleItem(si *remote.ScheduleItem, status string) string { + if si == nil { + return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", status) + } + + resp := fmt.Sprintf("Your event with subject `%s` will start soon.", si.Subject) + if si.Subject == "" { + resp = "An event with no subject will start soon." + } + + resp += "\nShall I change your status to %s?" + return resp +} + +func statusChangeAttachments(event *remote.ScheduleItem, status, url string) *model.SlackAttachment { + actionYes := &model.PostAction{ + Name: "Yes", + Integration: &model.PostActionIntegration{ + URL: url, + Context: map[string]interface{}{ + "value": true, + "change_to": status, + }, + }, + } + + actionNo := &model.PostAction{ + Name: "No", + Integration: &model.PostActionIntegration{ + URL: url, + Context: map[string]interface{}{ + "value": false, + }, + }, + } + + sa := &model.SlackAttachment{ + Title: "Status change", + Text: renderScheduleItem(event, status), + Actions: []*model.PostAction{actionYes, actionNo}, + } + + return sa +} From 18d40cc59eedf229b3c920c83b0191cbbbced880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 13 Apr 2020 11:22:01 +0200 Subject: [PATCH 05/20] Change start and end times types to pointers and be more clear with hasStarted --- server/mscalendar/availability.go | 6 +++--- server/mscalendar/user.go | 2 +- server/remote/schedule.go | 4 ++-- server/store/user_store.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 0a35815e..3a4a5f89 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -147,11 +147,11 @@ func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus s } if !m.HasAvailabilityChanged(u, currentAvailability) { - ok, newEventTime := m.HasNewEventStarted(u, s) - if !ok { + hasStarted, newEventTime := m.HasNewEventStarted(u, s) + if !hasStarted { return "Status not changed because there is no update since last status change." } - u.LastStatusUpdateEventTime = *newEventTime + u.LastStatusUpdateEventTime = newEventTime } u.LastStatusUpdateAvailability = currentAvailability diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index a73544ea..0368b510 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -140,7 +140,7 @@ func (m *mscalendar) HasAvailabilityChanged(u *User, currentAvailability byte) b func (m *mscalendar) HasNewEventStarted(u *User, schedule *remote.ScheduleInformation) (bool, *remote.DateTime) { for _, e := range schedule.ScheduleItems { if e.Start.Time().After(u.LastStatusUpdateEventTime.Time()) { - return true, &e.Start + return true, e.Start } } return false, nil diff --git a/server/remote/schedule.go b/server/remote/schedule.go index c06c3f6b..7bfeade4 100644 --- a/server/remote/schedule.go +++ b/server/remote/schedule.go @@ -48,6 +48,6 @@ type ScheduleItem struct { Status string Subject string Location string - Start DateTime - End DateTime + Start *DateTime + End *DateTime } diff --git a/server/store/user_store.go b/server/store/user_store.go index 3c6c36b4..348320f3 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -34,7 +34,7 @@ type User struct { OAuth2Token *oauth2.Token Settings Settings `json:"mattermostSettings,omitempty"` LastStatusUpdateAvailability byte - LastStatusUpdateEventTime remote.DateTime + LastStatusUpdateEventTime *remote.DateTime } type Settings struct { From b2f4f5c78f66b39982ac96763a771e1b5d4b8bce Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Wed, 15 Apr 2020 03:08:30 -0400 Subject: [PATCH 06/20] Use GetCalendarView to determine user's current status --- server/mscalendar/availability.go | 221 ++++++++++++------ server/mscalendar/daily_summary.go | 8 +- server/mscalendar/daily_summary_test.go | 4 +- .../mock_mscalendar/mock_mscalendar.go | 10 +- .../views/status_change_notification.go | 23 +- server/remote/calendar.go | 12 +- .../msgraph/get_default_calendar_view.go | 10 +- server/store/mock_store/mock_store.go | 14 ++ server/store/user_store.go | 13 ++ .../testdata/get_calendar_view_response.json | 67 ++++++ 10 files changed, 275 insertions(+), 107 deletions(-) create mode 100644 server/testdata/get_calendar_view_response.json diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 9b62c650..7121628d 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -12,15 +12,16 @@ 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-server/v5/model" "github.com/pkg/errors" ) const ( - availabilityTimeWindowSize = 15 + availabilityTimeWindowSize = 10 * time.Minute ) type Availability interface { - GetAvailabilities(remoteUserID string, scheduleIDs []string) ([]*remote.ScheduleInformation, error) + GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) SyncStatus(mattermostUserID string) (string, error) SyncStatusAll() (string, error) } @@ -58,25 +59,27 @@ func (m *mscalendar) SyncStatusAll() (string, error) { return m.syncStatusUsers(userIndex) } -func (m *mscalendar) syncStatusUsers(users store.UserIndex) (string, error) { - err := m.Filter( - withClient, - withUserExpanded(m.actingUser), - ) - if err != nil { - return "", err - } - - if len(users) == 0 { +func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) { + if len(userIndex) == 0 { return "No connected users found", nil } - scheduleIDs := []string{} - for _, u := range users { - scheduleIDs = append(scheduleIDs, u.Email) + 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 { + return "", err + } + if user.Settings.UpdateStatus { + users = append(users, user) + } + } + if len(users) == 0 { + return "No users want to have their status updated", nil } - schedules, err := m.GetAvailabilities(m.actingUser.Remote.ID, scheduleIDs) + schedules, err := m.GetAvailabilities(users) if err != nil { return "", err } @@ -87,8 +90,12 @@ func (m *mscalendar) syncStatusUsers(users store.UserIndex) (string, error) { return m.setUserStatuses(users, schedules) } -func (m *mscalendar) setUserStatuses(users store.UserIndex, schedules []*remote.ScheduleInformation) (string, error) { - mattermostUserIDs := users.GetMattermostUserIDs() +func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { + mattermostUserIDs := []string{} + for _, u := range users { + mattermostUserIDs = append(mattermostUserIDs, u.MattermostUserID) + } + statuses, appErr := m.PluginAPI.GetMattermostUserStatusesByIds(mattermostUserIDs) if appErr != nil { return "", appErr @@ -98,21 +105,30 @@ func (m *mscalendar) setUserStatuses(users store.UserIndex, schedules []*remote. statusMap[s.UserId] = s.Status } - usersByEmail := users.ByEmail() + usersByRemoteID := map[string]*store.User{} + for _, u := range users { + usersByRemoteID[u.Remote.ID] = u + } + var res string for _, s := range schedules { + user := usersByRemoteID[s.RemoteUserID] if s.Error != nil { - m.Logger.Errorf("Error getting availability for %s: %s", s.ScheduleID, s.Error.ResponseCode) + m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } - mattermostUserID := usersByEmail[s.ScheduleID].MattermostUserID + mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID status, ok := statusMap[mattermostUserID] if !ok { continue } - res = m.setStatusFromAvailability(mattermostUserID, status, s) + var err error + res, err = m.setStatusFromAvailability(user, status, s) + if err != nil { + m.Logger.Errorf("Error setting user %s status. %s", user.Remote.Mail, err.Error()) + } } if res != "" { return res, nil @@ -121,73 +137,124 @@ func (m *mscalendar) setUserStatuses(users store.UserIndex, schedules []*remote. return utils.JSONBlock(schedules), nil } -func (m *mscalendar) GetAvailabilities(remoteUserID string, scheduleIDs []string) ([]*remote.ScheduleInformation, error) { - err := m.Filter(withClient) - if err != nil { - return nil, err - } - - start := remote.NewDateTime(time.Now().UTC(), "UTC") - end := remote.NewDateTime(time.Now().UTC().Add(availabilityTimeWindowSize*time.Minute), "UTC") +func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, res *remote.ViewCalendarResponse) (string, error) { + events := filterBusyEvents(res.Events) - return m.client.GetSchedule(remoteUserID, scheduleIDs, start, end, availabilityTimeWindowSize) -} - -func (m *mscalendar) setStatusFromAvailability(mattermostUserID, currentStatus string, s *remote.ScheduleInformation) string { - currentAvailability := s.AvailabilityView[0] - u := NewUser(mattermostUserID) - - if !m.ShouldChangeStatus(u) { - return "Status not changed by user configuration." + if len(user.ActiveEvents) == 0 && len(events) == 0 { + return "No events in local or remote. No status change.", nil } - if !m.HasAvailabilityChanged(u, currentAvailability) { - hasStarted, newEventTime := m.HasNewEventStarted(u, s) - if !hasStarted { - return "Status not changed because there is no update since last status change." + var message string + if len(user.ActiveEvents) > 0 && len(events) == 0 { + if currentStatus == model.STATUS_DND { + err := m.setStatusOrAskUser(user, model.STATUS_ONLINE, events) + if err != nil { + return "", err + } + message = "User is no longer busy in calendar. Set status to online." + } else { + message = "User is no longer busy in calendar, but is not set to DND. No status change." + } + err := m.Store.StoreUserActiveEvents(user.MattermostUserID, []string{}) + if err != nil { + return "", err } - u.LastStatusUpdateEventTime = newEventTime + return message, nil } - u.LastStatusUpdateAvailability = currentAvailability - m.Store.StoreUser(u.User) + remoteHashes := []string{} + for _, e := range events { + h := fmt.Sprintf("%s %s", e.ID, e.Start.Time().UTC().Format(time.RFC3339)) + remoteHashes = append(remoteHashes, h) + } - url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) + if len(user.ActiveEvents) == 0 { + err := m.setStatusOrAskUser(user, model.STATUS_DND, events) + if err != nil { + return "", err + } + err = m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) + if err != nil { + return "", err + } + return "User was free, but is now busy. Set status to DND.", nil + } - switch currentAvailability { - case remote.AvailabilityViewFree: - if currentStatus == "dnd" { - if m.ShouldNotifyStatusChange(u) { - m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Online", url)) - return fmt.Sprintf("User asked for status change from %s to online", currentStatus) + newEventExists := false + for _, r := range remoteHashes { + found := false + for _, loc := range user.ActiveEvents { + if loc == r { + found = true + break } - m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "online") - return fmt.Sprintf("User is free. Setting user from %s to online.", currentStatus) } - return fmt.Sprintf("User is free, and is already set to %s.", currentStatus) - case remote.AvailabilityViewTentative, remote.AvailabilityViewBusy: - if currentStatus != "dnd" { - if m.ShouldNotifyStatusChange(u) { - m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Do Not Disturb", url)) - return fmt.Sprintf("User asked for status change from %s to dnd.", currentStatus) - } - m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "dnd") - return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus) + if !found { + newEventExists = true + break } - return fmt.Sprintf("User is busy, and is already set to %s.", currentStatus) - case remote.AvailabilityViewOutOfOffice: - if currentStatus != "offline" { - if m.ShouldNotifyStatusChange(u) { - m.Poster.DMWithAttachments(mattermostUserID, views.RenderStatusChangeNotificationView(s.ScheduleItems, "Offline", url)) - return fmt.Sprintf("User asked for status change fom %s to offline", currentStatus) - } - m.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, "offline") - return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus) + } + + if !newEventExists { + return fmt.Sprintf("No change in active events. Total number of events: %d", len(events)), nil + } + + if currentStatus != model.STATUS_DND { + err := m.setStatusOrAskUser(user, model.STATUS_DND, events) + if err != nil { + return "", err } - return fmt.Sprintf("User is out of office, and is already set to %s.", currentStatus) - case remote.AvailabilityViewWorkingElsewhere: - return fmt.Sprintf("User is working elsewhere. Pending implementation.") + message = "User was free, but is now busy. Set status to DND." + } else { + message = "User is already busy. No status change." + } + err := m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) + if err != nil { + return "", err + } + return message, nil +} + +func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events []*remote.Event) error { + if user.Settings.GetConfirmation { + url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) + return m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(events, status, url)) + } + + _, appErr := m.PluginAPI.UpdateMattermostUserStatus(user.MattermostUserID, model.STATUS_DND) + if appErr != nil { + return appErr + } + return nil +} + +func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) { + err := m.Filter(withClient) + if err != nil { + return nil, err } - return fmt.Sprintf("Availability view doesn't match %d", currentAvailability) + start := time.Now().UTC() + end := time.Now().UTC().Add(availabilityTimeWindowSize) + + params := []*remote.ViewCalendarParams{} + for _, u := range users { + params = append(params, &remote.ViewCalendarParams{ + RemoteUserID: u.Remote.ID, + StartTime: start, + EndTime: end, + }) + } + + return m.client.DoBatchViewCalendarRequests(params) +} + +func filterBusyEvents(events []*remote.Event) []*remote.Event { + result := []*remote.Event{} + for _, e := range events { + if e.ShowAs == "busy" { + result = append(result, e) + } + } + return result } diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index b6f10a39..eec5f9a0 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -136,9 +136,9 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { start, end := getTodayHoursForTimezone(now, dsum.Timezone) req := &remote.ViewCalendarParams{ - RemoteID: dsum.RemoteID, - StartTime: start, - EndTime: end, + RemoteUserID: dsum.RemoteID, + StartTime: start, + EndTime: end, } requests = append(requests, req) } @@ -151,7 +151,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { mappedPostTimes := map[string]string{} byRemoteID := dsumIndex.ByRemoteID() for _, res := range responses { - dsum := byRemoteID[res.RemoteID] + dsum := byRemoteID[res.RemoteUserID] if res.Error != nil { m.Logger.Errorf("Error rendering user %s calendar: %s %s", dsum.MattermostUserID, res.Error.Code, res.Error.Message) } diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 83897a76..43ea336a 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -152,8 +152,8 @@ func TestProcessAllDailySummary(t *testing.T) { hour, minute := 10, 0 // Time is "10:00AM" moment := makeTime(hour, minute, loc) mockClient.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Return([]*remote.ViewCalendarResponse{ - {RemoteID: "user1_remote_id", Events: []*remote.Event{}}, - {RemoteID: "user2_remote_id", Events: []*remote.Event{ + {RemoteUserID: "user1_remote_id", Events: []*remote.Event{}}, + {RemoteUserID: "user2_remote_id", Events: []*remote.Event{ { Subject: "The subject", Start: remote.NewDateTime(moment, "Mountain Standard Time"), diff --git a/server/mscalendar/mock_mscalendar/mock_mscalendar.go b/server/mscalendar/mock_mscalendar/mock_mscalendar.go index 7ce6d423..b19b58b0 100644 --- a/server/mscalendar/mock_mscalendar/mock_mscalendar.go +++ b/server/mscalendar/mock_mscalendar/mock_mscalendar.go @@ -195,18 +195,18 @@ func (mr *MockMSCalendarMockRecorder) GetActingUser() *gomock.Call { } // GetAvailabilities mocks base method -func (m *MockMSCalendar) GetAvailabilities(arg0 string, arg1 []string) ([]*remote.ScheduleInformation, error) { +func (m *MockMSCalendar) GetAvailabilities(arg0 []*store.User) ([]*remote.ViewCalendarResponse, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAvailabilities", arg0, arg1) - ret0, _ := ret[0].([]*remote.ScheduleInformation) + ret := m.ctrl.Call(m, "GetAvailabilities", arg0) + ret0, _ := ret[0].([]*remote.ViewCalendarResponse) ret1, _ := ret[1].(error) return ret0, ret1 } // GetAvailabilities indicates an expected call of GetAvailabilities -func (mr *MockMSCalendarMockRecorder) GetAvailabilities(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockMSCalendarMockRecorder) GetAvailabilities(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAvailabilities", reflect.TypeOf((*MockMSCalendar)(nil).GetAvailabilities), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAvailabilities", reflect.TypeOf((*MockMSCalendar)(nil).GetAvailabilities), arg0) } // GetCalendars mocks base method diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index f1507737..a89836c5 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -8,7 +8,14 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) -func RenderStatusChangeNotificationView(events []*remote.ScheduleItem, status, url string) *model.SlackAttachment { +var prettyStatuses = map[string]string{ + model.STATUS_ONLINE: "Online", + model.STATUS_AWAY: "Away", + model.STATUS_DND: "DND", + model.STATUS_OFFLINE: "Offline", +} + +func RenderStatusChangeNotificationView(events []*remote.Event, status, url string) *model.SlackAttachment { for _, e := range events { if e.Start.Time().After(time.Now()) { return statusChangeAttachments(e, status, url) @@ -17,21 +24,21 @@ func RenderStatusChangeNotificationView(events []*remote.ScheduleItem, status, u return statusChangeAttachments(nil, status, url) } -func renderScheduleItem(si *remote.ScheduleItem, status string) string { - if si == nil { - return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", status) +func renderScheduleItem(event *remote.Event, status string) string { + if event == nil { + return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", prettyStatuses[status]) } - resp := fmt.Sprintf("Your event with subject `%s` will start soon.", si.Subject) - if si.Subject == "" { + resp := fmt.Sprintf("Your event with subject `%s` will start soon.", event.Subject) + if event.Subject == "" { resp = "An event with no subject will start soon." } - resp += "\nShall I change your status to %s?" + resp += fmt.Sprintf("\nShall I change your status to %s?", prettyStatuses[status]) return resp } -func statusChangeAttachments(event *remote.ScheduleItem, status, url string) *model.SlackAttachment { +func statusChangeAttachments(event *remote.Event, status, url string) *model.SlackAttachment { actionYes := &model.PostAction{ Name: "Yes", Integration: &model.PostActionIntegration{ diff --git a/server/remote/calendar.go b/server/remote/calendar.go index 91e7d4d5..ff37981a 100644 --- a/server/remote/calendar.go +++ b/server/remote/calendar.go @@ -16,13 +16,13 @@ type Calendar struct { } type ViewCalendarParams struct { - RemoteID string - StartTime time.Time - EndTime time.Time + RemoteUserID string + StartTime time.Time + EndTime time.Time } type ViewCalendarResponse struct { - RemoteID string - Events []*Event - Error *ApiError + RemoteUserID string + Events []*Event + Error *ApiError } diff --git a/server/remote/msgraph/get_default_calendar_view.go b/server/remote/msgraph/get_default_calendar_view.go index 2c362021..0f8bc06c 100644 --- a/server/remote/msgraph/get_default_calendar_view.go +++ b/server/remote/msgraph/get_default_calendar_view.go @@ -45,7 +45,7 @@ func (c *client) DoBatchViewCalendarRequests(allParams []*remote.ViewCalendarPar for _, params := range allParams { u := getCalendarViewURL(params) req := &singleRequest{ - ID: params.RemoteID, + ID: params.RemoteUserID, URL: u, Method: http.MethodGet, Headers: map[string]string{}, @@ -69,9 +69,9 @@ func (c *client) DoBatchViewCalendarRequests(allParams []*remote.ViewCalendarPar for _, batchRes := range batchResponses { for _, res := range batchRes.Responses { viewCalRes := &remote.ViewCalendarResponse{ - RemoteID: res.ID, - Events: res.Body.Value, - Error: res.Body.Error, + RemoteUserID: res.ID, + Events: res.Body.Value, + Error: res.Body.Error, } result = append(result, viewCalRes) } @@ -82,7 +82,7 @@ func (c *client) DoBatchViewCalendarRequests(allParams []*remote.ViewCalendarPar func getCalendarViewURL(params *remote.ViewCalendarParams) string { paramStr := getQueryParamStringForCalendarView(params.StartTime, params.EndTime) - return "/Users/" + params.RemoteID + "/calendarView" + paramStr + return "/Users/" + params.RemoteUserID + "/calendarView" + paramStr } func getQueryParamStringForCalendarView(start, end time.Time) string { diff --git a/server/store/mock_store/mock_store.go b/server/store/mock_store/mock_store.go index 66b167ee..8c76a8bf 100644 --- a/server/store/mock_store/mock_store.go +++ b/server/store/mock_store/mock_store.go @@ -293,6 +293,20 @@ func (mr *MockStoreMockRecorder) StoreUser(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreUser", reflect.TypeOf((*MockStore)(nil).StoreUser), arg0) } +// StoreUserActiveEvents mocks base method +func (m *MockStore) StoreUserActiveEvents(arg0 string, arg1 []string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "StoreUserActiveEvents", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// StoreUserActiveEvents indicates an expected call of StoreUserActiveEvents +func (mr *MockStoreMockRecorder) StoreUserActiveEvents(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreUserActiveEvents", reflect.TypeOf((*MockStore)(nil).StoreUserActiveEvents), arg0, arg1) +} + // StoreUserEvent mocks base method func (m *MockStore) StoreUserEvent(arg0 string, arg1 *store.Event) error { m.ctrl.T.Helper() diff --git a/server/store/user_store.go b/server/store/user_store.go index 505fb81e..291f25f2 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -22,6 +22,7 @@ type UserStore interface { ModifyUserIndex(modify func(userIndex UserIndex) (UserIndex, error)) error StoreUserInIndex(user *User) error DeleteUserFromIndex(mattermostUserID string) error + StoreUserActiveEvents(mattermostUserID string, events []string) error } type UserIndex []*UserShort @@ -40,10 +41,13 @@ type User struct { Settings Settings `json:"mattermostSettings,omitempty"` LastStatusUpdateAvailability byte LastStatusUpdateEventTime *remote.DateTime + ActiveEvents []string `json:"events"` } type Settings struct { EventSubscriptionID string + UpdateStatus bool + GetConfirmation bool } func (settings Settings) String() string { @@ -206,6 +210,15 @@ func (s *pluginStore) DeleteUserFromIndex(mattermostUserID string) error { }) } +func (s *pluginStore) StoreUserActiveEvents(mattermostUserID string, events []string) error { + u, err := s.LoadUser(mattermostUserID) + if err != nil { + return err + } + u.ActiveEvents = events + return kvstore.StoreJSON(s.userKV, mattermostUserID, u) +} + func (index UserIndex) ByMattermostID() map[string]*UserShort { result := map[string]*UserShort{} diff --git a/server/testdata/get_calendar_view_response.json b/server/testdata/get_calendar_view_response.json new file mode 100644 index 00000000..94f11d5b --- /dev/null +++ b/server/testdata/get_calendar_view_response.json @@ -0,0 +1,67 @@ +{ + "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#users('837ba3dd-5f18-4f6d-a1c8-609408de0f72')/calendarView", + "value": [ + { + "@odata.etag": "W/\"Rsx2xZXkVkex2DxZILyPUQAAGuVK+Q==\"", + "id": "AAMkADRjMDgzMWFjLWIxYjgtNGJlMC1iY2U2LWVlMzg3ZTc4ZDQwNgBGAAAAAADeW-I_YodJRJbshVQzIa1zBwBGzHbFleRWR7HYPFkgvI9RAAAAAAENAABGzHbFleRWR7HYPFkgvI9RAAAa6ocNAAA=", + "createdDateTime": "2020-04-14T20:33:33.6273939Z", + "lastModifiedDateTime": "2020-04-14T20:33:34.2891014Z", + "changeKey": "Rsx2xZXkVkex2DxZILyPUQAAGuVK+Q==", + "categories": [], + "originalStartTimeZone": "Pacific Standard Time", + "originalEndTimeZone": "Pacific Standard Time", + "iCalUId": "040000008200E00074C5B7101A82E0080000000021968AF79B12D601000000000000000010000000C0783E2C35DD7145B7943E4F751C3F29", + "reminderMinutesBeforeStart": 15, + "isReminderOn": true, + "hasAttachments": false, + "subject": "The Greatest Event of All Time", + "bodyPreview": "", + "importance": "normal", + "sensitivity": "normal", + "isAllDay": false, + "isCancelled": false, + "isOrganizer": true, + "responseRequested": true, + "seriesMasterId": null, + "showAs": "busy", + "type": "singleInstance", + "webLink": "https://outlook.office365.com/owa/?itemid=AAMkADRjMDgzMWFjLWIxYjgtNGJlMC1iY2U2LWVlMzg3ZTc4ZDQwNgBGAAAAAADeW%2FI%2BYodJRJbshVQzIa1zBwBGzHbFleRWR7HYPFkgvI9RAAAAAAENAABGzHbFleRWR7HYPFkgvI9RAAAa6ocNAAA%3D&exvsurl=1&path=/calendar/item", + "onlineMeetingUrl": null, + "isOnlineMeeting": false, + "onlineMeetingProvider": "unknown", + "recurrence": null, + "onlineMeeting": null, + "responseStatus": { + "response": "organizer", + "time": "0001-01-01T00:00:00Z" + }, + "body": { + "contentType": "html", + "content": "" + }, + "start": { + "dateTime": "2020-04-14T20:30:00.0000000", + "timeZone": "UTC" + }, + "end": { + "dateTime": "2020-04-15T01:00:00.0000000", + "timeZone": "UTC" + }, + "location": { + "displayName": "", + "locationType": "default", + "uniqueIdType": "unknown", + "address": {}, + "coordinates": {} + }, + "locations": [], + "attendees": [], + "organizer": { + "emailAddress": { + "name": "First Last", + "address": "useremail@test.onmicrosoft.com" + } + } + } + ] +} From 3b40dc10d7a24962a9324dd0a5308dede10b4047 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Fri, 17 Apr 2020 17:52:39 -0400 Subject: [PATCH 07/20] Test status sync job --- server/mscalendar/availability.go | 5 +- server/mscalendar/availability_test.go | 241 ++++++++++++------ server/mscalendar/user.go | 35 --- .../views/status_change_notification.go | 2 +- server/store/mock_store/mock_store.go | 28 +- 5 files changed, 187 insertions(+), 124 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 7908d6b3..f827b74b 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -173,6 +173,9 @@ func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus s } if len(user.ActiveEvents) == 0 { + if currentStatus == model.STATUS_DND { + return "User was already marked as busy. No status change.", nil + } err := m.setStatusOrAskUser(user, model.STATUS_DND, events) if err != nil { return "", err @@ -226,7 +229,7 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events return err } - _, appErr := m.PluginAPI.UpdateMattermostUserStatus(user.MattermostUserID, model.STATUS_DND) + _, appErr := m.PluginAPI.UpdateMattermostUserStatus(user.MattermostUserID, status) if appErr != nil { return appErr } diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 1f6e9da1..cd5bd586 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -6,6 +6,7 @@ package mscalendar import ( "context" "testing" + "time" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -24,116 +25,210 @@ import ( ) func TestSyncStatusAll(t *testing.T) { + moment := time.Now().UTC() + eventHash := "event_id " + moment.Format(time.RFC3339) + busyEvent := &remote.Event{ID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} + for name, tc := range map[string]struct { - sched *remote.ScheduleInformation + remoteEvents []*remote.Event + activeEvents []string currentStatus string newStatus string + eventsToStore []string }{ - "User is free but dnd, mark user as online": { - sched: &remote.ScheduleInformation{ - ScheduleID: "user_email@example.com", - AvailabilityView: "0", - }, - currentStatus: "dnd", - newStatus: "online", + "Most common case, no events local or remote. No status change.": { + remoteEvents: []*remote.Event{}, + activeEvents: []string{}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, }, - "User is busy but online, mark as dnd": { - sched: &remote.ScheduleInformation{ - ScheduleID: "user_email@example.com", - AvailabilityView: "2", - }, + "New remote event. Change status to DND.": { + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{}, currentStatus: "online", newStatus: "dnd", + eventsToStore: []string{eventHash}, }, - "User is free and online, do not change status": { - sched: &remote.ScheduleInformation{ - ScheduleID: "user_email@example.com", - AvailabilityView: "0", - }, + "Locally stored event is finished. Change status to online.": { + remoteEvents: []*remote.Event{}, + activeEvents: []string{eventHash}, + currentStatus: "dnd", + newStatus: "online", + eventsToStore: []string{}, + }, + "Locally stored event is still happening. No status change.": { + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{eventHash}, + currentStatus: "dnd", + newStatus: "", + eventsToStore: nil, + }, + "User has manually set themselves to online during event. Locally stored event is still happening, but we will ignore it. No status change.": { + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{eventHash}, currentStatus: "online", newStatus: "", + eventsToStore: nil, }, - "User is busy and dnd, do not change status": { - sched: &remote.ScheduleInformation{ - ScheduleID: "user_email@example.com", - AvailabilityView: "2", - }, - currentStatus: "dnd", + "Ignore non-busy event": { + remoteEvents: []*remote.Event{{ID: "event_id_2", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "free"}}, + activeEvents: []string{}, + currentStatus: "online", newStatus: "", + eventsToStore: nil, }, } { t.Run(name, func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - s := mock_store.NewMockStore(ctrl) - poster := mock_bot.NewMockPoster(ctrl) - mockRemote := mock_remote.NewMockRemote(ctrl) - mockClient := mock_remote.NewMockClient(ctrl) - mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) - - logger := &bot.TestLogger{TB: t} - conf := &config.Config{BotUserID: "bot_mm_id"} - env := Env{ - Config: conf, - Dependencies: &Dependencies{ - Store: s, - Logger: logger, - Poster: poster, - Remote: mockRemote, - PluginAPI: mockPluginAPI, - IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return true, nil }, - }, - } + env, client := makeStatusSyncTestEnv(ctrl) + deps := env.Dependencies - s.EXPECT().LoadUserIndex().Return(store.UserIndex{ - &store.UserShort{ - MattermostUserID: "user_mm_id", - RemoteID: "user_remote_id", - Email: "user_email@example.com", - }, - }, nil).Times(1) + c, papi, s := client.(*mock_remote.MockClient), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Store.(*mock_store.MockStore) - token := &oauth2.Token{ - AccessToken: "bot_oauth_token", - } - s.EXPECT().LoadUser("bot_mm_id").Return(&store.User{ - MattermostUserID: "bot_mm_id", - OAuth2Token: token, - Remote: &remote.User{ - ID: "bot_remote_id", - Mail: "bot_email@example.com", - }, - }, nil).Times(1) s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ MattermostUserID: "user_mm_id", - OAuth2Token: token, Remote: &remote.User{ ID: "user_remote_id", Mail: "user_email@example.com", }, + Settings: store.Settings{UpdateStatus: true}, + ActiveEvents: tc.activeEvents, }, nil).Times(1) - s.EXPECT().StoreUser(gomock.Any()).Return(nil).Times(1) - - mockPluginAPI.EXPECT().GetMattermostUser("bot_mm_id").Return(&model.User{}, nil).Times(1) - - mockRemote.EXPECT().MakeClient(context.Background(), token).Return(mockClient).Times(1) - mockClient.EXPECT().GetSuperuserToken().Return("bot_bearer_token", nil) - mockRemote.EXPECT().MakeSuperuserClient(context.Background(), "bot_bearer_token").Return(mockClient) - mockClient.EXPECT().GetSchedule("bot_remote_id", []string{"user_email@example.com"}, gomock.Any(), gomock.Any(), 15).Return([]*remote.ScheduleInformation{tc.sched}, nil) + c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Return([]*remote.ViewCalendarResponse{ + {Events: tc.remoteEvents, RemoteUserID: "user_remote_id"}, + }, nil) - mockPluginAPI.EXPECT().GetMattermostUserStatusesByIds([]string{"user_mm_id"}).Return([]*model.Status{&model.Status{Status: tc.currentStatus, UserId: "user_mm_id"}}, nil) + papi.EXPECT().GetMattermostUserStatusesByIds([]string{"user_mm_id"}).Return([]*model.Status{&model.Status{Status: tc.currentStatus, UserId: "user_mm_id"}}, nil) if tc.newStatus == "" { - mockPluginAPI.EXPECT().UpdateMattermostUserStatus("user_mm_id", gomock.Any()).Times(0) + papi.EXPECT().UpdateMattermostUserStatus("user_mm_id", gomock.Any()).Times(0) } else { - poster.EXPECT().DMWithAttachments("user_mm_id", gomock.Any()).Return(nil) + papi.EXPECT().UpdateMattermostUserStatus("user_mm_id", tc.newStatus).Return(nil, nil) } + if tc.eventsToStore == nil { + s.EXPECT().StoreUserActiveEvents("user_mm_id", gomock.Any()).Return(nil).Times(0) + } else { + s.EXPECT().StoreUserActiveEvents("user_mm_id", tc.eventsToStore).Return(nil).Times(1) + } + + mscalendar := New(env, "") + _, err := mscalendar.SyncStatusAll() + require.Nil(t, err) + }) + } +} + +func TestSyncStatusUserConfig(t *testing.T) { + for name, tc := range map[string]struct { + settings store.Settings + runAssertions func(deps *Dependencies, client remote.Client) + }{ + "UpdateStatus disabled": { + settings: store.Settings{ + UpdateStatus: false, + }, + runAssertions: func(deps *Dependencies, client remote.Client) { + c := client.(*mock_remote.MockClient) + c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Times(0) + }, + }, + "GetConfirmation enabled": { + settings: store.Settings{ + UpdateStatus: true, + 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) + moment := time.Now().UTC() + busyEvent := &remote.Event{ID: "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) + papi.EXPECT().GetMattermostUserStatusesByIds([]string{"user_mm_id"}).Return([]*model.Status{&model.Status{Status: "online", UserId: "user_mm_id"}}, nil) + + s.EXPECT().StoreUserActiveEvents("user_mm_id", []string{"event_id " + moment.Format(time.RFC3339)}) + poster.EXPECT().DMWithAttachments("user_mm_id", gomock.Any()).Times(1) + papi.EXPECT().UpdateMattermostUserStatus("user_mm_id", gomock.Any()).Times(0) + }, + }, + } { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + env, client := makeStatusSyncTestEnv(ctrl) + + s := env.Dependencies.Store.(*mock_store.MockStore) + s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ + MattermostUserID: "user_mm_id", + Remote: &remote.User{ + ID: "user_remote_id", + Mail: "user_email@example.com", + }, + Settings: tc.settings, + }, nil).Times(1) + + tc.runAssertions(env.Dependencies, client) + mscalendar := New(env, "") _, err := mscalendar.SyncStatusAll() require.Nil(t, err) }) } } + +func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { + s := mock_store.NewMockStore(ctrl) + poster := mock_bot.NewMockPoster(ctrl) + mockRemote := mock_remote.NewMockRemote(ctrl) + mockClient := mock_remote.NewMockClient(ctrl) + mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) + + logger := &bot.NilLogger{} + conf := &config.Config{BotUserID: "bot_mm_id"} + env := Env{ + Config: conf, + Dependencies: &Dependencies{ + Store: s, + Logger: logger, + Poster: poster, + Remote: mockRemote, + PluginAPI: mockPluginAPI, + IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return true, nil }, + }, + } + + s.EXPECT().LoadUserIndex().Return(store.UserIndex{ + &store.UserShort{ + MattermostUserID: "user_mm_id", + RemoteID: "user_remote_id", + Email: "user_email@example.com", + }, + }, nil).Times(1) + + token := &oauth2.Token{ + AccessToken: "bot_oauth_token", + } + s.EXPECT().LoadUser("bot_mm_id").Return(&store.User{ + MattermostUserID: "bot_mm_id", + OAuth2Token: token, + Remote: &remote.User{ + ID: "bot_remote_id", + Mail: "bot_email@example.com", + }, + }, nil).Times(1) + + mockPluginAPI.EXPECT().GetMattermostUser("bot_mm_id").Return(&model.User{}, nil).Times(1) + + mockRemote.EXPECT().MakeClient(context.Background(), token).Return(mockClient).Times(1) + mockClient.EXPECT().GetSuperuserToken().Return("bot_bearer_token", nil) + mockRemote.EXPECT().MakeSuperuserClient(context.Background(), "bot_bearer_token").Return(mockClient) + + return env, mockClient +} diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index 556f0a73..fe8f3e30 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -114,41 +114,6 @@ func (user *User) Markdown() string { } } -func (m *mscalendar) ShouldNotifyStatusChange(u *User) bool { - err := m.ExpandRemoteUser(u) - if err != nil { - return false - } - // TODO: finish when related PRs are merged - return true -} - -func (m *mscalendar) ShouldChangeStatus(u *User) bool { - err := m.ExpandRemoteUser(u) - if err != nil { - return false - } - // TODO: finish when related PRs are merged - return true -} - -func (m *mscalendar) HasAvailabilityChanged(u *User, currentAvailability byte) bool { - err := m.ExpandRemoteUser(u) - if err != nil { - return false - } - return u.LastStatusUpdateAvailability != currentAvailability -} - -func (m *mscalendar) HasNewEventStarted(u *User, schedule *remote.ScheduleInformation) (bool, *remote.DateTime) { - for _, e := range schedule.ScheduleItems { - if e.Start.Time().After(u.LastStatusUpdateEventTime.Time()) { - return true, e.Start - } - } - return false, nil -} - func (m *mscalendar) DisconnectUser(mattermostUserID string) error { err := m.Filter( withClient, diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index a89836c5..98cea61d 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -11,7 +11,7 @@ import ( var prettyStatuses = map[string]string{ model.STATUS_ONLINE: "Online", model.STATUS_AWAY: "Away", - model.STATUS_DND: "DND", + model.STATUS_DND: "Do Not Disturb", model.STATUS_OFFLINE: "Offline", } diff --git a/server/store/mock_store/mock_store.go b/server/store/mock_store/mock_store.go index b154c247..36011b82 100644 --- a/server/store/mock_store/mock_store.go +++ b/server/store/mock_store/mock_store.go @@ -295,20 +295,6 @@ func (mr *MockStoreMockRecorder) ModifyUserIndex(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModifyUserIndex", reflect.TypeOf((*MockStore)(nil).ModifyUserIndex), arg0) } -// StoreDailySummaryUserSettings mocks base method -func (m *MockStore) StoreDailySummaryUserSettings(arg0 *store.DailySummaryUserSettings) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "StoreDailySummaryUserSettings", arg0) - ret0, _ := ret[0].(error) - return ret0 -} - -// StoreDailySummaryUserSettings indicates an expected call of StoreDailySummaryUserSettings -func (mr *MockStoreMockRecorder) StoreDailySummaryUserSettings(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreDailySummaryUserSettings", reflect.TypeOf((*MockStore)(nil).StoreDailySummaryUserSettings), arg0) -} - // SetPanelPostID mocks base method func (m *MockStore) SetPanelPostID(arg0, arg1 string) error { m.ctrl.T.Helper() @@ -337,6 +323,20 @@ func (mr *MockStoreMockRecorder) SetSetting(arg0, arg1, arg2 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSetting", reflect.TypeOf((*MockStore)(nil).SetSetting), arg0, arg1, arg2) } +// StoreDailySummaryUserSettings mocks base method +func (m *MockStore) StoreDailySummaryUserSettings(arg0 *store.DailySummaryUserSettings) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "StoreDailySummaryUserSettings", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// StoreDailySummaryUserSettings indicates an expected call of StoreDailySummaryUserSettings +func (mr *MockStoreMockRecorder) StoreDailySummaryUserSettings(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreDailySummaryUserSettings", reflect.TypeOf((*MockStore)(nil).StoreDailySummaryUserSettings), arg0) +} + // StoreOAuth2State mocks base method func (m *MockStore) StoreOAuth2State(arg0 string) error { m.ctrl.T.Helper() From f92f794ac98ec25b6c31356ed379ef0978fe708b Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 21 Apr 2020 02:56:08 -0400 Subject: [PATCH 08/20] treat bot as admin --- server/plugin/plugin.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index afc3ede4..842057b1 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -218,6 +218,9 @@ func (p *Plugin) ServeHTTP(pc *plugin.Context, w http.ResponseWriter, req *http. func (p *Plugin) IsAuthorizedAdmin(mattermostUserID string) (bool, error) { env := p.getEnv() + if mattermostUserID == env.BotUserID { + return true, nil + } for _, userID := range strings.Split(env.Config.AdminUserIDs, ",") { if userID == mattermostUserID { From 0607f524c754efea2f5b039a9162f2ca82fa3469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 22 Apr 2020 15:55:05 +0200 Subject: [PATCH 09/20] Use pretty statuses on slack action response --- server/api/post_action.go | 8 +++++++- server/mscalendar/views/status_change_notification.go | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/server/api/post_action.go b/server/api/post_action.go index eaab65e1..0445012e 100644 --- a/server/api/post_action.go +++ b/server/api/post_action.go @@ -165,8 +165,14 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R return } stringChangeTo := changeTo.(string) + prettyChangeTo, ok := request.Context["pretty_change_to"] + if !ok { + prettyChangeTo = changeTo + } + stringPrettyChangeTo := prettyChangeTo.(string) + api.PluginAPI.UpdateMattermostUserStatus(mattermostUserID, stringChangeTo) - returnText = fmt.Sprintf("The status has been changed to %s.", stringChangeTo) + returnText = fmt.Sprintf("The status has been changed to %s.", stringPrettyChangeTo) } sa := &model.SlackAttachment{ diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index 98cea61d..51ba0e91 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -44,8 +44,9 @@ func statusChangeAttachments(event *remote.Event, status, url string) *model.Sla Integration: &model.PostActionIntegration{ URL: url, Context: map[string]interface{}{ - "value": true, - "change_to": status, + "value": true, + "change_to": status, + "pretty_change_to": prettyStatuses[status], }, }, } From a9c482ba10f0648c90e279c52adc29f370bcab9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 22 Apr 2020 15:58:04 +0200 Subject: [PATCH 10/20] Take in mind cancelled events for notifications and status change --- server/mscalendar/availability.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index f827b74b..6735ac79 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -168,6 +168,9 @@ func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus s remoteHashes := []string{} for _, e := range events { + if e.IsCancelled { + continue + } h := fmt.Sprintf("%s %s", e.ID, e.Start.Time().UTC().Format(time.RFC3339)) remoteHashes = append(remoteHashes, h) } @@ -260,6 +263,9 @@ func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalen func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remote.Event) { var timezone string for _, event := range events { + if event.IsCancelled { + continue + } upcomingTime := time.Now().Add(upcomingEventNotificationTime) start := event.Start.Time() diff := start.Sub(upcomingTime) From 18d48bd7acbba2ad0716fd9655a553c974542de9 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 23 Apr 2020 03:29:43 -0400 Subject: [PATCH 11/20] Use GetSchedule for status and GetCalendarView for reminders --- server/mscalendar/availability.go | 231 ++++++++++-------- server/mscalendar/settings.go | 7 + .../views/status_change_notification.go | 20 +- server/store/setting_store.go | 9 + server/store/user_store.go | 12 +- 5 files changed, 151 insertions(+), 128 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 6735ac79..f09f6a4e 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -5,6 +5,7 @@ package mscalendar import ( "fmt" + "sync" "time" "github.com/mattermost/mattermost-plugin-mscalendar/server/config" @@ -12,30 +13,32 @@ 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-server/v5/model" "github.com/pkg/errors" ) const ( - availabilityTimeWindowSize = 10 * time.Minute + availabilityTimeWindowSize = 10 // minutes StatusSyncJobInterval = 5 * time.Minute upcomingEventNotificationTime = 10 * time.Minute upcomingEventNotificationWindow = (StatusSyncJobInterval * 9) / 10 //90% of the interval ) type Availability interface { - GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) + GetAvailabilities(users []*store.User) ([]*remote.ScheduleInformation, error) SyncStatus(mattermostUserID string) (string, error) SyncStatusAll() (string, error) } func (m *mscalendar) SyncStatus(mattermostUserID string) (string, error) { - user, err := m.Store.LoadUserFromIndex(mattermostUserID) + user, err := m.Store.LoadUser(mattermostUserID) if err != nil { return "", err } + if !user.Settings.UpdateStatus { + return fmt.Sprintf("Your settings are set up to not update your status. You can change your settings using `/%s settings`", config.CommandTrigger), nil + } - return m.syncStatusUsers(store.UserIndex{user}) + return m.syncStatusUsers([]*store.User{user}) } func (m *mscalendar) SyncStatusAll() (string, error) { @@ -59,25 +62,88 @@ func (m *mscalendar) SyncStatusAll() (string, error) { return "", err } - return m.syncStatusUsers(userIndex) -} - -func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) { - if len(userIndex) == 0 { - return "No connected users found", nil - } - - users := []*store.User{} + statusSyncUsers := []*store.User{} + reminderUsers := []*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 { return "", err } if user.Settings.UpdateStatus { - users = append(users, user) + statusSyncUsers = append(statusSyncUsers, user) + } + if user.Settings.GetReminders { + reminderUsers = append(reminderUsers, user) } } + + w := sync.WaitGroup{} + m.Logger.Debugf("%d user(s) want their status updated", len(statusSyncUsers)) + if len(statusSyncUsers) > 0 { + w.Add(1) + go func() { + res, err := m.syncStatusUsers(statusSyncUsers) + if err != nil { + m.Logger.Errorf("Error syncing user statuses. Error: %v", err) + } + if res != "" { + m.Logger.Debugf(res) + } + m.Logger.Debugf("Status sync finished") + w.Done() + }() + } + + m.Logger.Debugf("%d user(s) want a reminder for upcoming events", len(reminderUsers)) + if len(reminderUsers) > 0 { + w.Add(1) + go func() { + err := m.deliverReminders(reminderUsers) + if err != nil { + m.Logger.Errorf("Error delivering reminders. Error: %v", err) + } + m.Logger.Debugf("Reminder deliveries finished") + w.Done() + }() + } + + w.Wait() + return "", nil +} + +func (m *mscalendar) deliverReminders(users []*store.User) error { + err := m.Filter(withClient) + if err != nil { + return err + } + + start := time.Now().UTC() + end := time.Now().UTC().Add(availabilityTimeWindowSize * time.Minute) + + usersByRemoteID := map[string]*store.User{} + params := []*remote.ViewCalendarParams{} + for _, u := range users { + params = append(params, &remote.ViewCalendarParams{ + RemoteUserID: u.Remote.ID, + StartTime: start, + EndTime: end, + }) + usersByRemoteID[u.Remote.ID] = u + } + + responses, err := m.client.DoBatchViewCalendarRequests(params) + if err != nil { + return err + } + for _, res := range responses { + u := usersByRemoteID[res.RemoteUserID] + m.notifyUpcomingEvent(u.MattermostUserID, res.Events) + } + + return nil +} + +func (m *mscalendar) syncStatusUsers(users []*store.User) (string, error) { if len(users) == 0 { return "No users want to have their status updated", nil } @@ -93,7 +159,7 @@ func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) return m.setUserStatuses(users, schedules) } -func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { +func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ScheduleInformation) (string, error) { mattermostUserIDs := []string{} for _, u := range users { mattermostUserIDs = append(mattermostUserIDs, u.MattermostUserID) @@ -108,21 +174,20 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi statusMap[s.UserId] = s.Status } - usersByRemoteID := map[string]*store.User{} + usersByEmail := map[string]*store.User{} for _, u := range users { - usersByRemoteID[u.Remote.ID] = u + usersByEmail[u.Remote.Mail] = u } var res string for _, s := range schedules { - user := usersByRemoteID[s.RemoteUserID] + user := usersByEmail[s.ScheduleID] if s.Error != nil { m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } - mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID - m.notifyUpcomingEvent(mattermostUserID, s.Events) + mattermostUserID := usersByEmail[s.ScheduleID].MattermostUserID status, ok := statusMap[mattermostUserID] if !ok { continue @@ -141,94 +206,47 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi return utils.JSONBlock(schedules), nil } -func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, res *remote.ViewCalendarResponse) (string, error) { - events := filterBusyEvents(res.Events) - - if len(user.ActiveEvents) == 0 && len(events) == 0 { - return "No events in local or remote. No status change.", nil +func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, sched *remote.ScheduleInformation) (string, error) { + if len(sched.AvailabilityView) == 0 { + return "No availabilities to process", nil } + currentAvailability := sched.AvailabilityView[0] - var message string - if len(user.ActiveEvents) > 0 && len(events) == 0 { - if currentStatus == model.STATUS_DND { - err := m.setStatusOrAskUser(user, model.STATUS_ONLINE, events) - if err != nil { - return "", err - } - message = "User is no longer busy in calendar. Set status to online." + switch currentAvailability { + case remote.AvailabilityViewFree: + if currentStatus == "dnd" { + m.setStatusOrAskUser(user, "online", sched) + return fmt.Sprintf("User is free. Setting user from %s to online.", currentStatus), nil } else { - message = "User is no longer busy in calendar, but is not set to DND. No status change." - } - err := m.Store.StoreUserActiveEvents(user.MattermostUserID, []string{}) - if err != nil { - return "", err - } - return message, nil - } - - remoteHashes := []string{} - for _, e := range events { - if e.IsCancelled { - continue - } - h := fmt.Sprintf("%s %s", e.ID, e.Start.Time().UTC().Format(time.RFC3339)) - remoteHashes = append(remoteHashes, h) - } - - if len(user.ActiveEvents) == 0 { - if currentStatus == model.STATUS_DND { - return "User was already marked as busy. No status change.", nil - } - err := m.setStatusOrAskUser(user, model.STATUS_DND, events) - if err != nil { - return "", err - } - err = m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) - if err != nil { - return "", err + return fmt.Sprintf("User is free, and is already set to %s.", currentStatus), nil } - return "User was free, but is now busy. Set status to DND.", nil - } - - newEventExists := false - for _, r := range remoteHashes { - found := false - for _, loc := range user.ActiveEvents { - if loc == r { - found = true - break - } + case remote.AvailabilityViewBusy: + if currentStatus != "dnd" { + m.setStatusOrAskUser(user, "dnd", sched) + return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus), nil + } else { + return fmt.Sprintf("User is busy, and is already set to %s.", currentStatus), nil } - if !found { - newEventExists = true - break + case remote.AvailabilityViewOutOfOffice: + if currentStatus != "offline" { + m.setStatusOrAskUser(user, "offline", sched) + return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus), nil + } else { + return fmt.Sprintf("User is out of office, and is already set to %s.", currentStatus), nil } + case remote.AvailabilityViewWorkingElsewhere: + return fmt.Sprintf("User is working elsewhere. Pending implementation."), nil + case remote.AvailabilityViewTentative: + return fmt.Sprintf("User's availability is tentative. Don't change status."), nil } - if !newEventExists { - return fmt.Sprintf("No change in active events. Total number of events: %d", len(events)), nil - } - - if currentStatus != model.STATUS_DND { - err := m.setStatusOrAskUser(user, model.STATUS_DND, events) - if err != nil { - return "", err - } - message = "User was free, but is now busy. Set status to DND." - } else { - message = "User is already busy. No status change." - } - err := m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) - if err != nil { - return "", err - } - return message, nil + return fmt.Sprintf("Availability view doesn't match %d", currentAvailability), nil } -func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events []*remote.Event) error { +func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, sched *remote.ScheduleInformation) error { if user.Settings.GetConfirmation { url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) - _, err := m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(events, status, url)) + _, err := m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(sched, status, url)) return err } @@ -239,25 +257,24 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events return nil } -func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) { +func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ScheduleInformation, error) { err := m.Filter(withClient) if err != nil { return nil, err } - start := time.Now().UTC() - end := time.Now().UTC().Add(availabilityTimeWindowSize) - - params := []*remote.ViewCalendarParams{} + params := []*remote.ScheduleUserInfo{} for _, u := range users { - params = append(params, &remote.ViewCalendarParams{ + params = append(params, &remote.ScheduleUserInfo{ RemoteUserID: u.Remote.ID, - StartTime: start, - EndTime: end, + Mail: u.Remote.Mail, }) } - return m.client.DoBatchViewCalendarRequests(params) + start := remote.NewDateTime(timeNowFunc().UTC(), "UTC") + end := remote.NewDateTime(timeNowFunc().UTC().Add(availabilityTimeWindowSize*time.Minute), "UTC") + + return m.client.GetSchedule(params, start, end, availabilityTimeWindowSize) } func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remote.Event) { diff --git a/server/mscalendar/settings.go b/server/mscalendar/settings.go index bf821692..a15395ef 100644 --- a/server/mscalendar/settings.go +++ b/server/mscalendar/settings.go @@ -35,6 +35,13 @@ func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingS store.UpdateStatusSettingID, settingStore, )) + settings = append(settings, settingspanel.NewBoolSetting( + store.GetRemindersSettingID, + "Get Reminders", + "Do you want to get reminders for upcoming events?", + "", + settingStore, + )) settings = append(settings, NewDailySummarySetting( settingStore, getTimezone, diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index 51ba0e91..c2e56710 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -15,22 +15,22 @@ var prettyStatuses = map[string]string{ model.STATUS_OFFLINE: "Offline", } -func RenderStatusChangeNotificationView(events []*remote.Event, status, url string) *model.SlackAttachment { - for _, e := range events { - if e.Start.Time().After(time.Now()) { - return statusChangeAttachments(e, status, url) +func RenderStatusChangeNotificationView(sched *remote.ScheduleInformation, status, url string) *model.SlackAttachment { + for _, s := range sched.ScheduleItems { + if s.Start.Time().After(time.Now()) { + return statusChangeAttachments(s, status, url) } } return statusChangeAttachments(nil, status, url) } -func renderScheduleItem(event *remote.Event, status string) string { - if event == nil { +func renderScheduleItem(s *remote.ScheduleItem, status string) string { + if s == nil { return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", prettyStatuses[status]) } - resp := fmt.Sprintf("Your event with subject `%s` will start soon.", event.Subject) - if event.Subject == "" { + resp := fmt.Sprintf("Your event with subject `%s` will start soon.", s.Subject) + if s.Subject == "" { resp = "An event with no subject will start soon." } @@ -38,7 +38,7 @@ func renderScheduleItem(event *remote.Event, status string) string { return resp } -func statusChangeAttachments(event *remote.Event, status, url string) *model.SlackAttachment { +func statusChangeAttachments(s *remote.ScheduleItem, status, url string) *model.SlackAttachment { actionYes := &model.PostAction{ Name: "Yes", Integration: &model.PostActionIntegration{ @@ -63,7 +63,7 @@ func statusChangeAttachments(event *remote.Event, status, url string) *model.Sla sa := &model.SlackAttachment{ Title: "Status change", - Text: renderScheduleItem(event, status), + Text: renderScheduleItem(s, status), Actions: []*model.PostAction{actionYes, actionNo}, } diff --git a/server/store/setting_store.go b/server/store/setting_store.go index e25928e3..514e71be 100644 --- a/server/store/setting_store.go +++ b/server/store/setting_store.go @@ -8,6 +8,7 @@ import ( const ( UpdateStatusSettingID = "update_status" GetConfirmationSettingID = "get_confirmation" + GetRemindersSettingID = "get_reminders" DailySummarySettingID = "summary_setting" ) @@ -30,6 +31,12 @@ 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.GetConfirmation = storableValue + case GetRemindersSettingID: + storableValue, ok := value.(bool) + if !ok { + return fmt.Errorf("cannot read value %v for setting %s (expecting bool)", value, settingID) + } + user.Settings.GetReminders = storableValue case DailySummarySettingID: s.updateDailySummarySettingForUser(userID, value) default: @@ -55,6 +62,8 @@ func (s *pluginStore) GetSetting(userID, settingID string) (interface{}, error) return user.Settings.UpdateStatus, nil case GetConfirmationSettingID: return user.Settings.GetConfirmation, nil + case GetRemindersSettingID: + return user.Settings.GetReminders, nil case DailySummarySettingID: return s.LoadDailySummaryUserSettings(userID) default: diff --git a/server/store/user_store.go b/server/store/user_store.go index 291f25f2..88678b33 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -22,7 +22,6 @@ type UserStore interface { ModifyUserIndex(modify func(userIndex UserIndex) (UserIndex, error)) error StoreUserInIndex(user *User) error DeleteUserFromIndex(mattermostUserID string) error - StoreUserActiveEvents(mattermostUserID string, events []string) error } type UserIndex []*UserShort @@ -41,13 +40,13 @@ type User struct { Settings Settings `json:"mattermostSettings,omitempty"` LastStatusUpdateAvailability byte LastStatusUpdateEventTime *remote.DateTime - ActiveEvents []string `json:"events"` } type Settings struct { EventSubscriptionID string UpdateStatus bool GetConfirmation bool + GetReminders bool } func (settings Settings) String() string { @@ -210,15 +209,6 @@ func (s *pluginStore) DeleteUserFromIndex(mattermostUserID string) error { }) } -func (s *pluginStore) StoreUserActiveEvents(mattermostUserID string, events []string) error { - u, err := s.LoadUser(mattermostUserID) - if err != nil { - return err - } - u.ActiveEvents = events - return kvstore.StoreJSON(s.userKV, mattermostUserID, u) -} - func (index UserIndex) ByMattermostID() map[string]*UserShort { result := map[string]*UserShort{} From dfbd7e053287f6d7ffcfa084c68c44b296cf60a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 23 Apr 2020 15:40:21 +0200 Subject: [PATCH 12/20] Revert "Use GetSchedule for status and GetCalendarView for reminders" This reverts commit 18d48bd7acbba2ad0716fd9655a553c974542de9. --- server/mscalendar/availability.go | 231 ++++++++---------- server/mscalendar/settings.go | 7 - .../views/status_change_notification.go | 20 +- server/store/setting_store.go | 9 - server/store/user_store.go | 12 +- 5 files changed, 128 insertions(+), 151 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index f09f6a4e..6735ac79 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -5,7 +5,6 @@ package mscalendar import ( "fmt" - "sync" "time" "github.com/mattermost/mattermost-plugin-mscalendar/server/config" @@ -13,32 +12,30 @@ 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-server/v5/model" "github.com/pkg/errors" ) const ( - availabilityTimeWindowSize = 10 // minutes + availabilityTimeWindowSize = 10 * time.Minute StatusSyncJobInterval = 5 * time.Minute upcomingEventNotificationTime = 10 * time.Minute upcomingEventNotificationWindow = (StatusSyncJobInterval * 9) / 10 //90% of the interval ) type Availability interface { - GetAvailabilities(users []*store.User) ([]*remote.ScheduleInformation, error) + GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) SyncStatus(mattermostUserID string) (string, error) SyncStatusAll() (string, error) } func (m *mscalendar) SyncStatus(mattermostUserID string) (string, error) { - user, err := m.Store.LoadUser(mattermostUserID) + user, err := m.Store.LoadUserFromIndex(mattermostUserID) if err != nil { return "", err } - if !user.Settings.UpdateStatus { - return fmt.Sprintf("Your settings are set up to not update your status. You can change your settings using `/%s settings`", config.CommandTrigger), nil - } - return m.syncStatusUsers([]*store.User{user}) + return m.syncStatusUsers(store.UserIndex{user}) } func (m *mscalendar) SyncStatusAll() (string, error) { @@ -62,88 +59,25 @@ func (m *mscalendar) SyncStatusAll() (string, error) { return "", err } - statusSyncUsers := []*store.User{} - reminderUsers := []*store.User{} + return m.syncStatusUsers(userIndex) +} + +func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) { + if len(userIndex) == 0 { + return "No connected users found", nil + } + + 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 { return "", err } if user.Settings.UpdateStatus { - statusSyncUsers = append(statusSyncUsers, user) - } - if user.Settings.GetReminders { - reminderUsers = append(reminderUsers, user) + users = append(users, user) } } - - w := sync.WaitGroup{} - m.Logger.Debugf("%d user(s) want their status updated", len(statusSyncUsers)) - if len(statusSyncUsers) > 0 { - w.Add(1) - go func() { - res, err := m.syncStatusUsers(statusSyncUsers) - if err != nil { - m.Logger.Errorf("Error syncing user statuses. Error: %v", err) - } - if res != "" { - m.Logger.Debugf(res) - } - m.Logger.Debugf("Status sync finished") - w.Done() - }() - } - - m.Logger.Debugf("%d user(s) want a reminder for upcoming events", len(reminderUsers)) - if len(reminderUsers) > 0 { - w.Add(1) - go func() { - err := m.deliverReminders(reminderUsers) - if err != nil { - m.Logger.Errorf("Error delivering reminders. Error: %v", err) - } - m.Logger.Debugf("Reminder deliveries finished") - w.Done() - }() - } - - w.Wait() - return "", nil -} - -func (m *mscalendar) deliverReminders(users []*store.User) error { - err := m.Filter(withClient) - if err != nil { - return err - } - - start := time.Now().UTC() - end := time.Now().UTC().Add(availabilityTimeWindowSize * time.Minute) - - usersByRemoteID := map[string]*store.User{} - params := []*remote.ViewCalendarParams{} - for _, u := range users { - params = append(params, &remote.ViewCalendarParams{ - RemoteUserID: u.Remote.ID, - StartTime: start, - EndTime: end, - }) - usersByRemoteID[u.Remote.ID] = u - } - - responses, err := m.client.DoBatchViewCalendarRequests(params) - if err != nil { - return err - } - for _, res := range responses { - u := usersByRemoteID[res.RemoteUserID] - m.notifyUpcomingEvent(u.MattermostUserID, res.Events) - } - - return nil -} - -func (m *mscalendar) syncStatusUsers(users []*store.User) (string, error) { if len(users) == 0 { return "No users want to have their status updated", nil } @@ -159,7 +93,7 @@ func (m *mscalendar) syncStatusUsers(users []*store.User) (string, error) { return m.setUserStatuses(users, schedules) } -func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ScheduleInformation) (string, error) { +func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { mattermostUserIDs := []string{} for _, u := range users { mattermostUserIDs = append(mattermostUserIDs, u.MattermostUserID) @@ -174,20 +108,21 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Sc statusMap[s.UserId] = s.Status } - usersByEmail := map[string]*store.User{} + usersByRemoteID := map[string]*store.User{} for _, u := range users { - usersByEmail[u.Remote.Mail] = u + usersByRemoteID[u.Remote.ID] = u } var res string for _, s := range schedules { - user := usersByEmail[s.ScheduleID] + user := usersByRemoteID[s.RemoteUserID] if s.Error != nil { m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } - mattermostUserID := usersByEmail[s.ScheduleID].MattermostUserID + mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID + m.notifyUpcomingEvent(mattermostUserID, s.Events) status, ok := statusMap[mattermostUserID] if !ok { continue @@ -206,47 +141,94 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Sc return utils.JSONBlock(schedules), nil } -func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, sched *remote.ScheduleInformation) (string, error) { - if len(sched.AvailabilityView) == 0 { - return "No availabilities to process", nil +func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, res *remote.ViewCalendarResponse) (string, error) { + events := filterBusyEvents(res.Events) + + if len(user.ActiveEvents) == 0 && len(events) == 0 { + return "No events in local or remote. No status change.", nil } - currentAvailability := sched.AvailabilityView[0] - switch currentAvailability { - case remote.AvailabilityViewFree: - if currentStatus == "dnd" { - m.setStatusOrAskUser(user, "online", sched) - return fmt.Sprintf("User is free. Setting user from %s to online.", currentStatus), nil + var message string + if len(user.ActiveEvents) > 0 && len(events) == 0 { + if currentStatus == model.STATUS_DND { + err := m.setStatusOrAskUser(user, model.STATUS_ONLINE, events) + if err != nil { + return "", err + } + message = "User is no longer busy in calendar. Set status to online." } else { - return fmt.Sprintf("User is free, and is already set to %s.", currentStatus), nil + message = "User is no longer busy in calendar, but is not set to DND. No status change." } - case remote.AvailabilityViewBusy: - if currentStatus != "dnd" { - m.setStatusOrAskUser(user, "dnd", sched) - return fmt.Sprintf("User is busy. Setting user from %s to dnd.", currentStatus), nil - } else { - return fmt.Sprintf("User is busy, and is already set to %s.", currentStatus), nil + err := m.Store.StoreUserActiveEvents(user.MattermostUserID, []string{}) + if err != nil { + return "", err } - case remote.AvailabilityViewOutOfOffice: - if currentStatus != "offline" { - m.setStatusOrAskUser(user, "offline", sched) - return fmt.Sprintf("User is out of office. Setting user from %s to offline", currentStatus), nil - } else { - return fmt.Sprintf("User is out of office, and is already set to %s.", currentStatus), nil + return message, nil + } + + remoteHashes := []string{} + for _, e := range events { + if e.IsCancelled { + continue + } + h := fmt.Sprintf("%s %s", e.ID, e.Start.Time().UTC().Format(time.RFC3339)) + remoteHashes = append(remoteHashes, h) + } + + if len(user.ActiveEvents) == 0 { + if currentStatus == model.STATUS_DND { + return "User was already marked as busy. No status change.", nil + } + err := m.setStatusOrAskUser(user, model.STATUS_DND, events) + if err != nil { + return "", err + } + err = m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) + if err != nil { + return "", err } - case remote.AvailabilityViewWorkingElsewhere: - return fmt.Sprintf("User is working elsewhere. Pending implementation."), nil - case remote.AvailabilityViewTentative: - return fmt.Sprintf("User's availability is tentative. Don't change status."), nil + return "User was free, but is now busy. Set status to DND.", nil } - return fmt.Sprintf("Availability view doesn't match %d", currentAvailability), nil + newEventExists := false + for _, r := range remoteHashes { + found := false + for _, loc := range user.ActiveEvents { + if loc == r { + found = true + break + } + } + if !found { + newEventExists = true + break + } + } + + if !newEventExists { + return fmt.Sprintf("No change in active events. Total number of events: %d", len(events)), nil + } + + if currentStatus != model.STATUS_DND { + err := m.setStatusOrAskUser(user, model.STATUS_DND, events) + if err != nil { + return "", err + } + message = "User was free, but is now busy. Set status to DND." + } else { + message = "User is already busy. No status change." + } + err := m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) + if err != nil { + return "", err + } + return message, nil } -func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, sched *remote.ScheduleInformation) error { +func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events []*remote.Event) error { if user.Settings.GetConfirmation { url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) - _, err := m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(sched, status, url)) + _, err := m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(events, status, url)) return err } @@ -257,24 +239,25 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, sched * return nil } -func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ScheduleInformation, error) { +func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) { err := m.Filter(withClient) if err != nil { return nil, err } - params := []*remote.ScheduleUserInfo{} + start := time.Now().UTC() + end := time.Now().UTC().Add(availabilityTimeWindowSize) + + params := []*remote.ViewCalendarParams{} for _, u := range users { - params = append(params, &remote.ScheduleUserInfo{ + params = append(params, &remote.ViewCalendarParams{ RemoteUserID: u.Remote.ID, - Mail: u.Remote.Mail, + StartTime: start, + EndTime: end, }) } - start := remote.NewDateTime(timeNowFunc().UTC(), "UTC") - end := remote.NewDateTime(timeNowFunc().UTC().Add(availabilityTimeWindowSize*time.Minute), "UTC") - - return m.client.GetSchedule(params, start, end, availabilityTimeWindowSize) + return m.client.DoBatchViewCalendarRequests(params) } func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remote.Event) { diff --git a/server/mscalendar/settings.go b/server/mscalendar/settings.go index a15395ef..bf821692 100644 --- a/server/mscalendar/settings.go +++ b/server/mscalendar/settings.go @@ -35,13 +35,6 @@ func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingS store.UpdateStatusSettingID, settingStore, )) - settings = append(settings, settingspanel.NewBoolSetting( - store.GetRemindersSettingID, - "Get Reminders", - "Do you want to get reminders for upcoming events?", - "", - settingStore, - )) settings = append(settings, NewDailySummarySetting( settingStore, getTimezone, diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index c2e56710..51ba0e91 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -15,22 +15,22 @@ var prettyStatuses = map[string]string{ model.STATUS_OFFLINE: "Offline", } -func RenderStatusChangeNotificationView(sched *remote.ScheduleInformation, status, url string) *model.SlackAttachment { - for _, s := range sched.ScheduleItems { - if s.Start.Time().After(time.Now()) { - return statusChangeAttachments(s, status, url) +func RenderStatusChangeNotificationView(events []*remote.Event, status, url string) *model.SlackAttachment { + for _, e := range events { + if e.Start.Time().After(time.Now()) { + return statusChangeAttachments(e, status, url) } } return statusChangeAttachments(nil, status, url) } -func renderScheduleItem(s *remote.ScheduleItem, status string) string { - if s == nil { +func renderScheduleItem(event *remote.Event, status string) string { + if event == nil { return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", prettyStatuses[status]) } - resp := fmt.Sprintf("Your event with subject `%s` will start soon.", s.Subject) - if s.Subject == "" { + resp := fmt.Sprintf("Your event with subject `%s` will start soon.", event.Subject) + if event.Subject == "" { resp = "An event with no subject will start soon." } @@ -38,7 +38,7 @@ func renderScheduleItem(s *remote.ScheduleItem, status string) string { return resp } -func statusChangeAttachments(s *remote.ScheduleItem, status, url string) *model.SlackAttachment { +func statusChangeAttachments(event *remote.Event, status, url string) *model.SlackAttachment { actionYes := &model.PostAction{ Name: "Yes", Integration: &model.PostActionIntegration{ @@ -63,7 +63,7 @@ func statusChangeAttachments(s *remote.ScheduleItem, status, url string) *model. sa := &model.SlackAttachment{ Title: "Status change", - Text: renderScheduleItem(s, status), + Text: renderScheduleItem(event, status), Actions: []*model.PostAction{actionYes, actionNo}, } diff --git a/server/store/setting_store.go b/server/store/setting_store.go index 514e71be..e25928e3 100644 --- a/server/store/setting_store.go +++ b/server/store/setting_store.go @@ -8,7 +8,6 @@ import ( const ( UpdateStatusSettingID = "update_status" GetConfirmationSettingID = "get_confirmation" - GetRemindersSettingID = "get_reminders" DailySummarySettingID = "summary_setting" ) @@ -31,12 +30,6 @@ 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.GetConfirmation = storableValue - case GetRemindersSettingID: - storableValue, ok := value.(bool) - if !ok { - return fmt.Errorf("cannot read value %v for setting %s (expecting bool)", value, settingID) - } - user.Settings.GetReminders = storableValue case DailySummarySettingID: s.updateDailySummarySettingForUser(userID, value) default: @@ -62,8 +55,6 @@ func (s *pluginStore) GetSetting(userID, settingID string) (interface{}, error) return user.Settings.UpdateStatus, nil case GetConfirmationSettingID: return user.Settings.GetConfirmation, nil - case GetRemindersSettingID: - return user.Settings.GetReminders, nil case DailySummarySettingID: return s.LoadDailySummaryUserSettings(userID) default: diff --git a/server/store/user_store.go b/server/store/user_store.go index 88678b33..291f25f2 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -22,6 +22,7 @@ type UserStore interface { ModifyUserIndex(modify func(userIndex UserIndex) (UserIndex, error)) error StoreUserInIndex(user *User) error DeleteUserFromIndex(mattermostUserID string) error + StoreUserActiveEvents(mattermostUserID string, events []string) error } type UserIndex []*UserShort @@ -40,13 +41,13 @@ type User struct { Settings Settings `json:"mattermostSettings,omitempty"` LastStatusUpdateAvailability byte LastStatusUpdateEventTime *remote.DateTime + ActiveEvents []string `json:"events"` } type Settings struct { EventSubscriptionID string UpdateStatus bool GetConfirmation bool - GetReminders bool } func (settings Settings) String() string { @@ -209,6 +210,15 @@ func (s *pluginStore) DeleteUserFromIndex(mattermostUserID string) error { }) } +func (s *pluginStore) StoreUserActiveEvents(mattermostUserID string, events []string) error { + u, err := s.LoadUser(mattermostUserID) + if err != nil { + return err + } + u.ActiveEvents = events + return kvstore.StoreJSON(s.userKV, mattermostUserID, u) +} + func (index UserIndex) ByMattermostID() map[string]*UserShort { result := map[string]*UserShort{} From 7ead072abf10fe49bce1ec3f34cc39bea1ca5b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 23 Apr 2020 16:18:40 +0200 Subject: [PATCH 13/20] Fix panic due to error handling and change check to include events among active even if the status did not change. --- server/mscalendar/availability.go | 21 +++++++++++++++++---- server/utils/pluginapi/api.go | 6 +++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index 6735ac79..f673fe1e 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -90,7 +90,12 @@ func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) return "No schedule info found", nil } - return m.setUserStatuses(users, schedules) + out, err := m.setUserStatuses(users, schedules) + if err != nil { + return out, err + } + + return out, nil } func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { @@ -171,15 +176,20 @@ func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus s if e.IsCancelled { continue } - h := fmt.Sprintf("%s %s", e.ID, e.Start.Time().UTC().Format(time.RFC3339)) + h := fmt.Sprintf("%s %s", e.ICalUID, e.Start.Time().UTC().Format(time.RFC3339)) remoteHashes = append(remoteHashes, h) } if len(user.ActiveEvents) == 0 { + var err error if currentStatus == model.STATUS_DND { + err = m.Store.StoreUserActiveEvents(user.MattermostUserID, remoteHashes) + if err != nil { + return "", err + } return "User was already marked as busy. No status change.", nil } - err := m.setStatusOrAskUser(user, model.STATUS_DND, events) + err = m.setStatusOrAskUser(user, model.STATUS_DND, events) if err != nil { return "", err } @@ -229,7 +239,10 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events if user.Settings.GetConfirmation { url := fmt.Sprintf("%s%s%s", m.Config.PluginURLPath, config.PathPostAction, config.PathConfirmStatusChange) _, err := m.Poster.DMWithAttachments(user.MattermostUserID, views.RenderStatusChangeNotificationView(events, status, url)) - return err + if err != nil { + return err + } + return nil } _, appErr := m.PluginAPI.UpdateMattermostUserStatus(user.MattermostUserID, status) diff --git a/server/utils/pluginapi/api.go b/server/utils/pluginapi/api.go index a1b1a536..88b12e5e 100644 --- a/server/utils/pluginapi/api.go +++ b/server/utils/pluginapi/api.go @@ -35,7 +35,11 @@ func (a *API) GetMattermostUserStatusesByIds(mattermostUserIDs []string) ([]*mod } func (a *API) UpdateMattermostUserStatus(mattermostUserID, status string) (*model.Status, error) { - return a.api.UpdateUserStatus(mattermostUserID, status) + s, err := a.api.UpdateUserStatus(mattermostUserID, status) + if err != nil { + return s, err + } + return s, nil } // IsSysAdmin returns true if the user is authorized to use the workflow plugin's admin-level APIs/commands. From cd3cc6d89ce7ab1766d90610b8323af750fdae91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 23 Apr 2020 17:53:28 +0200 Subject: [PATCH 14/20] Add GetReminder setting and fix issue where a user that decided not to update its status would not receive reminders --- server/command/availability.go | 4 +- server/jobs/status_sync_job.go | 6 +- server/mscalendar/availability.go | 73 ++++++++++++++----- server/mscalendar/availability_test.go | 4 +- .../mock_mscalendar/mock_mscalendar.go | 24 +++--- server/mscalendar/settings.go | 7 ++ server/store/setting_store.go | 9 +++ server/store/user_store.go | 1 + 8 files changed, 90 insertions(+), 38 deletions(-) diff --git a/server/command/availability.go b/server/command/availability.go index dbc3fe2f..0fe11438 100644 --- a/server/command/availability.go +++ b/server/command/availability.go @@ -6,14 +6,14 @@ package command func (c *Command) availability(parameters ...string) (string, error) { switch { case len(parameters) == 0: - resString, err := c.MSCalendar.SyncStatus(c.Args.UserId) + resString, err := c.MSCalendar.Sync(c.Args.UserId) if err != nil { return "", err } return resString, nil case len(parameters) == 1 && parameters[0] == "all": - resString, err := c.MSCalendar.SyncStatusAll() + resString, err := c.MSCalendar.SyncAll() if err != nil { return "", err } diff --git a/server/jobs/status_sync_job.go b/server/jobs/status_sync_job.go index 9a283429..9e917e7f 100644 --- a/server/jobs/status_sync_job.go +++ b/server/jobs/status_sync_job.go @@ -15,16 +15,16 @@ func NewStatusSyncJob() RegisteredJob { return RegisteredJob{ id: statusSyncJobID, interval: mscalendar.StatusSyncJobInterval, - work: runStatusSyncJob, + work: runSyncJob, isEnabledByConfig: isStatusSyncJobEnabled, } } // runStatusSyncJob synchronizes all users' statuses between mscalendar and Mattermost. -func runStatusSyncJob(env mscalendar.Env) { +func runSyncJob(env mscalendar.Env) { env.Logger.Debugf("User status sync job beginning") - _, err := mscalendar.New(env, "").SyncStatusAll() + _, err := mscalendar.New(env, "").SyncAll() if err != nil { env.Logger.Errorf("Error during user status sync job: %v", err) } diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index f673fe1e..af143e61 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -25,26 +25,26 @@ const ( type Availability interface { GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) - SyncStatus(mattermostUserID string) (string, error) - SyncStatusAll() (string, error) + Sync(mattermostUserID string) (string, error) + SyncAll() (string, error) } -func (m *mscalendar) SyncStatus(mattermostUserID string) (string, error) { +func (m *mscalendar) Sync(mattermostUserID string) (string, error) { user, err := m.Store.LoadUserFromIndex(mattermostUserID) if err != nil { return "", err } - return m.syncStatusUsers(store.UserIndex{user}) + return m.syncUsers(store.UserIndex{user}) } -func (m *mscalendar) SyncStatusAll() (string, error) { +func (m *mscalendar) SyncAll() (string, error) { isAdmin, err := m.IsAuthorizedAdmin(m.actingUser.MattermostUserID) if err != nil { return "", err } if !isAdmin { - return "", errors.Errorf("Non-admin user attempting SyncStatusAll %s", m.actingUser.MattermostUserID) + return "", errors.Errorf("Non-admin user attempting SyncAll %s", m.actingUser.MattermostUserID) } err = m.Filter(withSuperuserClient) if err != nil { @@ -59,10 +59,10 @@ func (m *mscalendar) SyncStatusAll() (string, error) { return "", err } - return m.syncStatusUsers(userIndex) + return m.syncUsers(userIndex) } -func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) { +func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, error) { if len(userIndex) == 0 { return "No connected users found", nil } @@ -74,12 +74,12 @@ func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) if err != nil { return "", err } - if user.Settings.UpdateStatus { + if user.Settings.UpdateStatus || user.Settings.GetReminders { users = append(users, user) } } if len(users) == 0 { - return "No users want to have their status updated", nil + return "No users need to be synced", nil } schedules, err := m.GetAvailabilities(users) @@ -90,18 +90,56 @@ func (m *mscalendar) syncStatusUsers(userIndex store.UserIndex) (string, error) return "No schedule info found", nil } + m.deliverReminders(users, schedules) out, err := m.setUserStatuses(users, schedules) if err != nil { - return out, err + return "", err } return out, nil } +func (m *mscalendar) deliverReminders(users []*store.User, schedules []*remote.ViewCalendarResponse) { + toNotify := []*store.User{} + for _, u := range users { + if u.Settings.GetReminders { + toNotify = append(toNotify, u) + } + } + + usersByRemoteID := map[string]*store.User{} + for _, u := range toNotify { + usersByRemoteID[u.Remote.ID] = u + } + + for _, s := range schedules { + user, ok := usersByRemoteID[s.RemoteUserID] + if !ok { + continue + } + if s.Error != nil { + m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) + continue + } + + mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID + m.notifyUpcomingEvent(mattermostUserID, s.Events) + } +} + func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { - mattermostUserIDs := []string{} + toUpdate := []*store.User{} for _, u := range users { + if u.Settings.UpdateStatus { + toUpdate = append(toUpdate, u) + } + } + + mattermostUserIDs := []string{} + usersByRemoteID := map[string]*store.User{} + for _, u := range toUpdate { mattermostUserIDs = append(mattermostUserIDs, u.MattermostUserID) + usersByRemoteID[u.Remote.ID] = u } statuses, appErr := m.PluginAPI.GetMattermostUserStatusesByIds(mattermostUserIDs) @@ -113,21 +151,18 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi statusMap[s.UserId] = s.Status } - usersByRemoteID := map[string]*store.User{} - for _, u := range users { - usersByRemoteID[u.Remote.ID] = u - } - var res string for _, s := range schedules { - user := usersByRemoteID[s.RemoteUserID] + user, ok := usersByRemoteID[s.RemoteUserID] + if !ok { + continue + } if s.Error != nil { m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID - m.notifyUpcomingEvent(mattermostUserID, s.Events) status, ok := statusMap[mattermostUserID] if !ok { continue diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 9a6d65c4..f02b34b4 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -116,7 +116,7 @@ func TestSyncStatusAll(t *testing.T) { } mscalendar := New(env, "") - res, err := mscalendar.SyncStatusAll() + res, err := mscalendar.SyncAll() require.Nil(t, err) require.NotEmpty(t, res) }) @@ -177,7 +177,7 @@ func TestSyncStatusUserConfig(t *testing.T) { tc.runAssertions(env.Dependencies, client) mscalendar := New(env, "") - _, err := mscalendar.SyncStatusAll() + _, err := mscalendar.SyncAll() require.Nil(t, err) }) } diff --git a/server/mscalendar/mock_mscalendar/mock_mscalendar.go b/server/mscalendar/mock_mscalendar/mock_mscalendar.go index 52d637ca..d723d85e 100644 --- a/server/mscalendar/mock_mscalendar/mock_mscalendar.go +++ b/server/mscalendar/mock_mscalendar/mock_mscalendar.go @@ -441,34 +441,34 @@ func (mr *MockMSCalendarMockRecorder) SetDailySummaryPostTime(arg0, arg1 interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetDailySummaryPostTime", reflect.TypeOf((*MockMSCalendar)(nil).SetDailySummaryPostTime), arg0, arg1) } -// SyncStatus mocks base method -func (m *MockMSCalendar) SyncStatus(arg0 string) (string, error) { +// Sync mocks base method +func (m *MockMSCalendar) Sync(arg0 string) (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SyncStatus", arg0) + ret := m.ctrl.Call(m, "Sync", arg0) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } -// SyncStatus indicates an expected call of SyncStatus -func (mr *MockMSCalendarMockRecorder) SyncStatus(arg0 interface{}) *gomock.Call { +// Sync indicates an expected call of Sync +func (mr *MockMSCalendarMockRecorder) Sync(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncStatus", reflect.TypeOf((*MockMSCalendar)(nil).SyncStatus), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sync", reflect.TypeOf((*MockMSCalendar)(nil).Sync), arg0) } -// SyncStatusAll mocks base method -func (m *MockMSCalendar) SyncStatusAll() (string, error) { +// SyncAll mocks base method +func (m *MockMSCalendar) SyncAll() (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SyncStatusAll") + ret := m.ctrl.Call(m, "SyncAll") ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } -// SyncStatusAll indicates an expected call of SyncStatusAll -func (mr *MockMSCalendarMockRecorder) SyncStatusAll() *gomock.Call { +// SyncAll indicates an expected call of SyncAll +func (mr *MockMSCalendarMockRecorder) SyncAll() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncStatusAll", reflect.TypeOf((*MockMSCalendar)(nil).SyncStatusAll)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncAll", reflect.TypeOf((*MockMSCalendar)(nil).SyncAll)) } // TentativelyAcceptEvent mocks base method diff --git a/server/mscalendar/settings.go b/server/mscalendar/settings.go index bf821692..a15395ef 100644 --- a/server/mscalendar/settings.go +++ b/server/mscalendar/settings.go @@ -35,6 +35,13 @@ func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingS store.UpdateStatusSettingID, settingStore, )) + settings = append(settings, settingspanel.NewBoolSetting( + store.GetRemindersSettingID, + "Get Reminders", + "Do you want to get reminders for upcoming events?", + "", + settingStore, + )) settings = append(settings, NewDailySummarySetting( settingStore, getTimezone, diff --git a/server/store/setting_store.go b/server/store/setting_store.go index e25928e3..514e71be 100644 --- a/server/store/setting_store.go +++ b/server/store/setting_store.go @@ -8,6 +8,7 @@ import ( const ( UpdateStatusSettingID = "update_status" GetConfirmationSettingID = "get_confirmation" + GetRemindersSettingID = "get_reminders" DailySummarySettingID = "summary_setting" ) @@ -30,6 +31,12 @@ 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.GetConfirmation = storableValue + case GetRemindersSettingID: + storableValue, ok := value.(bool) + if !ok { + return fmt.Errorf("cannot read value %v for setting %s (expecting bool)", value, settingID) + } + user.Settings.GetReminders = storableValue case DailySummarySettingID: s.updateDailySummarySettingForUser(userID, value) default: @@ -55,6 +62,8 @@ func (s *pluginStore) GetSetting(userID, settingID string) (interface{}, error) return user.Settings.UpdateStatus, nil case GetConfirmationSettingID: return user.Settings.GetConfirmation, nil + case GetRemindersSettingID: + return user.Settings.GetReminders, nil case DailySummarySettingID: return s.LoadDailySummaryUserSettings(userID) default: diff --git a/server/store/user_store.go b/server/store/user_store.go index 291f25f2..1e9ee6d1 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -48,6 +48,7 @@ type Settings struct { EventSubscriptionID string UpdateStatus bool GetConfirmation bool + GetReminders bool } func (settings Settings) String() string { From fd5c45db190c20c1626db0b3dd6d2a5d305437a8 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 23 Apr 2020 21:14:58 -0400 Subject: [PATCH 15/20] short circuit when no users want status, same with reminders. fix tests --- server/mscalendar/availability.go | 18 ++-- server/mscalendar/availability_test.go | 130 +++++++++++++++++++++++-- 2 files changed, 133 insertions(+), 15 deletions(-) diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index af143e61..b19be2bd 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -106,6 +106,9 @@ func (m *mscalendar) deliverReminders(users []*store.User, schedules []*remote.V toNotify = append(toNotify, u) } } + if len(toNotify) == 0 { + return + } usersByRemoteID := map[string]*store.User{} for _, u := range toNotify { @@ -118,7 +121,7 @@ func (m *mscalendar) deliverReminders(users []*store.User, schedules []*remote.V continue } if s.Error != nil { - m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) + m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } @@ -134,6 +137,9 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi toUpdate = append(toUpdate, u) } } + if len(toUpdate) == 0 { + return "No users want their status", nil + } mattermostUserIDs := []string{} usersByRemoteID := map[string]*store.User{} @@ -158,7 +164,7 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi continue } if s.Error != nil { - m.Logger.Errorf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) + m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) continue } @@ -171,7 +177,7 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi var err error res, err = m.setStatusFromAvailability(user, status, s) if err != nil { - m.Logger.Errorf("Error setting user %s status. %s", user.Remote.Mail, err.Error()) + m.Logger.Warnf("Error setting user %s status. %s", user.Remote.Mail, err.Error()) } } if res != "" { @@ -323,19 +329,19 @@ func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remo if timezone == "" { timezone, err = m.GetTimezoneByID(mattermostUserID) if err != nil { - m.Logger.Errorf("notifyUpcomingEvent error getting timezone, err=%s", err.Error()) + m.Logger.Warnf("notifyUpcomingEvent error getting timezone, err=%s", err.Error()) return } } message, err := views.RenderScheduleItem(event, timezone) if err != nil { - m.Logger.Errorf("notifyUpcomingEvent error rendering schedule item, err=", err.Error()) + m.Logger.Warnf("notifyUpcomingEvent error rendering schedule item, err=", err.Error()) continue } err = m.Poster.DM(mattermostUserID, message) if err != nil { - m.Logger.Errorf("notifyUpcomingEvent error creating DM, err=", err.Error()) + m.Logger.Warnf("notifyUpcomingEvent error creating DM, err=", err.Error()) continue } } diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index f02b34b4..078b7f56 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -19,21 +19,22 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/remote/mock_remote" "github.com/mattermost/mattermost-plugin-mscalendar/server/store" "github.com/mattermost/mattermost-plugin-mscalendar/server/store/mock_store" - "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot/mock_bot" ) func TestSyncStatusAll(t *testing.T) { moment := time.Now().UTC() eventHash := "event_id " + moment.Format(time.RFC3339) - busyEvent := &remote.Event{ID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} + busyEvent := &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} for name, tc := range map[string]struct { remoteEvents []*remote.Event + apiError *remote.ApiError activeEvents []string currentStatus string newStatus string eventsToStore []string + errorLogged string }{ "Most common case, no events local or remote. No status change.": { remoteEvents: []*remote.Event{}, @@ -41,6 +42,7 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "online", newStatus: "", eventsToStore: nil, + errorLogged: "", }, "New remote event. Change status to DND.": { remoteEvents: []*remote.Event{busyEvent}, @@ -48,6 +50,7 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "online", newStatus: "dnd", eventsToStore: []string{eventHash}, + errorLogged: "", }, "Locally stored event is finished. Change status to online.": { remoteEvents: []*remote.Event{}, @@ -55,6 +58,7 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "dnd", newStatus: "online", eventsToStore: []string{}, + errorLogged: "", }, "Locally stored event is still happening. No status change.": { remoteEvents: []*remote.Event{busyEvent}, @@ -62,6 +66,7 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "dnd", newStatus: "", eventsToStore: nil, + errorLogged: "", }, "User has manually set themselves to online during event. Locally stored event is still happening, but we will ignore it. No status change.": { remoteEvents: []*remote.Event{busyEvent}, @@ -69,6 +74,7 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "online", newStatus: "", eventsToStore: nil, + errorLogged: "", }, "Ignore non-busy event": { remoteEvents: []*remote.Event{{ID: "event_id_2", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "free"}}, @@ -76,6 +82,15 @@ func TestSyncStatusAll(t *testing.T) { currentStatus: "online", newStatus: "", eventsToStore: nil, + errorLogged: "", + }, + "Remote API error. Error should be logged": { + remoteEvents: nil, + apiError: &remote.ApiError{Code: "403", Message: "Forbidden"}, + activeEvents: []string{eventHash}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, }, } { t.Run(name, func(t *testing.T) { @@ -85,7 +100,7 @@ func TestSyncStatusAll(t *testing.T) { env, client := makeStatusSyncTestEnv(ctrl) deps := env.Dependencies - c, papi, s := client.(*mock_remote.MockClient), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Store.(*mock_store.MockStore) + c, papi, s, logger := client.(*mock_remote.MockClient), deps.PluginAPI.(*mock_plugin_api.MockPluginAPI), deps.Store.(*mock_store.MockStore), deps.Logger.(*mock_bot.MockLogger) s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ MattermostUserID: "user_mm_id", @@ -98,7 +113,7 @@ func TestSyncStatusAll(t *testing.T) { }, nil).Times(1) c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Return([]*remote.ViewCalendarResponse{ - {Events: tc.remoteEvents, RemoteUserID: "user_remote_id"}, + {Events: tc.remoteEvents, RemoteUserID: "user_remote_id", Error: tc.apiError}, }, nil) papi.EXPECT().GetMattermostUserStatusesByIds([]string{"user_mm_id"}).Return([]*model.Status{{Status: tc.currentStatus, UserId: "user_mm_id"}}, nil) @@ -115,8 +130,14 @@ func TestSyncStatusAll(t *testing.T) { s.EXPECT().StoreUserActiveEvents("user_mm_id", tc.eventsToStore).Return(nil).Times(1) } - mscalendar := New(env, "") - res, err := mscalendar.SyncAll() + if tc.apiError == nil { + logger.EXPECT().Warnf(gomock.Any()).Times(0) + } else { + logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", "Forbidden").Times(1) + } + + m := New(env, "") + res, err := m.SyncAll() require.Nil(t, err) require.NotEmpty(t, res) }) @@ -137,7 +158,7 @@ func TestSyncStatusUserConfig(t *testing.T) { c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Times(0) }, }, - "GetConfirmation enabled": { + "UpdateStatus enabled and GetConfirmation enabled": { settings: store.Settings{ UpdateStatus: true, GetConfirmation: true, @@ -145,7 +166,7 @@ func TestSyncStatusUserConfig(t *testing.T) { 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) moment := time.Now().UTC() - busyEvent := &remote.Event{ID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} + 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"}, @@ -183,14 +204,105 @@ func TestSyncStatusUserConfig(t *testing.T) { } } +func TestReminders(t *testing.T) { + for name, tc := range map[string]struct { + remoteEvents []*remote.Event + numReminders int + apiError *remote.ApiError + }{ + "Most common case, no remote events. No reminder.": { + remoteEvents: []*remote.Event{}, + numReminders: 0, + }, + "One remote event, but it is too far in the future.": { + remoteEvents: []*remote.Event{ + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + }, + numReminders: 0, + }, + "One remote event, but it is in the past.": { + remoteEvents: []*remote.Event{ + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(-15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + }, + numReminders: 0, + }, + "One remote event, but it is to soon in the future. Reminder has already occurred.": { + remoteEvents: []*remote.Event{ + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(2*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + }, + numReminders: 0, + }, + "One remote event, and is in the range for the reminder. Reminder should occur.": { + remoteEvents: []*remote.Event{ + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + }, + numReminders: 1, + }, + "Two remote event, and are in the range for the reminder. Two reminders should occur.": { + remoteEvents: []*remote.Event{ + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + }, + numReminders: 2, + }, + "Remote API Error. Error should be logged.": { + remoteEvents: []*remote.Event{}, + numReminders: 0, + // apiError: &remote.Api + }, + } { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + 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) + + loadUser := s.EXPECT().LoadUser("user_mm_id").Return(&store.User{ + MattermostUserID: "user_mm_id", + Remote: &remote.User{ + ID: "user_remote_id", + Mail: "user_email@example.com", + }, + Settings: store.Settings{GetReminders: true}, + }, nil) + c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Return([]*remote.ViewCalendarResponse{ + {Events: tc.remoteEvents, RemoteUserID: "user_remote_id", Error: tc.apiError}, + }, nil) + + if tc.numReminders > 0 { + poster.EXPECT().DM("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 { + poster.EXPECT().DM(gomock.Any(), gomock.Any()).Times(0) + loadUser.Times(1) + } + + if tc.apiError == nil { + logger.EXPECT().Warnf(gomock.Any()).Times(0) + } else { + logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", "Forbidden").Times(1) + } + + m := New(env, "") + res, err := m.SyncAll() + require.Nil(t, err) + require.NotEmpty(t, res) + }) + } +} + func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { s := mock_store.NewMockStore(ctrl) poster := mock_bot.NewMockPoster(ctrl) mockRemote := mock_remote.NewMockRemote(ctrl) mockClient := mock_remote.NewMockClient(ctrl) mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) + logger := mock_bot.NewMockLogger(ctrl) - logger := &bot.NilLogger{} conf := &config.Config{BotUserID: "bot_mm_id"} env := Env{ Config: conf, From 78d13e91b88ca18e054240918d47c66ac771f251 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 23 Apr 2020 21:30:13 -0400 Subject: [PATCH 16/20] Improve error testing --- server/mscalendar/availability_test.go | 145 +++++++++++++------------ 1 file changed, 77 insertions(+), 68 deletions(-) diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 078b7f56..be72d0aa 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -28,69 +28,70 @@ func TestSyncStatusAll(t *testing.T) { busyEvent := &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "busy"} for name, tc := range map[string]struct { - remoteEvents []*remote.Event - apiError *remote.ApiError - activeEvents []string - currentStatus string - newStatus string - eventsToStore []string - errorLogged string + remoteEvents []*remote.Event + apiError *remote.ApiError + activeEvents []string + currentStatus string + newStatus string + eventsToStore []string + shouldLogError bool }{ "Most common case, no events local or remote. No status change.": { - remoteEvents: []*remote.Event{}, - activeEvents: []string{}, - currentStatus: "online", - newStatus: "", - eventsToStore: nil, - errorLogged: "", + remoteEvents: []*remote.Event{}, + activeEvents: []string{}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, + shouldLogError: false, }, "New remote event. Change status to DND.": { - remoteEvents: []*remote.Event{busyEvent}, - activeEvents: []string{}, - currentStatus: "online", - newStatus: "dnd", - eventsToStore: []string{eventHash}, - errorLogged: "", + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{}, + currentStatus: "online", + newStatus: "dnd", + eventsToStore: []string{eventHash}, + shouldLogError: false, }, "Locally stored event is finished. Change status to online.": { - remoteEvents: []*remote.Event{}, - activeEvents: []string{eventHash}, - currentStatus: "dnd", - newStatus: "online", - eventsToStore: []string{}, - errorLogged: "", + remoteEvents: []*remote.Event{}, + activeEvents: []string{eventHash}, + currentStatus: "dnd", + newStatus: "online", + eventsToStore: []string{}, + shouldLogError: false, }, "Locally stored event is still happening. No status change.": { - remoteEvents: []*remote.Event{busyEvent}, - activeEvents: []string{eventHash}, - currentStatus: "dnd", - newStatus: "", - eventsToStore: nil, - errorLogged: "", + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{eventHash}, + currentStatus: "dnd", + newStatus: "", + eventsToStore: nil, + shouldLogError: false, }, "User has manually set themselves to online during event. Locally stored event is still happening, but we will ignore it. No status change.": { - remoteEvents: []*remote.Event{busyEvent}, - activeEvents: []string{eventHash}, - currentStatus: "online", - newStatus: "", - eventsToStore: nil, - errorLogged: "", + remoteEvents: []*remote.Event{busyEvent}, + activeEvents: []string{eventHash}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, + shouldLogError: false, }, "Ignore non-busy event": { - remoteEvents: []*remote.Event{{ID: "event_id_2", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "free"}}, - activeEvents: []string{}, - currentStatus: "online", - newStatus: "", - eventsToStore: nil, - errorLogged: "", + remoteEvents: []*remote.Event{{ID: "event_id_2", Start: remote.NewDateTime(moment, "UTC"), ShowAs: "free"}}, + activeEvents: []string{}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, + shouldLogError: false, }, "Remote API error. Error should be logged": { - remoteEvents: nil, - apiError: &remote.ApiError{Code: "403", Message: "Forbidden"}, - activeEvents: []string{eventHash}, - currentStatus: "online", - newStatus: "", - eventsToStore: nil, + remoteEvents: nil, + activeEvents: []string{eventHash}, + currentStatus: "online", + newStatus: "", + eventsToStore: nil, + apiError: &remote.ApiError{Code: "403", Message: "Forbidden"}, + shouldLogError: true, }, } { t.Run(name, func(t *testing.T) { @@ -130,10 +131,10 @@ func TestSyncStatusAll(t *testing.T) { s.EXPECT().StoreUserActiveEvents("user_mm_id", tc.eventsToStore).Return(nil).Times(1) } - if tc.apiError == nil { - logger.EXPECT().Warnf(gomock.Any()).Times(0) + if tc.shouldLogError { + logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", tc.apiError.Message).Times(1) } else { - logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", "Forbidden").Times(1) + logger.EXPECT().Warnf(gomock.Any()).Times(0) } m := New(env, "") @@ -206,49 +207,57 @@ func TestSyncStatusUserConfig(t *testing.T) { func TestReminders(t *testing.T) { for name, tc := range map[string]struct { - remoteEvents []*remote.Event - numReminders int - apiError *remote.ApiError + remoteEvents []*remote.Event + numReminders int + apiError *remote.ApiError + shouldLogError bool }{ "Most common case, no remote events. No reminder.": { - remoteEvents: []*remote.Event{}, - numReminders: 0, + remoteEvents: []*remote.Event{}, + numReminders: 0, + shouldLogError: false, }, "One remote event, but it is too far in the future.": { remoteEvents: []*remote.Event{ &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, - numReminders: 0, + numReminders: 0, + shouldLogError: false, }, "One remote event, but it is in the past.": { remoteEvents: []*remote.Event{ &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(-15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, - numReminders: 0, + numReminders: 0, + shouldLogError: false, }, "One remote event, but it is to soon in the future. Reminder has already occurred.": { remoteEvents: []*remote.Event{ &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(2*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, - numReminders: 0, + numReminders: 0, + shouldLogError: false, }, "One remote event, and is in the range for the reminder. Reminder should occur.": { remoteEvents: []*remote.Event{ &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, - numReminders: 1, + numReminders: 1, + shouldLogError: false, }, "Two remote event, and are in the range for the reminder. Two reminders should occur.": { remoteEvents: []*remote.Event{ &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, - numReminders: 2, + numReminders: 2, + shouldLogError: false, }, "Remote API Error. Error should be logged.": { - remoteEvents: []*remote.Event{}, - numReminders: 0, - // apiError: &remote.Api + remoteEvents: []*remote.Event{}, + numReminders: 0, + apiError: &remote.ApiError{Code: "403", Message: "Forbidden"}, + shouldLogError: true, }, } { t.Run(name, func(t *testing.T) { @@ -281,10 +290,10 @@ func TestReminders(t *testing.T) { loadUser.Times(1) } - if tc.apiError == nil { - logger.EXPECT().Warnf(gomock.Any()).Times(0) + if tc.shouldLogError { + logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", tc.apiError.Message).Times(1) } else { - logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", "Forbidden").Times(1) + logger.EXPECT().Warnf(gomock.Any()).Times(0) } m := New(env, "") From e268fcfa8a93c05a9544b65dee8b6aaeac18a275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 24 Apr 2020 14:10:08 +0200 Subject: [PATCH 17/20] Name changes refactoring, and changes on how upcoming events and calendar elements are shown --- server/jobs/status_sync_job.go | 2 +- server/mscalendar/availability.go | 64 +++++++++---------- server/mscalendar/availability_test.go | 14 ++-- .../mock_mscalendar/mock_mscalendar.go | 12 ++-- server/mscalendar/settings.go | 6 +- server/mscalendar/views/calendar.go | 52 +++++++++------ server/store/setting_store.go | 16 ++--- server/store/user_store.go | 2 +- server/utils/settingspanel/handler.go | 9 +-- 9 files changed, 95 insertions(+), 82 deletions(-) diff --git a/server/jobs/status_sync_job.go b/server/jobs/status_sync_job.go index 9e917e7f..62110bb7 100644 --- a/server/jobs/status_sync_job.go +++ b/server/jobs/status_sync_job.go @@ -20,7 +20,7 @@ func NewStatusSyncJob() RegisteredJob { } } -// runStatusSyncJob synchronizes all users' statuses between mscalendar and Mattermost. +// runSyncJob synchronizes all users' statuses between mscalendar and Mattermost. func runSyncJob(env mscalendar.Env) { env.Logger.Debugf("User status sync job beginning") diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index b19be2bd..a75da083 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -17,14 +17,14 @@ import ( ) const ( - availabilityTimeWindowSize = 10 * time.Minute + calendarViewTimeWindowSize = 10 * time.Minute StatusSyncJobInterval = 5 * time.Minute upcomingEventNotificationTime = 10 * time.Minute upcomingEventNotificationWindow = (StatusSyncJobInterval * 9) / 10 //90% of the interval ) type Availability interface { - GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) + GetCalendarViews(users []*store.User) ([]*remote.ViewCalendarResponse, error) Sync(mattermostUserID string) (string, error) SyncAll() (string, error) } @@ -74,7 +74,7 @@ func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, error) { if err != nil { return "", err } - if user.Settings.UpdateStatus || user.Settings.GetReminders { + if user.Settings.UpdateStatus || user.Settings.ReceiveReminders { users = append(users, user) } } @@ -82,16 +82,16 @@ func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, error) { return "No users need to be synced", nil } - schedules, err := m.GetAvailabilities(users) + calendarViews, err := m.GetCalendarViews(users) if err != nil { return "", err } - if len(schedules) == 0 { - return "No schedule info found", nil + if len(calendarViews) == 0 { + return "No calendar views found", nil } - m.deliverReminders(users, schedules) - out, err := m.setUserStatuses(users, schedules) + m.deliverReminders(users, calendarViews) + out, err := m.setUserStatuses(users, calendarViews) if err != nil { return "", err } @@ -99,10 +99,10 @@ func (m *mscalendar) syncUsers(userIndex store.UserIndex) (string, error) { return out, nil } -func (m *mscalendar) deliverReminders(users []*store.User, schedules []*remote.ViewCalendarResponse) { +func (m *mscalendar) deliverReminders(users []*store.User, calendarViews []*remote.ViewCalendarResponse) { toNotify := []*store.User{} for _, u := range users { - if u.Settings.GetReminders { + if u.Settings.ReceiveReminders { toNotify = append(toNotify, u) } } @@ -115,22 +115,22 @@ func (m *mscalendar) deliverReminders(users []*store.User, schedules []*remote.V usersByRemoteID[u.Remote.ID] = u } - for _, s := range schedules { - user, ok := usersByRemoteID[s.RemoteUserID] + for _, view := range calendarViews { + user, ok := usersByRemoteID[view.RemoteUserID] if !ok { continue } - if s.Error != nil { - m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) + if view.Error != nil { + m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, view.Error.Message) continue } - mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID - m.notifyUpcomingEvent(mattermostUserID, s.Events) + mattermostUserID := usersByRemoteID[view.RemoteUserID].MattermostUserID + m.notifyUpcomingEvents(mattermostUserID, view.Events) } } -func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.ViewCalendarResponse) (string, error) { +func (m *mscalendar) setUserStatuses(users []*store.User, calendarViews []*remote.ViewCalendarResponse) (string, error) { toUpdate := []*store.User{} for _, u := range users { if u.Settings.UpdateStatus { @@ -138,7 +138,7 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi } } if len(toUpdate) == 0 { - return "No users want their status", nil + return "No users want their status updated", nil } mattermostUserIDs := []string{} @@ -158,24 +158,24 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi } var res string - for _, s := range schedules { - user, ok := usersByRemoteID[s.RemoteUserID] + for _, view := range calendarViews { + user, ok := usersByRemoteID[view.RemoteUserID] if !ok { continue } - if s.Error != nil { - m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, s.Error.Message) + if view.Error != nil { + m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, view.Error.Message) continue } - mattermostUserID := usersByRemoteID[s.RemoteUserID].MattermostUserID + mattermostUserID := usersByRemoteID[view.RemoteUserID].MattermostUserID status, ok := statusMap[mattermostUserID] if !ok { continue } var err error - res, err = m.setStatusFromAvailability(user, status, s) + res, err = m.setStatusFromCalendarView(user, status, view) if err != nil { m.Logger.Warnf("Error setting user %s status. %s", user.Remote.Mail, err.Error()) } @@ -184,10 +184,10 @@ func (m *mscalendar) setUserStatuses(users []*store.User, schedules []*remote.Vi return res, nil } - return utils.JSONBlock(schedules), nil + return utils.JSONBlock(calendarViews), nil } -func (m *mscalendar) setStatusFromAvailability(user *store.User, currentStatus string, res *remote.ViewCalendarResponse) (string, error) { +func (m *mscalendar) setStatusFromCalendarView(user *store.User, currentStatus string, res *remote.ViewCalendarResponse) (string, error) { events := filterBusyEvents(res.Events) if len(user.ActiveEvents) == 0 && len(events) == 0 { @@ -293,14 +293,14 @@ func (m *mscalendar) setStatusOrAskUser(user *store.User, status string, events return nil } -func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalendarResponse, error) { +func (m *mscalendar) GetCalendarViews(users []*store.User) ([]*remote.ViewCalendarResponse, error) { err := m.Filter(withClient) if err != nil { return nil, err } start := time.Now().UTC() - end := time.Now().UTC().Add(availabilityTimeWindowSize) + end := time.Now().UTC().Add(calendarViewTimeWindowSize) params := []*remote.ViewCalendarParams{} for _, u := range users { @@ -314,7 +314,7 @@ func (m *mscalendar) GetAvailabilities(users []*store.User) ([]*remote.ViewCalen return m.client.DoBatchViewCalendarRequests(params) } -func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remote.Event) { +func (m *mscalendar) notifyUpcomingEvents(mattermostUserID string, events []*remote.Event) { var timezone string for _, event := range events { if event.IsCancelled { @@ -329,19 +329,19 @@ func (m *mscalendar) notifyUpcomingEvent(mattermostUserID string, events []*remo if timezone == "" { timezone, err = m.GetTimezoneByID(mattermostUserID) if err != nil { - m.Logger.Warnf("notifyUpcomingEvent error getting timezone, err=%s", err.Error()) + m.Logger.Warnf("notifyUpcomingEvents error getting timezone, err=%s", err.Error()) return } } - message, err := views.RenderScheduleItem(event, timezone) + message, err := views.RenderUpcomingEvent(event, timezone) if err != nil { m.Logger.Warnf("notifyUpcomingEvent error rendering schedule item, err=", err.Error()) continue } err = m.Poster.DM(mattermostUserID, message) if err != nil { - m.Logger.Warnf("notifyUpcomingEvent error creating DM, err=", err.Error()) + m.Logger.Warnf("notifyUpcomingEvents error creating DM, err=", err.Error()) continue } } diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index be72d0aa..167aa93f 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -219,36 +219,36 @@ func TestReminders(t *testing.T) { }, "One remote event, but it is too far in the future.": { remoteEvents: []*remote.Event{ - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, numReminders: 0, shouldLogError: false, }, "One remote event, but it is in the past.": { remoteEvents: []*remote.Event{ - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(-15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(-15*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, numReminders: 0, shouldLogError: false, }, "One remote event, but it is to soon in the future. Reminder has already occurred.": { remoteEvents: []*remote.Event{ - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(2*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(2*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, numReminders: 0, shouldLogError: false, }, "One remote event, and is in the range for the reminder. Reminder should occur.": { remoteEvents: []*remote.Event{ - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, numReminders: 1, shouldLogError: false, }, "Two remote event, and are in the range for the reminder. Two reminders should occur.": { remoteEvents: []*remote.Event{ - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, - &remote.Event{ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, + {ICalUID: "event_id", Start: remote.NewDateTime(time.Now().Add(7*time.Minute).UTC(), "UTC"), End: remote.NewDateTime(time.Now().Add(45*time.Minute).UTC(), "UTC")}, }, numReminders: 2, shouldLogError: false, @@ -275,7 +275,7 @@ func TestReminders(t *testing.T) { ID: "user_remote_id", Mail: "user_email@example.com", }, - Settings: store.Settings{GetReminders: true}, + Settings: store.Settings{ReceiveReminders: true}, }, nil) c.EXPECT().DoBatchViewCalendarRequests(gomock.Any()).Return([]*remote.ViewCalendarResponse{ {Events: tc.remoteEvents, RemoteUserID: "user_remote_id", Error: tc.apiError}, diff --git a/server/mscalendar/mock_mscalendar/mock_mscalendar.go b/server/mscalendar/mock_mscalendar/mock_mscalendar.go index d723d85e..e1a211a3 100644 --- a/server/mscalendar/mock_mscalendar/mock_mscalendar.go +++ b/server/mscalendar/mock_mscalendar/mock_mscalendar.go @@ -206,19 +206,19 @@ func (mr *MockMSCalendarMockRecorder) GetActingUser() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActingUser", reflect.TypeOf((*MockMSCalendar)(nil).GetActingUser)) } -// GetAvailabilities mocks base method -func (m *MockMSCalendar) GetAvailabilities(arg0 []*store.User) ([]*remote.ViewCalendarResponse, error) { +// GetCalendarViews mocks base method +func (m *MockMSCalendar) GetCalendarViews(arg0 []*store.User) ([]*remote.ViewCalendarResponse, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAvailabilities", arg0) + ret := m.ctrl.Call(m, "GetCalendarViews", arg0) ret0, _ := ret[0].([]*remote.ViewCalendarResponse) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetAvailabilities indicates an expected call of GetAvailabilities -func (mr *MockMSCalendarMockRecorder) GetAvailabilities(arg0 interface{}) *gomock.Call { +// GetCalendarViews indicates an expected call of GetCalendarViews +func (mr *MockMSCalendarMockRecorder) GetCalendarViews(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAvailabilities", reflect.TypeOf((*MockMSCalendar)(nil).GetAvailabilities), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCalendarViews", reflect.TypeOf((*MockMSCalendar)(nil).GetCalendarViews), arg0) } // GetCalendars mocks base method diff --git a/server/mscalendar/settings.go b/server/mscalendar/settings.go index a15395ef..2777974c 100644 --- a/server/mscalendar/settings.go +++ b/server/mscalendar/settings.go @@ -36,9 +36,9 @@ func NewSettingsPanel(bot bot.Bot, panelStore settingspanel.PanelStore, settingS settingStore, )) settings = append(settings, settingspanel.NewBoolSetting( - store.GetRemindersSettingID, - "Get Reminders", - "Do you want to get reminders for upcoming events?", + store.ReceiveRemindersSettingID, + "Receive Reminders", + "Do you want to receive reminders for upcoming events?", "", settingStore, )) diff --git a/server/mscalendar/views/calendar.go b/server/mscalendar/views/calendar.go index db04eeae..3569f26c 100644 --- a/server/mscalendar/views/calendar.go +++ b/server/mscalendar/views/calendar.go @@ -2,6 +2,7 @@ package views import ( "fmt" + "net/url" "sort" "time" @@ -20,22 +21,42 @@ func RenderCalendarView(events []*remote.Event, timeZone string) (string, error) } } - resp := "Times are shown in " + events[0].Start.TimeZone + "\n" + resp := "Times are shown in " + events[0].Start.TimeZone for _, group := range groupEventsByDate(events) { - resp += group[0].Start.Time().Format("Monday January 02") + "\n" + resp += "\n" + group[0].Start.Time().Format("Monday January 02") + "\n\n" + resp += renderTableHeader() for _, e := range group { - resp += fmt.Sprintf("* %s\n", renderEvent(e)) + eventString, err := renderEvent(e, true) + if err != nil { + return "", err + } + resp += fmt.Sprintf("\n%s", eventString) } } return resp, nil } -func renderEvent(event *remote.Event) string { +func renderTableHeader() string { + return `| Time | Subject | +| :--: | :-- |` +} + +func renderEvent(event *remote.Event, asRow bool) (string, error) { start := event.Start.Time().Format(time.Kitchen) end := event.End.Time().Format(time.Kitchen) - return fmt.Sprintf("%s - %s `%s`", start, end, event.Subject) + format := "(%s - %s) [%s](%s)" + if asRow { + format = "| %s - %s | [%s](%s) |" + } + + link, err := url.QueryUnescape(event.Weblink) + if err != nil { + return "", err + } + + return fmt.Sprintf(format, start, end, event.Subject, link), nil } func groupEventsByDate(events []*remote.Event) [][]*remote.Event { @@ -65,21 +86,12 @@ func groupEventsByDate(events []*remote.Event) [][]*remote.Event { return result } -func RenderScheduleItem(event *remote.Event, timezone string) (string, error) { - message := "You have an upcoming event:" - start := event.Start.In(timezone).Time() - end := event.End.In(timezone).Time() - - message += fmt.Sprintf("\n%s-%s (%s)", start.Format(time.Kitchen), end.Format(time.Kitchen), timezone) - if event.Subject == "" { - message += "\nSubject: unknown" - } else { - message += fmt.Sprintf("\nSubject: %s", event.Subject) - } - - if event.Location != nil && event.Location.DisplayName != "" { - message += fmt.Sprintf("\nLocation: %s", event.Location.DisplayName) +func RenderUpcomingEvent(event *remote.Event, timezone string) (string, error) { + message := "You have an upcoming event:\n" + eventString, err := renderEvent(event, false) + if err != nil { + return "", err } - return message, nil + return message + eventString, nil } diff --git a/server/store/setting_store.go b/server/store/setting_store.go index 514e71be..075685ca 100644 --- a/server/store/setting_store.go +++ b/server/store/setting_store.go @@ -6,10 +6,10 @@ import ( ) const ( - UpdateStatusSettingID = "update_status" - GetConfirmationSettingID = "get_confirmation" - GetRemindersSettingID = "get_reminders" - DailySummarySettingID = "summary_setting" + UpdateStatusSettingID = "update_status" + GetConfirmationSettingID = "get_confirmation" + ReceiveRemindersSettingID = "get_reminders" + DailySummarySettingID = "summary_setting" ) func (s *pluginStore) SetSetting(userID, settingID string, value interface{}) error { @@ -31,12 +31,12 @@ 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.GetConfirmation = storableValue - case GetRemindersSettingID: + case ReceiveRemindersSettingID: storableValue, ok := value.(bool) if !ok { return fmt.Errorf("cannot read value %v for setting %s (expecting bool)", value, settingID) } - user.Settings.GetReminders = storableValue + user.Settings.ReceiveReminders = storableValue case DailySummarySettingID: s.updateDailySummarySettingForUser(userID, value) default: @@ -62,8 +62,8 @@ func (s *pluginStore) GetSetting(userID, settingID string) (interface{}, error) return user.Settings.UpdateStatus, nil case GetConfirmationSettingID: return user.Settings.GetConfirmation, nil - case GetRemindersSettingID: - return user.Settings.GetReminders, nil + case ReceiveRemindersSettingID: + return user.Settings.ReceiveReminders, nil case DailySummarySettingID: return s.LoadDailySummaryUserSettings(userID) default: diff --git a/server/store/user_store.go b/server/store/user_store.go index 1e9ee6d1..8254c674 100644 --- a/server/store/user_store.go +++ b/server/store/user_store.go @@ -48,7 +48,7 @@ type Settings struct { EventSubscriptionID string UpdateStatus bool GetConfirmation bool - GetReminders bool + ReceiveReminders bool } func (settings Settings) String() string { diff --git a/server/utils/settingspanel/handler.go b/server/utils/settingspanel/handler.go index 3f421c07..bfc370b8 100644 --- a/server/utils/settingspanel/handler.go +++ b/server/utils/settingspanel/handler.go @@ -3,6 +3,7 @@ package settingspanel import ( "net/http" + "github.com/mattermost/mattermost-plugin-mscalendar/server/utils" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils/httputils" "github.com/mattermost/mattermost-server/v5/model" ) @@ -29,19 +30,19 @@ func Init(h *httputils.Handler, panel Panel) { func (sh *handler) handleAction(w http.ResponseWriter, r *http.Request) { mattermostUserID := r.Header.Get("Mattermost-User-ID") if mattermostUserID == "" { - http.Error(w, "Not authorized", http.StatusUnauthorized) + utils.SlackAttachmentError(w, "Error: not authorized") return } request := model.PostActionIntegrationRequestFromJson(r.Body) if request == nil { - http.Error(w, "invalid request", http.StatusBadRequest) + utils.SlackAttachmentError(w, "Error: invalid request") return } id, ok := request.Context[ContextIDKey] if !ok { - http.Error(w, "invalid request", http.StatusBadRequest) + utils.SlackAttachmentError(w, "Error: missing setting id") return } @@ -49,7 +50,7 @@ func (sh *handler) handleAction(w http.ResponseWriter, r *http.Request) { if !ok { value, ok = request.Context[ContextOptionValueKey] if !ok { - http.Error(w, "valid key not found", http.StatusBadRequest) + utils.SlackAttachmentError(w, "Error: valid key not found") return } } From fd149c7e6390e6ddf6b1ce0e941bbb283bf3feb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 24 Apr 2020 14:17:54 +0200 Subject: [PATCH 18/20] Fix test --- server/mscalendar/daily_summary_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 846849a6..3c398cd2 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -135,7 +135,10 @@ func TestProcessAllDailySummary(t *testing.T) { mockPoster.EXPECT().DM("user1_mm_id", "You have no upcoming events.").Return(nil).Times(1), mockPoster.EXPECT().DM("user2_mm_id", `Times are shown in Pacific Standard Time Wednesday February 12 -* 9:00AM - 11:00AM `+"`The subject`\n").Return(nil).Times(1), + +| Time | Subject | +| :--: | :-- | +| 9:00AM - 11:00AM | [The subject]() |`).Return(nil).Times(1), ) s.EXPECT().ModifyDailySummaryIndex(gomock.Any()).Return(nil) From 5a698c8bc08e13c8dd4301ff006e8c429d8be466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 24 Apr 2020 20:45:57 +0200 Subject: [PATCH 19/20] Remove unneeded botUser references --- server/command/availability.go | 9 +++++++ server/command/connect_test.go | 1 - server/command/disconnect_test.go | 1 - server/config/config.go | 1 - server/jobs/job_manager_test.go | 3 --- server/mscalendar/availability.go | 10 +------- server/mscalendar/availability_test.go | 14 +++++------ server/mscalendar/daily_summary.go | 8 ------- server/mscalendar/daily_summary_test.go | 32 ++++--------------------- server/mscalendar/filters.go | 11 +-------- server/mscalendar/mscalendar.go | 16 +++++-------- server/mscalendar/notification_test.go | 2 +- server/mscalendar/oauth2_test.go | 12 ++++------ server/mscalendar/user.go | 14 ++++++++++- server/plugin/plugin.go | 17 ------------- 15 files changed, 47 insertions(+), 104 deletions(-) diff --git a/server/command/availability.go b/server/command/availability.go index 0fe11438..799587b4 100644 --- a/server/command/availability.go +++ b/server/command/availability.go @@ -4,6 +4,15 @@ package command func (c *Command) availability(parameters ...string) (string, error) { + authorized, err := c.MSCalendar.IsAuthorizedAdmin(c.Args.UserId) + if err != nil { + return "", err + } + + if !authorized { + return "Not authorized", nil + } + switch { case len(parameters) == 0: resString, err := c.MSCalendar.Sync(c.Args.UserId) diff --git a/server/command/connect_test.go b/server/command/connect_test.go index 6e0b85e8..dd95d99f 100644 --- a/server/command/connect_test.go +++ b/server/command/connect_test.go @@ -51,7 +51,6 @@ func TestConnect(t *testing.T) { conf := &config.Config{ PluginURL: "http://localhost", - BotUserID: "bot_user_id", } mscal := mock_mscalendar.NewMockMSCalendar(ctrl) diff --git a/server/command/disconnect_test.go b/server/command/disconnect_test.go index d903757c..9151eaef 100644 --- a/server/command/disconnect_test.go +++ b/server/command/disconnect_test.go @@ -51,7 +51,6 @@ func TestDisconnect(t *testing.T) { conf := &config.Config{ PluginURL: "http://localhost", - BotUserID: "bot_user_id", } mscal := mock_mscalendar.NewMockMSCalendar(ctrl) diff --git a/server/config/config.go b/server/config/config.go index e665c3d9..b858908c 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -20,7 +20,6 @@ type StoredConfig struct { type Config struct { StoredConfig - BotUserID string BuildDate string BuildHash string BuildHashShort string diff --git a/server/jobs/job_manager_test.go b/server/jobs/job_manager_test.go index 51bb0a93..8858e5bf 100644 --- a/server/jobs/job_manager_test.go +++ b/server/jobs/job_manager_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-plugin-api/cluster" - "github.com/mattermost/mattermost-plugin-mscalendar/server/config" "github.com/mattermost/mattermost-plugin-mscalendar/server/jobs/mock_cluster" "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar" "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/mock_plugin_api" @@ -29,9 +28,7 @@ func newTestEnv(ctrl *gomock.Controller) mscalendar.Env { mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) logger := &bot.NilLogger{} - conf := &config.Config{BotUserID: "bot_mm_id"} return mscalendar.Env{ - Config: conf, Dependencies: &mscalendar.Dependencies{ Store: s, Logger: logger, diff --git a/server/mscalendar/availability.go b/server/mscalendar/availability.go index a75da083..64bc2225 100644 --- a/server/mscalendar/availability.go +++ b/server/mscalendar/availability.go @@ -13,7 +13,6 @@ import ( "github.com/mattermost/mattermost-plugin-mscalendar/server/store" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils" "github.com/mattermost/mattermost-server/v5/model" - "github.com/pkg/errors" ) const ( @@ -39,14 +38,7 @@ func (m *mscalendar) Sync(mattermostUserID string) (string, error) { } func (m *mscalendar) SyncAll() (string, error) { - isAdmin, err := m.IsAuthorizedAdmin(m.actingUser.MattermostUserID) - if err != nil { - return "", err - } - if !isAdmin { - return "", errors.Errorf("Non-admin user attempting SyncAll %s", m.actingUser.MattermostUserID) - } - err = m.Filter(withSuperuserClient) + err := m.Filter(withSuperuserClient) if err != nil { return "", err } diff --git a/server/mscalendar/availability_test.go b/server/mscalendar/availability_test.go index 167aa93f..da95e2fb 100644 --- a/server/mscalendar/availability_test.go +++ b/server/mscalendar/availability_test.go @@ -312,16 +312,14 @@ func makeStatusSyncTestEnv(ctrl *gomock.Controller) (Env, remote.Client) { mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) logger := mock_bot.NewMockLogger(ctrl) - conf := &config.Config{BotUserID: "bot_mm_id"} env := Env{ - Config: conf, + Config: &config.Config{}, Dependencies: &Dependencies{ - Store: s, - Logger: logger, - Poster: poster, - Remote: mockRemote, - PluginAPI: mockPluginAPI, - IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return true, nil }, + Store: s, + Logger: logger, + Poster: poster, + Remote: mockRemote, + PluginAPI: mockPluginAPI, }, } diff --git a/server/mscalendar/daily_summary.go b/server/mscalendar/daily_summary.go index abce458f..b4b31b6c 100644 --- a/server/mscalendar/daily_summary.go +++ b/server/mscalendar/daily_summary.go @@ -102,14 +102,6 @@ func (m *mscalendar) SetDailySummaryEnabled(user *User, enable bool) (*store.Dai } func (m *mscalendar) ProcessAllDailySummary(now time.Time) error { - isAdmin, err := m.IsAuthorizedAdmin(m.actingUser.MattermostUserID) - if err != nil { - return err - } - if !isAdmin { - return errors.Errorf("Non-admin user attempting ProcessAllDailySummary %s", m.actingUser.MattermostUserID) - } - dsumIndex, err := m.Store.LoadDailySummaryIndex() if err != nil { return err diff --git a/server/mscalendar/daily_summary_test.go b/server/mscalendar/daily_summary_test.go index 3c398cd2..9bce0c1d 100644 --- a/server/mscalendar/daily_summary_test.go +++ b/server/mscalendar/daily_summary_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/mattermost/mattermost-plugin-mscalendar/server/config" "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/mock_plugin_api" "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" "github.com/mattermost/mattermost-plugin-mscalendar/server/remote/mock_remote" @@ -23,24 +22,6 @@ func TestProcessAllDailySummary(t *testing.T) { err string runAssertions func(deps *Dependencies, client remote.Client) }{ - { - name: "Error fetching user admin", - err: "admin store error", - runAssertions: func(deps *Dependencies, client remote.Client) { - deps.IsAuthorizedAdmin = func(string) (bool, error) { - return false, errors.New("admin store error") - } - }, - }, - { - name: "User is not admin", - err: "Non-admin user attempting ProcessAllDailySummary bot_mm_id", - runAssertions: func(deps *Dependencies, client remote.Client) { - deps.IsAuthorizedAdmin = func(string) (bool, error) { - return false, nil - } - }, - }, { name: "Error fetching index", err: "index store error", @@ -159,16 +140,13 @@ Wednesday February 12 mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) logger := mock_bot.NewMockLogger(ctrl) - conf := &config.Config{BotUserID: "bot_mm_id"} env := Env{ - Config: conf, Dependencies: &Dependencies{ - Store: s, - Logger: logger, - Poster: poster, - Remote: mockRemote, - PluginAPI: mockPluginAPI, - IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return true, nil }, + Store: s, + Logger: logger, + Poster: poster, + Remote: mockRemote, + PluginAPI: mockPluginAPI, }, } diff --git a/server/mscalendar/filters.go b/server/mscalendar/filters.go index 9256dd02..be79136d 100644 --- a/server/mscalendar/filters.go +++ b/server/mscalendar/filters.go @@ -3,8 +3,6 @@ package mscalendar -import "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" - type filterf func(*mscalendar) error func (m *mscalendar) Filter(filters ...filterf) error { @@ -44,14 +42,7 @@ func withClient(m *mscalendar) error { return nil } - var client remote.Client - var err error - if m.actingUser.MattermostUserID == m.Config.BotUserID { - client, err = m.MakeSuperuserClient() - - } else { - client, err = m.MakeClient() - } + client, err := m.MakeClient() if err != nil { return err diff --git a/server/mscalendar/mscalendar.go b/server/mscalendar/mscalendar.go index 2d9ebc65..06292a26 100644 --- a/server/mscalendar/mscalendar.go +++ b/server/mscalendar/mscalendar.go @@ -25,13 +25,12 @@ type MSCalendar interface { // Dependencies contains all API dependencies type Dependencies struct { - Logger bot.Logger - PluginAPI PluginAPI - Poster bot.Poster - Remote remote.Remote - Store store.Store - SettingsPanel settingspanel.Panel - IsAuthorizedAdmin func(string) (bool, error) + Logger bot.Logger + PluginAPI PluginAPI + Poster bot.Poster + Remote remote.Remote + Store store.Store + SettingsPanel settingspanel.Panel } type PluginAPI interface { @@ -56,9 +55,6 @@ type mscalendar struct { } func New(env Env, actingMattermostUserID string) MSCalendar { - if actingMattermostUserID == "" { - actingMattermostUserID = env.Config.BotUserID - } return &mscalendar{ Env: env, actingUser: NewUser(actingMattermostUserID), diff --git a/server/mscalendar/notification_test.go b/server/mscalendar/notification_test.go index f8dca521..89505160 100644 --- a/server/mscalendar/notification_test.go +++ b/server/mscalendar/notification_test.go @@ -130,7 +130,7 @@ func TestProcessNotification(t *testing.T) { mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl) mockClient := mock_remote.NewMockClient(ctrl) - conf := &config.Config{BotUserID: "bot_mm_id", PluginVersion: "x.x.x"} + conf := &config.Config{PluginVersion: "x.x.x"} env := Env{ Config: conf, Dependencies: &Dependencies{ diff --git a/server/mscalendar/oauth2_test.go b/server/mscalendar/oauth2_test.go index d0b59db8..5bfcd05c 100644 --- a/server/mscalendar/oauth2_test.go +++ b/server/mscalendar/oauth2_test.go @@ -363,18 +363,16 @@ func newOAuth2TestApp(ctrl *gomock.Controller) (oauth2connect.App, Env) { OAuth2ClientSecret: "fakeclientsecret", }, PluginURL: "http://localhost", - BotUserID: "bot-user-id", } env := Env{ Config: conf, Dependencies: &Dependencies{ - Store: mock_store.NewMockStore(ctrl), - Logger: &bot.NilLogger{}, - Poster: mock_bot.NewMockPoster(ctrl), - Remote: remote.Makers[msgraph.Kind](conf, &bot.NilLogger{}), - PluginAPI: mock_plugin_api.NewMockPluginAPI(ctrl), - IsAuthorizedAdmin: func(mattermostUserID string) (bool, error) { return false, nil }, + Store: mock_store.NewMockStore(ctrl), + Logger: &bot.NilLogger{}, + Poster: mock_bot.NewMockPoster(ctrl), + Remote: remote.Makers[msgraph.Kind](conf, &bot.NilLogger{}), + PluginAPI: mock_plugin_api.NewMockPluginAPI(ctrl), }, } diff --git a/server/mscalendar/user.go b/server/mscalendar/user.go index 2f1c3f79..483690f7 100644 --- a/server/mscalendar/user.go +++ b/server/mscalendar/user.go @@ -5,6 +5,7 @@ package mscalendar import ( "fmt" + "strings" "github.com/mattermost/mattermost-server/v5/model" "github.com/pkg/errors" @@ -168,7 +169,18 @@ func (m *mscalendar) GetRemoteUser(mattermostUserID string) (*remote.User, error } func (m *mscalendar) IsAuthorizedAdmin(mattermostUserID string) (bool, error) { - return m.Dependencies.IsAuthorizedAdmin(mattermostUserID) + for _, userID := range strings.Split(m.AdminUserIDs, ",") { + if userID == mattermostUserID { + return true, nil + } + } + + ok, err := m.PluginAPI.IsSysAdmin(mattermostUserID) + if err != nil { + return false, err + } + + return ok, nil } func (m *mscalendar) GetUserSettings(user *User) (*store.Settings, error) { diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index f6a8241e..84aec002 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -127,11 +127,9 @@ func (p *Plugin) OnConfigurationChange() (err error) { e.Dependencies.Remote = remote.Makers[msgraph.Kind](e.Config, e.Logger) e.bot = e.bot.WithConfig(stored.BotConfig) - e.Config.BotUserID = e.bot.MattermostUserID() e.Dependencies.Logger = e.bot e.Dependencies.Poster = e.bot e.Dependencies.Store = store.NewPluginStore(p.API, e.bot) - e.Dependencies.IsAuthorizedAdmin = p.IsAuthorizedAdmin e.Dependencies.SettingsPanel = mscalendar.NewSettingsPanel( e.bot, e.Dependencies.Store, @@ -216,21 +214,6 @@ func (p *Plugin) ServeHTTP(pc *plugin.Context, w http.ResponseWriter, req *http. env.httpHandler.ServeHTTP(w, req) } -func (p *Plugin) IsAuthorizedAdmin(mattermostUserID string) (bool, error) { - env := p.getEnv() - if mattermostUserID == env.BotUserID { - return true, nil - } - - for _, userID := range strings.Split(env.Config.AdminUserIDs, ",") { - if userID == mattermostUserID { - return true, nil - } - } - - return env.PluginAPI.IsSysAdmin(mattermostUserID) -} - func (p *Plugin) getEnv() Env { p.envLock.RLock() defer p.envLock.RUnlock() From 7b2a6fe8e6dacdae5ae9768d59e4b138945c9dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 30 Apr 2020 12:55:35 +0200 Subject: [PATCH 20/20] Handle mistaken string when the availability handles an ongoing event and add event information to post action result. --- server/api/post_action.go | 51 +++++++++++++++++-- .../views/status_change_notification.go | 45 ++++++++++++++-- 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/server/api/post_action.go b/server/api/post_action.go index 0445012e..841e029e 100644 --- a/server/api/post_action.go +++ b/server/api/post_action.go @@ -4,14 +4,18 @@ package api import ( + "encoding/json" + "errors" "fmt" "net/http" "strings" + "time" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-plugin-mscalendar/server/config" "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar" + "github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views" "github.com/mattermost/mattermost-plugin-mscalendar/server/utils" ) @@ -138,7 +142,7 @@ func prettyOption(option string) string { func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.Request) { mattermostUserID := req.Header.Get("Mattermost-User-ID") if mattermostUserID == "" { - http.Error(w, "Not authorized", http.StatusUnauthorized) + utils.SlackAttachmentError(w, "Not authorized.") return } @@ -147,13 +151,13 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R request := model.PostActionIntegrationRequestFromJson(req.Body) if request == nil { - http.Error(w, "invalid request", http.StatusBadRequest) + utils.SlackAttachmentError(w, "Invalid request.") return } value, ok := request.Context["value"].(bool) if !ok { - http.Error(w, `No recognizable value for property "value"`, http.StatusBadRequest) + utils.SlackAttachmentError(w, `No recognizable value for property "value".`) return } @@ -161,7 +165,7 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R if value { changeTo, ok := request.Context["change_to"] if !ok { - http.Error(w, "No state to change", http.StatusBadRequest) + utils.SlackAttachmentError(w, "No state to change to.") return } stringChangeTo := changeTo.(string) @@ -175,6 +179,16 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R returnText = fmt.Sprintf("The status has been changed to %s.", stringPrettyChangeTo) } + eventInfo, err := getEventInfo(request.Context) + if err != nil { + utils.SlackAttachmentError(w, err.Error()) + return + } + + if eventInfo != "" { + returnText = eventInfo + "\n" + returnText + } + sa := &model.SlackAttachment{ Title: "Status Change", Text: returnText, @@ -186,3 +200,32 @@ func (api *api) postActionConfirmStatusChange(w http.ResponseWriter, req *http.R w.Header().Set("Content-Type", "application/json") w.Write(response.ToJson()) } + +func getEventInfo(ctx map[string]interface{}) (string, error) { + hasEvent, ok := ctx["hasEvent"].(bool) + if !ok { + return "", errors.New("Cannot check whether there is an event attached.") + } + if !hasEvent { + return "", nil + } + + subject, ok := ctx["subject"].(string) + if !ok { + return "", errors.New("Cannot find the event subject.") + } + + weblink, ok := ctx["weblink"].(string) + if !ok { + return "", errors.New("Cannot find the event weblink.") + } + + marshalledStartTime, ok := ctx["startTime"].(string) + if !ok { + return "", errors.New("Cannot find the event start time.") + } + var startTime time.Time + json.Unmarshal([]byte(marshalledStartTime), &startTime) + + return views.RenderEventWillStartLine(subject, weblink, startTime), nil +} diff --git a/server/mscalendar/views/status_change_notification.go b/server/mscalendar/views/status_change_notification.go index 51ba0e91..379faa9f 100644 --- a/server/mscalendar/views/status_change_notification.go +++ b/server/mscalendar/views/status_change_notification.go @@ -1,7 +1,9 @@ package views import ( + "encoding/json" "fmt" + "net/url" "time" "github.com/mattermost/mattermost-plugin-mscalendar/server/remote" @@ -21,18 +23,36 @@ func RenderStatusChangeNotificationView(events []*remote.Event, status, url stri return statusChangeAttachments(e, status, url) } } + + nEvents := len(events) + if nEvents > 0 && status == model.STATUS_DND { + return statusChangeAttachments(events[nEvents-1], status, url) + } + return statusChangeAttachments(nil, status, url) } +func RenderEventWillStartLine(subject, weblink string, startTime time.Time) string { + link, _ := url.QueryUnescape(weblink) + eventString := fmt.Sprintf("Your event [%s](%s) will start soon.", subject, link) + if subject == "" { + eventString = fmt.Sprintf("[An event with no subject](%s) will start soon.", link) + } + if startTime.Before(time.Now()) { + eventString = fmt.Sprintf("Your event [%s](%s) is ongoing.", subject, link) + if subject == "" { + eventString = fmt.Sprintf("[An event with no subject](%s) is ongoing.", link) + } + } + return eventString +} + func renderScheduleItem(event *remote.Event, status string) string { if event == nil { return fmt.Sprintf("You have no upcoming events.\n Shall I change your status to %s?", prettyStatuses[status]) } - resp := fmt.Sprintf("Your event with subject `%s` will start soon.", event.Subject) - if event.Subject == "" { - resp = "An event with no subject will start soon." - } + resp := RenderEventWillStartLine(event.Subject, event.Weblink, event.Start.Time()) resp += fmt.Sprintf("\nShall I change your status to %s?", prettyStatuses[status]) return resp @@ -47,6 +67,7 @@ func statusChangeAttachments(event *remote.Event, status, url string) *model.Sla "value": true, "change_to": status, "pretty_change_to": prettyStatuses[status], + "hasEvent": false, }, }, } @@ -56,11 +77,25 @@ func statusChangeAttachments(event *remote.Event, status, url string) *model.Sla Integration: &model.PostActionIntegration{ URL: url, Context: map[string]interface{}{ - "value": false, + "value": false, + "hasEvent": false, }, }, } + if event != nil { + marshalledStart, _ := json.Marshal(event.Start.Time()) + actionYes.Integration.Context["hasEvent"] = true + actionYes.Integration.Context["subject"] = event.Subject + actionYes.Integration.Context["weblink"] = event.Weblink + actionYes.Integration.Context["startTime"] = string(marshalledStart) + + actionNo.Integration.Context["hasEvent"] = true + actionNo.Integration.Context["subject"] = event.Subject + actionNo.Integration.Context["weblink"] = event.Weblink + actionNo.Integration.Context["startTime"] = string(marshalledStart) + } + sa := &model.SlackAttachment{ Title: "Status change", Text: renderScheduleItem(event, status),