Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
dist
server/manifest.go
webapp/src/manifest.ts
.DS_Store
.npminstall
.idea
bin

node_modules
# notice
.notice-work
16 changes: 10 additions & 6 deletions calendar/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,19 @@ func Init(h *httputils.Handler, env engine.Env, notificationProcessor engine.Not
postActionRouter.HandleFunc(config.PathConfirmStatusChange, api.postActionConfirmStatusChange).Methods(http.MethodPost)

dialogRouter := h.Router.PathPrefix(config.PathAutocomplete).Subrouter()
dialogRouter.HandleFunc(config.PathUsers, api.autocompleteConnectedUsers)
dialogRouter.HandleFunc(config.PathUsers, api.autocompleteConnectedUsers).Methods(http.MethodGet)

apiRoutes := h.Router.PathPrefix(config.InternalAPIPath).Subrouter()
eventsRouter := apiRoutes.PathPrefix(config.PathEvents).Subrouter()
eventsRouter.HandleFunc(config.PathCreate, api.createEvent).Methods(http.MethodPost)
apiRoutes.HandleFunc(config.PathConnectedUser, api.connectedUserHandler)
eventsRouter.HandleFunc(config.PathView, api.viewEvents).Methods(http.MethodGet)
apiRoutes.HandleFunc(config.PathConnectedUser, api.connectedUserHandler).Methods(http.MethodGet)

// Returns provider information for the plugin to use
apiRoutes.HandleFunc(config.PathProvider, func(w http.ResponseWriter, r *http.Request) {
httputils.WriteJSONResponse(w, config.Provider, http.StatusOK)
})
apiRoutes.HandleFunc(config.PathProvider, api.getProviderConfiguration).Methods(http.MethodGet)
}

func (api *api) getProviderConfiguration(w http.ResponseWriter, r *http.Request) {
resp := config.Provider
resp.Features.EnableExperimentalUI = api.Config.EnableExperimentalUI
httputils.WriteJSONResponse(w, resp, http.StatusOK)
}
16 changes: 12 additions & 4 deletions calendar/api/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/store"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/utils/bot"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/utils/httputils"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/utils/tz"
)

