Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-21689] Handle timezone complications between Go and Microsoft Graph #25

Merged
merged 15 commits into from
Jan 30, 2020
Merged
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ require (
github.com/stretchr/testify v1.4.0
github.com/yaegashi/msgraph.go v0.0.0-20191104022859-3f9096c750b2
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
google.golang.org/appengine v1.6.4
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,10 @@ github.com/mattermost/gorp v2.0.1-0.20190301154413-3b31e9a39d05+incompatible h1:
github.com/mattermost/gorp v2.0.1-0.20190301154413-3b31e9a39d05+incompatible/go.mod h1:0kX1qa3DOpaPJyOdMLeo7TcBN0QmUszj9a/VygOhDe0=
github.com/mattermost/ldap v3.0.4+incompatible h1:SOeNnz+JNR+foQ3yHkYqijb9MLPhXN2BZP/PdX23VDU=
github.com/mattermost/ldap v3.0.4+incompatible/go.mod h1:b4reDCcGpBxJ4WX0f224KFY+OR0npin7or7EFpeIko4=
github.com/mattermost/mattermost-server v5.11.1+incompatible h1:LPzKY0+2Tic/ik67qIg6VrydRCgxNXZQXOeaiJ2rMBY=
mickmister marked this conversation as resolved.
Show resolved Hide resolved
github.com/mattermost/mattermost-server/v5 v5.18.0-rc.test h1:XRbwQ+r1X0++KR7BmUK6wI0nHTGGm4WNTxTz86xf/Z4=
github.com/mattermost/mattermost-server/v5 v5.18.0-rc.test/go.mod h1:hq/S3zdC99iZALqM90Fq9TKp0x3M24/WVbOpHVb+ukI=
github.com/mattermost/mattermost-server/v5 v5.19.1 h1:lV9LKShbW3PsSQJknDN5lxd0X5yjylu8JmWAbAasufU=
github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 h1:G9tL6JXRBMzjuD1kkBtcnd42kUiT6QDwxfFYu7adM6o=
github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0/go.mod h1:nV5bfVpT//+B1RPD2JvRnxbkLmJEYXmRaaVl15fsXjs=
github.com/mattermost/viper v1.0.4 h1:cMYOz4PhguscGSPxrSokUtib5HrG4gCpiUh27wyA3d0=
Expand Down
7 changes: 5 additions & 2 deletions server/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/plugin_api"
"github.com/pkg/errors"
)

