From 093c657e56902dc066700898a251964bd16e5ddc Mon Sep 17 00:00:00 2001 From: cyrilzhang-mm <112951043+cyrilzhang-mm@users.noreply.github.com> Date: Thu, 20 Oct 2022 14:37:24 -0400 Subject: [PATCH] [MM-45052] Add error checking in insights API (#21048) --- api4/insights.go | 60 +++++++++++++++++++++++++++++++++++-------- api4/insights_test.go | 24 +++++++++++++++++ app/channel_test.go | 8 +++--- app/reaction_test.go | 4 +-- i18n/en.json | 4 +++ model/insights.go | 18 +++++++++++++ 6 files changed, 102 insertions(+), 16 deletions(-) diff --git a/api4/insights.go b/api4/insights.go index fce03cfa04..9d74a57c2f 100644 --- a/api4/insights.go +++ b/api4/insights.go @@ -73,7 +73,11 @@ func getTopReactionsForTeamSince(c *Context, w http.ResponseWriter, r *http.Requ return } - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + if appErr != nil { + c.Err = appErr + return + } topReactionList, appErr := c.App.GetTopReactionsForTeamSince(c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -134,7 +138,11 @@ func getTopReactionsForUserSince(c *Context, w http.ResponseWriter, r *http.Requ return } - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + if appErr != nil { + c.Err = appErr + return + } topReactionList, appErr := c.App.GetTopReactionsForUserSince(c.AppContext.Session().UserId, c.Params.TeamId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -193,7 +201,11 @@ func getTopChannelsForTeamSince(c *Context, w http.ResponseWriter, r *http.Reque } loc := user.GetTimezoneLocation() - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, loc) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, loc) + if appErr != nil { + c.Err = appErr + return + } topChannels, appErr := c.App.GetTopChannelsForTeamSince(c.AppContext, c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -261,7 +273,11 @@ func getTopChannelsForUserSince(c *Context, w http.ResponseWriter, r *http.Reque } loc := user.GetTimezoneLocation() - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, loc) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, loc) + if appErr != nil { + c.Err = appErr + return + } topChannels, appErr := c.App.GetTopChannelsForUserSince(c.AppContext, c.AppContext.Session().UserId, c.Params.TeamId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -325,7 +341,11 @@ func getTopThreadsForTeamSince(c *Context, w http.ResponseWriter, r *http.Reques return } - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + if appErr != nil { + c.Err = appErr + return + } topThreads, appErr := c.App.GetTopThreadsForTeamSince(c.AppContext, c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -386,7 +406,11 @@ func getTopThreadsForUserSince(c *Context, w http.ResponseWriter, r *http.Reques } } - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + if appErr != nil { + c.Err = appErr + return + } topThreads, appErr := c.App.GetTopThreadsForUserSince(c.AppContext, c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -427,7 +451,11 @@ func getTopDMsForUserSince(c *Context, w http.ResponseWriter, r *http.Request) { return } - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, user.GetTimezoneLocation()) + if appErr != nil { + c.Err = appErr + return + } topDMs, err := c.App.GetTopDMsForUserSince(user.Id, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -487,7 +515,11 @@ func getTopInactiveChannelsForTeamSince(c *Context, w http.ResponseWriter, r *ht } loc := user.GetTimezoneLocation() - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, loc) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, loc) + if appErr != nil { + c.Err = appErr + return + } topChannels, err := c.App.GetTopInactiveChannelsForTeamSince(c.AppContext, c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -548,7 +580,11 @@ func getTopInactiveChannelsForUserSince(c *Context, w http.ResponseWriter, r *ht } loc := user.GetTimezoneLocation() - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, loc) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, loc) + if appErr != nil { + c.Err = appErr + return + } topChannels, err := c.App.GetTopInactiveChannelsForUserSince(c.AppContext, c.Params.TeamId, c.AppContext.Session().UserId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), @@ -622,7 +658,11 @@ func getNewTeamMembersSince(c *Context, w http.ResponseWriter, r *http.Request) return } loc := user.GetTimezoneLocation() - startTime := model.StartOfDayForTimeRange(c.Params.TimeRange, loc) + startTime, appErr := model.GetStartOfDayForTimeRange(c.Params.TimeRange, loc) + if appErr != nil { + c.Err = appErr + return + } ntms, count, err := c.App.GetNewTeamMembersSince(c.AppContext, c.Params.TeamId, &model.InsightsOpts{ StartUnixMilli: startTime.UnixMilli(), diff --git a/api4/insights_test.go b/api4/insights_test.go index 089bd26842..2443c1ea26 100644 --- a/api4/insights_test.go +++ b/api4/insights_test.go @@ -230,6 +230,12 @@ func TestGetTopReactionsForTeamSince(t *testing.T) { CheckNotFoundStatus(t, resp) }) + t.Run("get-top-reactions-for-team-since invalid time range", func(t *testing.T) { + _, resp, err := client.GetTopReactionsForTeamSince(teamId, "7_days", 0, 5) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + t.Run("get-top-reactions-for-team-since not a member of team", func(t *testing.T) { th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam) _, resp, err := client.GetTopReactionsForTeamSince(teamId, model.TimeRangeToday, 0, 5) @@ -417,6 +423,12 @@ func TestGetTopReactionsForUserSince(t *testing.T) { CheckNotFoundStatus(t, resp) }) + t.Run("get-top-reactions-for-user-since invalid time range", func(t *testing.T) { + _, resp, err := client.GetTopReactionsForUserSince(teamId, "7_days", 0, 5) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + t.Run("get-top-reactions-for-user-since not a member of team", func(t *testing.T) { th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam) _, resp, err := client.GetTopReactionsForUserSince(teamId, model.TimeRangeToday, 0, 5) @@ -515,6 +527,12 @@ func TestGetTopChannelsForTeamSince(t *testing.T) { CheckNotFoundStatus(t, resp) }) + t.Run("get-top-channels-for-team-since invalid time range", func(t *testing.T) { + _, resp, err := client.GetTopChannelsForTeamSince(teamId, "7_days", 0, 5) + assert.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + t.Run("get-top-channels-for-team-since not a member of team", func(t *testing.T) { th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam) _, resp, err := client.GetTopChannelsForTeamSince(teamId, model.TimeRangeToday, 0, 5) @@ -592,6 +610,12 @@ func TestGetTopChannelsForUserSince(t *testing.T) { CheckNotFoundStatus(t, resp) }) + t.Run("get-top-channels-for-user-since invalid time range", func(t *testing.T) { + _, resp, err := client.GetTopChannelsForUserSince(teamId, "7_days", 0, 5) + assert.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + t.Run("get-top-channels-for-user-since not a member of team", func(t *testing.T) { th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam) _, resp, err := client.GetTopChannelsForUserSince(teamId, model.TimeRangeToday, 0, 5) diff --git a/app/channel_test.go b/app/channel_test.go index 260b46d641..0361122027 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -2498,7 +2498,7 @@ func TestGetTopChannelsForTeamSince(t *testing.T) { {ID: channel5.Id, MessageCount: 2}, } - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-channels-for-team-since", func(t *testing.T) { topChannels, err := th.App.GetTopChannelsForTeamSince(th.Context, th.BasicChannel.TeamId, th.BasicUser.Id, &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 5}) @@ -2576,7 +2576,7 @@ func TestGetTopChannelsForUserSince(t *testing.T) { {ID: channel5.Id, MessageCount: 2}, } - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-channels-for-user-since", func(t *testing.T) { topChannels, err := th.App.GetTopChannelsForUserSince(th.Context, th.BasicUser.Id, "", &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 5}) @@ -2787,7 +2787,7 @@ func TestGetTopInactiveChannelsForTeamSince(t *testing.T) { {ID: channel2.Id, MessageCount: 6}, } - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-channels-for-team-since", func(t *testing.T) { topChannels, err := th.App.GetTopInactiveChannelsForTeamSince(th.Context, th.BasicChannel.TeamId, th.BasicUser.Id, &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 5}) @@ -2885,7 +2885,7 @@ func TestGetTopInactiveChannelsForUserSince(t *testing.T) { {ID: channel2.Id, MessageCount: 6}, } - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-channels-for-user-since", func(t *testing.T) { topChannels, err := th.App.GetTopInactiveChannelsForUserSince(th.Context, th.BasicChannel.TeamId, th.BasicUser.Id, &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 4}) diff --git a/app/reaction_test.go b/app/reaction_test.go index d9670e4eeb..33d747136b 100644 --- a/app/reaction_test.go +++ b/app/reaction_test.go @@ -230,7 +230,7 @@ func TestGetTopReactionsForTeamSince(t *testing.T) { expectedTopReactions[3] = &model.TopReaction{EmojiName: "sad", Count: int64(3)} expectedTopReactions[4] = &model.TopReaction{EmojiName: "happy", Count: int64(2)} - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-reactions-for-team-since", func(t *testing.T) { topReactions, err := th.App.GetTopReactionsForTeamSince(teamId, userId, &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 5}) @@ -401,7 +401,7 @@ func TestGetTopReactionsForUserSince(t *testing.T) { expectedTopReactions[3] = &model.TopReaction{EmojiName: "heart", Count: int64(3)} expectedTopReactions[4] = &model.TopReaction{EmojiName: "blush", Count: int64(2)} - timeRange := model.StartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) + timeRange, _ := model.GetStartOfDayForTimeRange(model.TimeRangeToday, time.Now().Location()) t.Run("get-top-reactions-for-user-since", func(t *testing.T) { topReactions, err := th.App.GetTopReactionsForUserSince(userId, teamId, &model.InsightsOpts{StartUnixMilli: timeRange.UnixMilli(), Page: 0, PerPage: 5}) diff --git a/i18n/en.json b/i18n/en.json index f5db7894ed..710054bf1a 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -8739,6 +8739,10 @@ "id": "model.incoming_hook.username.app_error", "translation": "Invalid username." }, + { + "id": "model.insights.get_start_of_day_for_time_range.time_range.app_error", + "translation": "Invalid time range." + }, { "id": "model.job.is_valid.create_at.app_error", "translation": "Create at must be a valid time." diff --git a/model/insights.go b/model/insights.go index 438ca9402c..d66d48c283 100644 --- a/model/insights.go +++ b/model/insights.go @@ -4,6 +4,7 @@ package model import ( + "net/http" "time" ) @@ -260,6 +261,23 @@ func StartOfDayForTimeRange(timeRange string, location *time.Location) *time.Tim return &resultTime } +// GetStartOfDayForTimeRange gets the unix start time in milliseconds from the given time range. +// Time range can be one of: "today", "7_day", or "28_day". +func GetStartOfDayForTimeRange(timeRange string, location *time.Location) (*time.Time, *AppError) { + now := time.Now().In(location) + resultTime := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, location) + switch timeRange { + case TimeRangeToday: + case TimeRange7Day: + resultTime = resultTime.Add(time.Hour * time.Duration(-144)) + case TimeRange28Day: + resultTime = resultTime.Add(time.Hour * time.Duration(-648)) + default: + return nil, NewAppError("GetStartOfDayForTimeRange", "model.insights.get_start_of_day_for_time_range.time_range.app_error", nil, "", http.StatusBadRequest) + } + return &resultTime, nil +} + // GetTopReactionListWithPagination adds a rank to each item in the given list of TopReaction and checks if there is // another page that can be fetched based on the given limit and offset. The given list of TopReaction is assumed to be // sorted by Count. Returns a TopReactionList.