const (
Expand Down Expand Up @@ -82,7 +83,7 @@ func (cep createEventPayload) ToRemoteEvent(loc *time.Location) (*remote.Event,
if cep.Description != "" {
evt.Body = &remote.ItemBody{
Content: cep.Description,
ContentType: "text/plain",
ContentType: "text",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

verify this doesn't break if the Google Calendar provider expects "text/plain".

}
}
evt.Subject = cep.Subject
Expand Down Expand Up @@ -195,10 +196,17 @@ func (api *api) createEvent(w http.ResponseWriter, r *http.Request) {
return
}

loc, errLocation := time.LoadLocation(mailbox.TimeZone)
tzName := tz.Go(mailbox.TimeZone)
if tzName == "" && mailbox.TimeZone != "" {
errTz := fmt.Errorf("unknown time zone %q", mailbox.TimeZone)
api.Logger.With(bot.LogContext{"err": errTz.Error(), "timezone": mailbox.TimeZone}).Errorf("createEvent, error occurred while loading mailbox timezone location")
httputils.WriteInternalServerError(w, errTz)
return
}
loc, errLocation := time.LoadLocation(tzName)
if errLocation != nil {
api.Logger.With(bot.LogContext{"err": errLocation.Error(), "timezone": mailbox.TimeZone}).Errorf("createEvent, error occurred while loading mailbox timezone location")
httputils.WriteInternalServerError(w, errLocation)
api.Logger.With(bot.LogContext{"err": errLocation.Error(), "timezone": mailbox.TimeZone, "converted": tzName}).Errorf("createEvent, error occurred while loading mailbox timezone location")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both error paths pass the raw error to WriteInternalServerError. The errTz includes the user's timezone string which is fine, but errLocation from time.LoadLocation can expose Go runtime details. Should return a generic message to the client.

httputils.WriteInternalServerError(w, fmt.Errorf("unable to resolve mailbox timezone"))
return
}

Expand Down
4 changes: 2 additions & 2 deletions calendar/api/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestToRemoteEvent(t *testing.T) {
},
Body: &remote.ItemBody{
Content: "Discuss the quarterly results.",
ContentType: "text/plain",
ContentType: "text",
},
}
assert.Equal(t, expectedEvent, event)
Expand Down Expand Up @@ -332,7 +332,7 @@ func TestCreateEvent(t *testing.T) {
mockStore.EXPECT().LoadUser(MockUserID).Return(&store.User{MattermostUserID: MockUserID, OAuth2Token: &mockOAauthToken, Remote: &remote.User{ID: MockRemoteUserID}}, nil).Times(1)
mockPluginAPI.EXPECT().CanLinkEventToChannel(MockChannelID, MockUserID).Return(true).Times(1)
mockRemote.EXPECT().MakeUserClient(gomock.Any(), &mockOAauthToken, gomock.Any(), gomock.Any(), gomock.Any()).Return(mockRemoteClient).Times(1)
mockRemoteClient.EXPECT().GetMailboxSettings(MockRemoteUserID).Return(&remote.MailboxSettings{TimeZone: "Invalid/TimeZone"}, nil).Times(1)
mockRemoteClient.EXPECT().GetMailboxSettings(MockRemoteUserID).Return(&remote.MailboxSettings{TimeZone: "Not_A_Real_Timezone_!@#"}, nil).Times(1)
mockLogger.EXPECT().With(gomock.Any()).Return(mockLoggerWith).Times(1)
mockLoggerWith.EXPECT().Errorf("createEvent, error occurred while loading mailbox timezone location").Times(1)
},
Expand Down
81 changes: 81 additions & 0 deletions calendar/api/events_view.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

package api

import (
"errors"
"fmt"
"net/http"
"time"

"github.com/mattermost/mattermost-plugin-mscalendar/calendar/engine"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/remote"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/store"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/utils/bot"
"github.com/mattermost/mattermost-plugin-mscalendar/calendar/utils/httputils"
)

func (api *api) viewEvents(w http.ResponseWriter, r *http.Request) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This new endpoint has a lot of new code paths and zero tests. Needs a TestViewEvents table covering at least: missing header, missing params, invalid date, from after to, range > 62 days, store.ErrNotFound, engine error, happy path.

mattermostUserID := r.Header.Get("Mattermost-User-Id")
if mattermostUserID == "" {
api.Logger.Errorf("viewEvents, unauthorized user")
httputils.WriteUnauthorizedError(w, fmt.Errorf("unauthorized"))
return
}

fromStr := r.URL.Query().Get("from")
toStr := r.URL.Query().Get("to")
if fromStr == "" || toStr == "" {
httputils.WriteBadRequestError(w, fmt.Errorf("from and to query parameters are required"))
return
}

from, err := time.Parse(time.RFC3339, fromStr)
if err != nil {
httputils.WriteBadRequestError(w, fmt.Errorf("invalid from parameter: %w", err))
return
}

to, err := time.Parse(time.RFC3339, toStr)
if err != nil {
httputils.WriteBadRequestError(w, fmt.Errorf("invalid to parameter: %w", err))
return
}

if from.After(to) {
httputils.WriteBadRequestError(w, fmt.Errorf("from must be before or equal to to"))
return
}

const maxWindow = 62 * 24 * time.Hour
if to.Sub(from) > maxWindow {
httputils.WriteBadRequestError(w, fmt.Errorf("date range must not exceed 62 days"))
return
}

eng := engine.New(api.Env, mattermostUserID)
user := engine.NewUser(mattermostUserID)

events, err := eng.ViewCalendar(user, from, to)
if err != nil {
if errors.Is(err, store.ErrNotFound) {
api.Logger.With(bot.LogContext{"err": err.Error()}).Errorf("viewEvents, user not found in store")
httputils.WriteUnauthorizedError(w, fmt.Errorf("unauthorized"))
return
}
api.Logger.With(bot.LogContext{"err": err.Error()}).Errorf("viewEvents, error fetching calendar events")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The raw err which could contain store driver details, remote API errors, etc. is passed directly to the client.

httputils.WriteInternalServerError(w, fmt.Errorf("error fetching calendar events"))
return
}

if events == nil {
events = []*remote.Event{}
}

for _, e := range events {
remote.NormalizeDateTimeToRFC3339(e)
}

httputils.WriteJSONResponse(w, events, http.StatusOK)
}
Loading
Loading