type OAuth2 interface {
Expand All @@ -35,6 +36,7 @@ type Calendar interface {
DeleteCalendar(calendarID string) error
FindMeetingTimes(meetingParams *remote.FindMeetingTimesParameters) (*remote.MeetingTimeSuggestionResults, error)
GetUserCalendars(userID string) ([]*remote.Calendar, error)
GetUserTimezone(mattermostUserID string) (string, error)
}

type Event interface {
Expand All @@ -61,6 +63,7 @@ type API interface {
Event
OAuth2
Subscriptions
bot.Logger
}

// Dependencies contains all API dependencies
Expand All @@ -69,7 +72,7 @@ type Dependencies struct {
OAuth2StateStore store.OAuth2StateStore
SubscriptionStore store.SubscriptionStore
EventStore store.EventStore
Logger bot.Logger
bot.Logger
Poster bot.Poster
Remote remote.Remote
IsAuthorizedAdmin func(userId string) (bool, error)
Expand Down Expand Up @@ -126,7 +129,7 @@ func withUser(api *api) error {

user, err := api.UserStore.LoadUser(api.mattermostUserID)
if err != nil {
return err
return errors.Wrap(err, "User not connected")
}

api.user = user
Expand Down
4 changes: 2 additions & 2 deletions server/api/availability.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ func (api *api) setUserStatuses(filteredUsers store.UserIndex, schedules []*remo
func (api *api) GetUserAvailabilities(remoteUserID string, scheduleIDs []string) ([]*remote.ScheduleInformation, error) {
client := api.MakeSuperuserClient()

start := remote.NewDateTime(time.Now())
end := remote.NewDateTime(time.Now().Add(availabilityTimeWindowSize * time.Minute))
start := remote.NewDateTime(time.Now().UTC(), "UTC")
end := remote.NewDateTime(time.Now().UTC().Add(availabilityTimeWindowSize*time.Minute), "UTC")

return client.GetSchedule(remoteUserID, scheduleIDs, start, end, availabilityTimeWindowSize)
}
Expand Down
25 changes: 25 additions & 0 deletions server/api/calendar.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,28 @@ func (api *api) GetUserCalendars(userID string) ([]*remote.Calendar, error) {
me, err := client.GetMe()
return client.GetUserCalendars(me.ID)
}

func (api *api) GetUserTimezone(mattermostUserID string) (string, error) {
client, err := api.MakeClient()
if err != nil {
return "", err
}

var remoteUserID string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this is handled in solar lottery as a separate function do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crspeller Can you confirm I've addressed this in the most recent push?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not sure what the ask is here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I wasn't clear. I like the split out you did. I really meant the withX stuff the solar lottery plugin does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming in #29

if api.user != nil && api.user.MattermostUserID == mattermostUserID {
remoteUserID = api.user.Remote.ID
} else {
u, storeErr := api.UserStore.LoadUser(mattermostUserID)
if storeErr != nil {
return "", storeErr
}
remoteUserID = u.Remote.ID
}

settings, err := client.GetUserMailboxSettings(remoteUserID)
if err != nil {
return "", err
}

return settings.TimeZone, nil
}
15 changes: 15 additions & 0 deletions server/api/mock_api/mock_calendar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 4 additions & 10 deletions server/api/status_sync_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package api
import (
"sync"
"time"

"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot"
)

const JOB_INTERVAL = 5 * time.Minute
Expand All @@ -19,20 +17,16 @@ type StatusSyncJob struct {
cancelOnce sync.Once
}

func (j *StatusSyncJob) getLogger() bot.Logger {
return j.api.(*api).Logger
}

func (j *StatusSyncJob) work() {
log := j.getLogger()
log.Debugf("User status sync job beginning")
api := j.api
api.Debugf("User status sync job beginning")

_, err := j.api.SyncStatusForAllUsers()
if err != nil {
log.Errorf("Error during user status sync job", "error", err.Error())
api.Errorf("Error during user status sync job", "error", err.Error())
}

log.Debugf("User status sync job finished")
api.Debugf("User status sync job finished")
}

func NewStatusSyncJob(api API) *StatusSyncJob {
Expand Down
13 changes: 9 additions & 4 deletions server/plugin/command/create_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ func (c *Command) createEvent(parameters ...string) (string, error) {
return fmt.Sprintf(getCreateEventFlagSet().FlagUsages()), nil
}

event, err := parseCreateArgs(parameters)
tz, err := c.API.GetUserTimezone(c.Args.UserId)
if err != nil {
return "", nil
}

event, err := parseCreateArgs(parameters, tz)
if err != nil {
return err.Error(), nil
}
Expand All @@ -58,7 +63,7 @@ func (c *Command) createEvent(parameters ...string) (string, error) {
return resp, nil
}

func parseCreateArgs(args []string) (*remote.Event, error) {
func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {

event := &remote.Event{}

Expand Down Expand Up @@ -117,7 +122,7 @@ func parseCreateArgs(args []string) (*remote.Event, error) {
}
event.Start = &remote.DateTime{
DateTime: startTime,
TimeZone: "Pacific Standard Time",
TimeZone: timeZone,
}

endTime, err := createFlagSet.GetString("endtime")
Expand All @@ -129,7 +134,7 @@ func parseCreateArgs(args []string) (*remote.Event, error) {
}
event.End = &remote.DateTime{
DateTime: endTime,
TimeZone: "Pacific Standard Time",
TimeZone: timeZone,
}

allday, err := createFlagSet.GetBool("allday")
Expand Down
19 changes: 18 additions & 1 deletion server/plugin/command/find_meeting_times.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package command

import (
"fmt"
"strings"

"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
Expand Down Expand Up @@ -32,10 +33,26 @@ func (c *Command) findMeetings(parameters ...string) (string, error) {
return "", err
}

var timeZone string
tz, err := c.API.GetUserTimezone(c.Args.UserId)
if err == nil {
timeZone = tz
}

resp := ""
for _, m := range meetings.MeetingTimeSuggestions {
resp += utils.JSONBlock(m)
if timeZone != "" {
m.MeetingTimeSlot.Start = m.MeetingTimeSlot.Start.ConvertToTimezone(timeZone)
m.MeetingTimeSlot.End = m.MeetingTimeSlot.End.ConvertToTimezone(timeZone)
}
resp += utils.JSONBlock(renderMeetingTime(m))
}

return resp, nil
}

func renderMeetingTime(m *remote.MeetingTimeSuggestion) string {
start := m.MeetingTimeSlot.Start.PrettyString()
end := m.MeetingTimeSlot.End.PrettyString()
return fmt.Sprintf("%s - %s (%s)", start, end, m.MeetingTimeSlot.Start.TimeZone)
}
8 changes: 6 additions & 2 deletions server/plugin/command/get_calendars.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package command

import (
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
)

func (c *Command) showCalendars(parameters ...string) (string, error) {

_, err := c.API.GetUserCalendars("")
r, err := c.API.GetUserCalendars("")
if err != nil {
return "", err
}

return "", nil
return utils.JSONBlock(r), nil
}
15 changes: 15 additions & 0 deletions server/plugin/command/view_calendar.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,23 @@ func (c *Command) viewCalendar(parameters ...string) (string, error) {
return "", err
}

var timeZone string
tz, err := c.API.GetUserTimezone(c.Args.UserId)
if err == nil {
timeZone = tz
}

if timeZone != "" {
for _, event := range events {
event.Start = event.Start.ConvertToTimezone(timeZone)
mickmister marked this conversation as resolved.
Show resolved Hide resolved
event.End = event.End.ConvertToTimezone(timeZone)
}
}

resp := ""
for _, e := range events {
e.Start = e.Start.ConvertToTimezone(timeZone)
e.End = e.End.ConvertToTimezone(timeZone)
resp += " - " + e.ID + utils.JSONBlock(e)
}

Expand Down
1 change: 0 additions & 1 deletion server/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func (p *Plugin) OnActivate() error {
if err != nil {
return err
}

p.httpHandler = http.NewHandler()

p.notificationHandler = api.NewNotificationHandler(p.newAPIConfig())
Expand Down
1 change: 1 addition & 0 deletions server/remote/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type Client interface {
GetSchedule(remoteUserID string, schedules []string, startTime, endTime *DateTime, availabilityViewInterval int) ([]*ScheduleInformation, error)
GetUserDefaultCalendarView(userID string, startTime, endTime time.Time) ([]*Event, error)
GetUserEvent(userID, eventID string) (*Event, error)
GetUserMailboxSettings(remoteUserID string) (*MailboxSettings, error)
ListSubscriptions() ([]*Subscription, error)
RenewSubscription(subscriptionID string) (*Subscription, error)
TentativelyAcceptUserEvent(userID, eventID string) error
Expand Down
55 changes: 50 additions & 5 deletions server/remote/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@

package remote

import "time"
import (
"time"
mickmister marked this conversation as resolved.
Show resolved Hide resolved

"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
)

type EmailAddress struct {
Address string `json:"address"`
Expand All @@ -16,11 +20,14 @@ type DateTime struct {

const RFC3339NanoNoTimezone = "2006-01-02T15:04:05.999999999"

func NewDateTime(t time.Time) *DateTime {
// NewDateTime creates a DateTime that is compatible with Microsoft's API
// Callers of this function are responsible for supplying a valid Windows Timezone
// If the context is for a specific user, we will fetch their timezone from Microsoft before calling this function
// Else for system time we use UTC
func NewDateTime(t time.Time, winTZ string) *DateTime {
return &DateTime{
DateTime: t.Format(RFC3339NanoNoTimezone),
// TimeZone: t.Format("MST"),
TimeZone: "Eastern Standard Time",
TimeZone: winTZ,
}
}

Expand All @@ -32,8 +39,37 @@ func (dt DateTime) String() string {
return t.Format(time.RFC3339)
}

func (dt DateTime) PrettyString() string {
t := dt.Time()
if t.IsZero() {
return "n/a"
}
return t.Format(time.RFC822)
}

func (dt DateTime) ConvertToTimezone(timeZone string) *DateTime {
mickmister marked this conversation as resolved.
Show resolved Hide resolved
t := dt.Time()
if t.IsZero() {
return &dt
}

tz := safeTimeZone(timeZone)

loc, err := time.LoadLocation(tz)
if err == nil {
t = t.In(loc)
}

return &DateTime{
TimeZone: timeZone,
DateTime: t.Format(RFC3339NanoNoTimezone),
}
}

func (dt DateTime) Time() time.Time {
loc, err := time.LoadLocation(dt.TimeZone)
tz := safeTimeZone(dt.TimeZone)

loc, err := time.LoadLocation(tz)
if err != nil {
return time.Time{}
}
Expand All @@ -44,3 +80,12 @@ func (dt DateTime) Time() time.Time {
}
return t
}

func safeTimeZone(timeZone string) string {
levb marked this conversation as resolved.
Show resolved Hide resolved
_, err := time.LoadLocation(timeZone)
if err != nil {
return utils.ConvertWindowsTimezoneToIANA(timeZone)
}

return timeZone
}
Loading