From f573e93ed429d1d1dd0a7be8a3b0042f0817cc00 Mon Sep 17 00:00:00 2001 From: siddweiker Date: Fri, 25 Jun 2021 12:59:25 -0400 Subject: [PATCH] Fix heatmap activity (#15252) * Group heatmap actions by 15 minute intervals Signed-off-by: Sidd Weiker * Add multi-contribution test for user heatmap Signed-off-by: Sidd Weiker * Add timezone aware summation for activity heatmap Signed-off-by: Sidd Weiker * Fix api user heatmap test Signed-off-by: Sidd Weiker * Update variable declaration style Co-authored-by: Lunny Xiao Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: techknowlogick --- integrations/api_user_heatmap_test.go | 2 +- models/fixtures/action.yml | 24 +++++++++++++++++++++++ models/user_heatmap.go | 11 ++++------- models/user_heatmap_test.go | 28 +++++++++++++++++++-------- web_src/js/features/heatmap.js | 11 +++++++++-- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/integrations/api_user_heatmap_test.go b/integrations/api_user_heatmap_test.go index 105d39e9ae..a0f0552a17 100644 --- a/integrations/api_user_heatmap_test.go +++ b/integrations/api_user_heatmap_test.go @@ -26,7 +26,7 @@ func TestUserHeatmap(t *testing.T) { var heatmap []*models.UserHeatmapData DecodeJSON(t, resp, &heatmap) var dummyheatmap []*models.UserHeatmapData - dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1603152000, Contributions: 1}) + dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1603227600, Contributions: 1}) assert.Equal(t, dummyheatmap, heatmap) } diff --git a/models/fixtures/action.yml b/models/fixtures/action.yml index 14cfd90423..e3f3d2a971 100644 --- a/models/fixtures/action.yml +++ b/models/fixtures/action.yml @@ -32,3 +32,27 @@ repo_id: 22 is_private: true created_unix: 1603267920 + +- id: 5 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 6 + is_private: true + created_unix: 1603010100 + +- id: 6 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 7 + is_private: true + created_unix: 1603011300 + +- id: 7 + user_id: 10 + op_type: 1 # create repo + act_user_id: 10 + repo_id: 8 + is_private: false + created_unix: 1603011540 # grouped with id:7 diff --git a/models/user_heatmap.go b/models/user_heatmap.go index 0e2767212e..306bd1819b 100644 --- a/models/user_heatmap.go +++ b/models/user_heatmap.go @@ -32,17 +32,14 @@ func getUserHeatmapData(user *User, team *Team, doer *User) ([]*UserHeatmapData, return hdata, nil } - var groupBy string + // Group by 15 minute intervals which will allow the client to accurately shift the timestamp to their timezone. + // The interval is based on the fact that there are timezones such as UTC +5:30 and UTC +12:45. + groupBy := "created_unix / 900 * 900" groupByName := "timestamp" // We need this extra case because mssql doesn't allow grouping by alias switch { - case setting.Database.UseSQLite3: - groupBy = "strftime('%s', strftime('%Y-%m-%d', created_unix, 'unixepoch'))" case setting.Database.UseMySQL: - groupBy = "UNIX_TIMESTAMP(DATE(FROM_UNIXTIME(created_unix)))" - case setting.Database.UsePostgreSQL: - groupBy = "extract(epoch from date_trunc('day', to_timestamp(created_unix)))" + groupBy = "created_unix DIV 900 * 900" case setting.Database.UseMSSQL: - groupBy = "datediff(SECOND, '19700101', dateadd(DAY, 0, datediff(day, 0, dateadd(s, created_unix, '19700101'))))" groupByName = groupBy } diff --git a/models/user_heatmap_test.go b/models/user_heatmap_test.go index 31e78a19cc..b2aaea6499 100644 --- a/models/user_heatmap_test.go +++ b/models/user_heatmap_test.go @@ -19,12 +19,20 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { CountResult int JSONResult string }{ - {2, 2, 1, `[{"timestamp":1603152000,"contributions":1}]`}, // self looks at action in private repo - {2, 1, 1, `[{"timestamp":1603152000,"contributions":1}]`}, // admin looks at action in private repo - {2, 3, 0, `[]`}, // other user looks at action in private repo - {2, 0, 0, `[]`}, // nobody looks at action in private repo - {16, 15, 1, `[{"timestamp":1603238400,"contributions":1}]`}, // collaborator looks at action in private repo - {3, 3, 0, `[]`}, // no action action not performed by target user + // self looks at action in private repo + {2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`}, + // admin looks at action in private repo + {2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`}, + // other user looks at action in private repo + {2, 3, 0, `[]`}, + // nobody looks at action in private repo + {2, 0, 0, `[]`}, + // collaborator looks at action in private repo + {16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`}, + // no action action not performed by target user + {3, 3, 0, `[]`}, + // multiple actions performed with two grouped together + {10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`}, } // Prepare assert.NoError(t, PrepareTestDatabase()) @@ -51,9 +59,13 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { // Get the heatmap and compare heatmap, err := GetUserHeatmapDataByUser(user, doer) + var contributions int + for _, hm := range heatmap { + contributions += int(hm.Contributions) + } assert.NoError(t, err) - assert.Len(t, heatmap, len(actions), "invalid action count: did the test data became too old?") - assert.Len(t, heatmap, tc.CountResult, fmt.Sprintf("testcase %d", i)) + assert.Len(t, actions, contributions, "invalid action count: did the test data became too old?") + assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase %d", i)) // Test JSON rendering json := jsoniter.ConfigCompatibleWithStandardLibrary diff --git a/web_src/js/features/heatmap.js b/web_src/js/features/heatmap.js index d1cb43dde0..07ecaee461 100644 --- a/web_src/js/features/heatmap.js +++ b/web_src/js/features/heatmap.js @@ -7,8 +7,15 @@ export default async function initHeatmap() { if (!el) return; try { - const values = JSON.parse(el.dataset.heatmapData).map(({contributions, timestamp}) => { - return {date: new Date(timestamp * 1000), count: contributions}; + const heatmap = {}; + JSON.parse(el.dataset.heatmapData).forEach(({contributions, timestamp}) => { + // Convert to user timezone and sum contributions by date + const dateStr = new Date(timestamp * 1000).toDateString(); + heatmap[dateStr] = (heatmap[dateStr] || 0) + contributions; + }); + + const values = Object.keys(heatmap).map((v) => { + return {date: new Date(v), count: heatmap[v]}; }); const View = Vue.extend({