From 4d22f83da5d55d7a6b2c26d33bac67f983f77294 Mon Sep 17 00:00:00 2001
From: Tim Raymond <tim@timraymond.com>
Date: Tue, 13 Jun 2017 14:42:52 -0400
Subject: [PATCH] Fix :dashboardTime: by introducing tvar precedence

In order for :autoGroupBy: and :dashboardTime: to co-exist in a query,
it's necessary to introduce template variable precedence to the backend.
This is done by adding a `Precedence()` method to the TemplateVariable
interface that returns an ordinal indicating the precedence level of the
template variable. Precedence starts from 0 (highest) proceeding to the
maximum that a `uint` can represent.

A template variable at a given precedence level can expect that all
template variables with higher precedence will have already been
replaced in the query that is passed to its `Exec` call.

For example, :autoGroupBy: has lower precedence than :dashboardTime:
because it needs to know the selected time range for the query. When the
`Exec` method of `GroupByVar` is invoked, it will see the query after
:dashboardTime: has already been replaced, allowing it to extract the
duration successfully.
---
 chronograf.go                                 | 11 +++++-
 influx/templates.go                           | 36 +++++++++++++------
 influx/templates_test.go                      | 24 ++++++++++++-
 ui/src/dashboards/containers/DashboardPage.js |  3 +-
 4 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/chronograf.go b/chronograf.go
index 70f92b03f..e5e4511fb 100644
--- a/chronograf.go
+++ b/chronograf.go
@@ -134,7 +134,8 @@ type Range struct {
 
 type TemplateVariable interface {
 	fmt.Stringer
-	Name() string // returns the variable name
+	Name() string     // returns the variable name
+	Precedence() uint // ordinal indicating precedence level for replacement
 }
 
 type ExecutableVar interface {
@@ -178,6 +179,10 @@ func (t BasicTemplateVar) String() string {
 	}
 }
 
+func (t BasicTemplateVar) Precedence() uint {
+	return 0
+}
+
 type GroupByVar struct {
 	Var               string        `json:"tempVar"`                     // the name of the variable as present in the query
 	Duration          time.Duration `json:"duration,omitempty"`          // the Duration supplied by the query
@@ -231,6 +236,10 @@ func (g *GroupByVar) Name() string {
 	return g.Var
 }
 
+func (g *GroupByVar) Precedence() uint {
+	return 1
+}
+
 // TemplateID is the unique ID used to identify a template
 type TemplateID string
 
diff --git a/influx/templates.go b/influx/templates.go
index 669aa526a..c006f7fa1 100644
--- a/influx/templates.go
+++ b/influx/templates.go
@@ -8,19 +8,33 @@ import (
 
 // TemplateReplace replaces templates with values within the query string
 func TemplateReplace(query string, templates chronograf.TemplateVars) string {
-	replacements := []string{}
-
-	for _, v := range templates {
-		if evar, ok := v.(chronograf.ExecutableVar); ok {
-			evar.Exec(query)
-		}
-		newVal := v.String()
-		if newVal != "" {
-			replacements = append(replacements, v.Name(), newVal)
+	tvarsByPrecedence := make(map[uint]chronograf.TemplateVars, len(templates))
+	maxPrecedence := uint(0)
+	for _, tmp := range templates {
+		precedence := tmp.Precedence()
+		if precedence > maxPrecedence {
+			maxPrecedence = precedence
 		}
+		tvarsByPrecedence[precedence] = append(tvarsByPrecedence[precedence], tmp)
+	}
+
+	replaced := query
+	for prc := uint(0); prc <= maxPrecedence; prc++ {
+		replacements := []string{}
+
+		for _, v := range tvarsByPrecedence[prc] {
+			if evar, ok := v.(chronograf.ExecutableVar); ok {
+				evar.Exec(replaced)
+			}
+			newVal := v.String()
+			if newVal != "" {
+				replacements = append(replacements, v.Name(), newVal)
+			}
+		}
+
+		replacer := strings.NewReplacer(replacements...)
+		replaced = replacer.Replace(replaced)
 	}
 
-	replacer := strings.NewReplacer(replacements...)
-	replaced := replacer.Replace(query)
 	return replaced
 }
diff --git a/influx/templates_test.go b/influx/templates_test.go
index 44a6a76ee..8d0e7ba0f 100644
--- a/influx/templates_test.go
+++ b/influx/templates_test.go
@@ -147,7 +147,29 @@ func TestTemplateReplace(t *testing.T) {
 					ReportingInterval: 10 * time.Second,
 				},
 			},
-			want: `SELECT mean(usage_idle) from "cpu" where time > now() - 4320h group by time(1555s)`,
+			want: `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 4320h group by time(1555s)`,
+		},
+		{
+			name:  "auto group by with :dashboardTime:",
+			query: `SELECT mean(usage_idle) from "cpu" WHERE time > :dashboardTime: :autoGroupBy:`,
+			vars: chronograf.TemplateVars{
+				&chronograf.GroupByVar{
+					Var:               ":autoGroupBy:",
+					Duration:          0 * time.Minute,
+					Resolution:        1000,
+					ReportingInterval: 10 * time.Second,
+				},
+				&chronograf.BasicTemplateVar{
+					Var: ":dashboardTime:",
+					Values: []chronograf.BasicTemplateValue{
+						{
+							Type:  "constant",
+							Value: "now() - 4320h",
+						},
+					},
+				},
+			},
+			want: `SELECT mean(usage_idle) from "cpu" WHERE time > now() - 4320h group by time(1555s)`,
 		},
 	}
 	for _, tt := range tests {
diff --git a/ui/src/dashboards/containers/DashboardPage.js b/ui/src/dashboards/containers/DashboardPage.js
index 106550edb..eea499798 100644
--- a/ui/src/dashboards/containers/DashboardPage.js
+++ b/ui/src/dashboards/containers/DashboardPage.js
@@ -268,8 +268,7 @@ class DashboardPage extends Component {
     }
 
     const templatesIncludingDashTime = (dashboard &&
-      dashboard.templates.concat(dashboardTime) &&
-      dashboard.templates.concat(autoGroupBy)) || []
+      dashboard.templates.concat(dashboardTime).concat(autoGroupBy)) || []
 
     const {selectedCell, isEditMode, isTemplating} = this.state