From 699216023dda59fdb652db45f76edf8656d44fb6 Mon Sep 17 00:00:00 2001 From: Yves Duhem Date: Tue, 6 Nov 2018 11:47:58 -0500 Subject: [PATCH] PR Feedback --- model/stats/db.go | 12 ++++++------ model/stats/query.go | 10 ++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/model/stats/db.go b/model/stats/db.go index 5fe2c682133..7d4a725f9d0 100644 --- a/model/stats/db.go +++ b/model/stats/db.go @@ -599,7 +599,7 @@ func addMatchTestPagination(matchExpr bson.M, filter *StatsFilter) { values = []interface{}{filter.StartAt.Date, filter.StartAt.BuildVariant, filter.StartAt.Task, filter.StartAt.Test, filter.StartAt.Distro} } - addPaginationOrBranches(matchExpr, fields, operators, values) + matchExpr["$or"] = buildPaginationOrBranches(fields, operators, values) } // Creates an aggregation pipeline to query task statistics. @@ -694,14 +694,14 @@ func addMatchTaskPagination(matchExpr bson.M, filter *StatsFilter) { values = []interface{}{filter.StartAt.Date, filter.StartAt.BuildVariant, filter.StartAt.Task, filter.StartAt.Distro} } - addPaginationOrBranches(matchExpr, fields, operators, values) + matchExpr["$or"] = buildPaginationOrBranches(fields, operators, values) } -// Edits a match expression to include the or branches of the pagination constraints. +// Builds and returns the $or branches of the pagination constraints. // fields is an array of field names, they must be in the same order as the sort order. // operators is a list of MongoDB comparison operators ("$gte", "$gt", "$lte", "$lt") for the fields. // values is a list of values for the fields. -func addPaginationOrBranches(matchExpr bson.M, fields []string, operators []string, values []interface{}) { +func buildPaginationOrBranches(fields []string, operators []string, values []interface{}) []bson.M { baseConstraints := bson.M{} branches := []bson.M{} @@ -714,7 +714,7 @@ func addPaginationOrBranches(matchExpr bson.M, fields []string, operators []stri branches = append(branches, branch) baseConstraints[fields[i]] = values[i] } - matchExpr["$or"] = branches + return branches } // Returns the date boundaries when splitting the period between 'start' and 'end' in groups of 'numDays' days. @@ -730,7 +730,7 @@ func dateBoundaries(start time.Time, end time.Time, numDays int) []time.Time { boundary := start boundaries := []time.Time{} - for boundary.Unix() < end.Unix() { + for boundary.Before(end) { boundaries = append(boundaries, boundary) boundary = boundary.Add(duration) } diff --git a/model/stats/query.go b/model/stats/query.go index 0f3a4d8f6f1..a6a298915b3 100644 --- a/model/stats/query.go +++ b/model/stats/query.go @@ -39,7 +39,7 @@ type StartAt struct { // Validates that the StartAt struct is valid for its intended use. func (s *StartAt) validate(isTestFilter bool, groupBy GroupBy) error { catcher := grip.NewBasicCatcher() - if s.Date.Unix() != util.GetUTCDay(s.Date).Unix() { + if !s.Date.Equal(util.GetUTCDay(s.Date)) { catcher.Add(errors.New("Invalid StartAt Date value")) } if isTestFilter && len(s.Test) == 0 { @@ -116,15 +116,13 @@ func (f *StatsFilter) validate(isTestFilter bool) error { if f.GroupNumDays <= 0 { catcher.Add(errors.New("Invalid GroupNumDays value")) } - afterUnix := f.AfterDate.Unix() - beforeUnix := f.BeforeDate.Unix() - if afterUnix != util.GetUTCDay(f.AfterDate).Unix() { + if !f.AfterDate.Equal(util.GetUTCDay(f.AfterDate)) { catcher.Add(errors.New("Invalid AfterDate value")) } - if beforeUnix != util.GetUTCDay(f.BeforeDate).Unix() { + if !f.BeforeDate.Equal(util.GetUTCDay(f.BeforeDate)) { catcher.Add(errors.New("Invalid BeforeDate value")) } - if afterUnix >= beforeUnix { + if !f.BeforeDate.After(f.AfterDate) { catcher.Add(errors.New("Invalid AfterDate/BeforeDate values")) } if isTestFilter {