Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Yves Duhem committed Nov 6, 2018
1 parent 4c4b25a commit 6992160
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 12 deletions.
12 changes: 6 additions & 6 deletions model/stats/db.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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{}

Expand All @@ -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.
Expand All @@ -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)
}
Expand Down
10 changes: 4 additions & 6 deletions model/stats/query.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6992160

Please sign in to comment.