Add compare query version support (re #7)

client/app/pages/queries/compare-query-dialog.css

@@ -0,0 +1,54 @@
+/* Compare Query Version container */
+/* Offers slight visual improvement (alignment) to modern UAs */
+.compare-query-version {
+  display: flex;
+  justify-content: space-between;
+  align-items: center;
+}
+
+.diff-removed {
+  background-color: rgba(208, 2, 27, 0.3);
+}
+
+.diff-added {
+  background-color: rgba(65, 117, 5, 0.3);
+}
+
+.query-diff-container span {
+  display: inline-block;
+  border-radius: 3px;
+  line-height: 20px;
+  vertical-align: middle;
+  margin: 0 5px 0 0;
+}
+
+.query-diff-container > div:not(.compare-query-version-controls) {
+  float: left;
+  width: calc(50% - 5px);
+  margin: 0 10px 0 0;
+}
+
+.compare-query-version {
+  background-color: #f5f5f5;
+  padding: 5px;
+  border: 1px solid #ccc;
+  margin-right: 15px;
+  border-radius: 3px;
+}
+
+.diff-content {
+  border: 1px solid #ccc;
+  background-color: #f5f5f5;
+  border-radius: 3px;
+  padding: 15px;
+}
+
+.query-diff-container > div:last-child {
+  margin: 0;
+}
+
+.compare-query-version-controls {
+  display: flex;
+  align-items: center;
+  margin-bottom: 25px;
+}

client/app/pages/queries/compare-query-dialog.html

@@ -0,0 +1,33 @@
+<div class="modal-header">
+    <button type="button" class="close" aria-label="Close" ng-click="$ctrl.close()">
+        <span aria-hidden="true">&times;</span>
+    </button>
+    <h4 class="modal-title">Compare Query</h4>
+</div>
+<div class="modal-body query-diff-container">
+    <div class="compare-query-version-controls">
+        <div class="compare-query-version">
+            <span>Compare current version to</span>
+            <select id="version-choice" ng-model="compareQueryVersion" ng-change="$ctrl.compareQueries()">
+                <option ng-repeat="version in $ctrl.versions.slice(0, $ctrl.versions.length-1)" value="{{version.object_version}}">Version {{version.object_version}}</option>
+            </select>
+        </div>
+        <div class="compare-query-revert-wrapper hidden"><a ng-click="$ctrl.revertQuery()" class="btn btn-default">Revert to version {{$ctrl.previousQueryVersion + 1}}</a></div>
+    </div>
+    <div>
+        <h5>Current Version {{$ctrl.currentQuery.version}}</h5>
+        <div class="diff-content">
+            <p id="current-query-diff">
+                <span ng-class="{'diff-added': part.added, 'diff-removed': part.removed}" ng-repeat="part in $ctrl.currentDiff">
+                    {{part.value}}
+                </span>
+            </p>
+        </div>
+    </div>
+    <div>
+        <h5>Previous Version {{$ctrl.previousQueryVersion + 1}}</h5>
+        <div class="diff-content">
+            <p>{{$ctrl.previousQuery}}</p>
+        </div>
+    </div>
+</div>

client/app/pages/queries/compare-query-dialog.js

@@ -0,0 +1,66 @@
+import * as jsDiff from "diff";
+import template from "./compare-query-dialog.html";
+import "./compare-query-dialog.css";
+
+const CompareQueryDialog = {
+  controller(clientConfig, $http) {
+    "ngInject";
+
+    this.currentQuery = this.resolve.query;
+    this.previousQuery = "";
+    this.currentDiff = [];
+    this.previousDiff = [];
+    this.versions = [];
+    this.previousQueryVersion = this.currentQuery.version - 2; // due to 0-indexed versions[]
+
+    this.compareQueries = isInitialLoad => {
+      if (!isInitialLoad) {
+        this.previousQueryVersion = document.getElementById("version-choice").value - 1; // due to 0-indexed versions[]
+      }
+
+      this.previousQuery = this.versions[this.previousQueryVersion].change.query.current;
+      this.currentDiff = jsDiff.diffChars(this.previousQuery, this.currentQuery.query);
+      document.querySelector(".compare-query-revert-wrapper").classList.remove("hidden");
+    };
+
+    this.revertQuery = () => {
+      this.resolve.query.query = this.previousQuery;
+      this.resolve.saveQuery();
+
+      // Close modal.
+      this.dismiss();
+    };
+
+    $http.get(`/api/queries/${this.currentQuery.id}/version`).then(response => {
+      this.versions = response.data;
+
+      const compare = (a, b) => {
+        if (a.object_version < b.object_version) {
+          return -1;
+        } else if (a.object_version > b.object_version) {
+          return 1;
+        }
+        return 0;
+      };
+
+      this.versions.sort(compare);
+      this.compareQueries(true);
+    });
+  },
+  scope: {
+    query: "=",
+    saveQuery: "<",
+  },
+  bindings: {
+    resolve: "<",
+    close: "&",
+    dismiss: "&",
+  },
+  template,
+};
+
+export default function init(ngModule) {
+  ngModule.component("compareQueryDialog", CompareQueryDialog);
+}
+
+init.init = true;

client/app/pages/queries/query.html

@@ -74,6 +74,9 @@ <h3>
                 <li ng-if="query.id != undefined">
                   <a ng-click="showApiKey()">Show API Key</a>
                 </li>
+                <li ng-show="canEdit" ng-if="query.id && (query.version > 1)">
+                  <a ng-click="compareQueryVersion()">Query Versions</a>
+                </li>
               </ul>
             </div>
           </div>

client/app/pages/queries/view.js

@@ -415,6 +415,21 @@ function QueryViewCtrl(
     });
   };
 
+  $scope.compareQueryVersion = () => {
+    if (!$scope.query.query) {
+      return;
+    }
+
+    $uibModal.open({
+      windowClass: "modal-xl",
+      component: "compareQueryDialog",
+      resolve: {
+        query: $scope.query,
+        saveQuery: () => $scope.saveQuery,
+      },
+    });
+  };
+
   $scope.$watch("query.name", () => {
     Title.set($scope.query.name);
   });

package.json

@@ -58,6 +58,7 @@
     "d3": "^3.5.17",
     "d3-cloud": "^1.2.4",
     "debug": "^3.1.0",
+    "diff": "^3.3.0",
     "font-awesome": "^4.7.0",
     "hoist-non-react-statics": "^3.3.0",
     "jquery": "^3.2.1",

redash/handlers/api.py

@@ -46,6 +46,7 @@
     ObjectPermissionsListResource,
 )
 from redash.handlers.queries import (
+    ChangeResource,
     MyQueriesResource,
     QueryArchiveResource,
     QueryFavoriteListResource,
@@ -57,6 +58,7 @@
     QuerySearchResource,
     QueryTagsResource,
     QueryRegenerateApiKeyResource,
+    QueryVersionListResource,
 )
 from redash.handlers.query_results import (
     JobResource,
@@ -221,6 +223,12 @@ def json_representation(data, code, headers=None):
     "/api/queries/<query_id>/regenerate_api_key",
     endpoint="query_regenerate_api_key",
 )
+api.add_org_resource(
+    QueryVersionListResource,
+    "/api/queries/<query_id>/version",
+    endpoint="query_versions",
+)
+api.add_org_resource(ChangeResource, "/api/changes/<change_id>", endpoint="changes")
 
 api.add_org_resource(
     ObjectPermissionsListResource,

redash/handlers/dashboards.py

@@ -105,6 +105,7 @@ def post(self):
             is_draft=True,
             layout="[]",
         )
+        dashboard.record_changes(changed_by=self.current_user)
         models.db.session.add(dashboard)
         models.db.session.commit()
         return serialize_dashboard(dashboard)

redash/handlers/queries.py

@@ -256,6 +256,7 @@ def post(self):
         query_def["org"] = self.current_org
         query_def["is_draft"] = True
         query = models.Query.create(**query_def)
+        query.record_changes(changed_by=self.current_user)
         models.db.session.add(query)
         models.db.session.commit()
 
@@ -376,6 +377,7 @@ def post(self, query_id):
 
         try:
             self.update_model(query, query_def)
+            query.record_changes(self.current_user)
             models.db.session.commit()
         except StaleDataError:
             abort(409)
@@ -549,3 +551,18 @@ def get(self):
         )
 
         return response
+
+
+class QueryVersionListResource(BaseResource):
+    @require_permission("view_query")
+    def get(self, query_id):
+        results = models.Change.query.filter(
+            models.Change.object_id == query_id, models.Change.object_type == "queries",
+        )
+        return [q.to_dict() for q in results]
+
+
+class ChangeResource(BaseResource):
+    @require_permission("view_query")
+    def get(self, change_id):
+        return models.Change.query.get(change_id).to_dict()

redash/models/__init__.py

@@ -441,7 +441,7 @@ def should_schedule_next(
 )
 class Query(ChangeTrackingMixin, TimestampMixin, BelongsToOrgMixin, db.Model):
     id = Column(db.Integer, primary_key=True)
-    version = Column(db.Integer, default=1)
+    version = Column(db.Integer, default=0)
     org_id = Column(db.Integer, db.ForeignKey("organizations.id"))
     org = db.relationship(Organization, backref="queries")
     data_source_id = Column(db.Integer, db.ForeignKey("data_sources.id"), nullable=True)
@@ -767,6 +767,7 @@ def fork(self, user):
         forked_query = Query(
             name="Copy of (#{}) {}".format(self.id, self.name), user=user, **kwargs
         )
+        forked_query.record_changes(changed_by=user)
 
         for v in sorted(self.visualizations, key=lambda v: v.id):
             forked_v = v.copy()

redash/models/changes.py

@@ -22,7 +22,6 @@ def to_dict(self, full=True):
             "id": self.id,
             "object_id": self.object_id,
             "object_type": self.object_type,
-            "change_type": self.change_type,
             "object_version": self.object_version,
             "change": self.change,
             "created_at": self.created_at,
@@ -50,44 +49,40 @@ class ChangeTrackingMixin(object):
     skipped_fields = ("id", "created_at", "updated_at", "version")
     _clean_values = None
 
-    def __init__(self, *a, **kw):
-        super(ChangeTrackingMixin, self).__init__(*a, **kw)
-        self.record_changes(self.user)
-
     def prep_cleanvalues(self):
         self.__dict__["_clean_values"] = {}
         for attr in inspect(self.__class__).column_attrs:
-            col, = attr.columns
+            (col,) = attr.columns
             # 'query' is col name but not attr name
             self._clean_values[col.name] = None
 
     def __setattr__(self, key, value):
         if self._clean_values is None:
             self.prep_cleanvalues()
-        for attr in inspect(self.__class__).column_attrs:
-            col, = attr.columns
-            previous = getattr(self, attr.key, None)
-            self._clean_values[col.name] = previous
-
+        if key in inspect(self.__class__).column_attrs:
+            previous = getattr(self, key, None)
+            self._clean_values[key] = previous
         super(ChangeTrackingMixin, self).__setattr__(key, value)
 
     def record_changes(self, changed_by):
         db.session.add(self)
         db.session.flush()
         changes = {}
         for attr in inspect(self.__class__).column_attrs:
-            col, = attr.columns
+            (col,) = attr.columns
             if attr.key not in self.skipped_fields:
-                changes[col.name] = {
-                    "previous": self._clean_values[col.name],
-                    "current": getattr(self, attr.key),
-                }
-
-        db.session.add(
-            Change(
+                prev = self._clean_values[col.name]
+                current = getattr(self, attr.key)
+                if prev != current:
+                    changes[col.name] = {"previous": prev, "current": current}
+
+        if changes:
+            self.version = (self.version or 0) + 1
+            change = Change(
                 object=self,
                 object_version=self.version,
                 user=changed_by,
                 change=changes,
             )
-        )
+            db.session.add(change)
+            return change

tests/handlers/test_queries.py

@@ -510,3 +510,30 @@ def test_format_sql_query(self):
         )
 
         self.assertEqual(rv.json["query"], expected)
+
+
+class ChangeResourceTests(BaseTestCase):
+    def test_list(self):
+        query = self.factory.create_query()
+        query.name = "version A"
+        query.record_changes(self.factory.user)
+        query.name = "version B"
+        query.record_changes(self.factory.user)
+        rv = self.make_request("get", "/api/queries/{}/version".format(query.id))
+        self.assertEquals(rv.status_code, 200)
+        self.assertEquals(len(rv.json), 2)
+        self.assertEquals(rv.json[0]["change"]["name"]["current"], "version A")
+        self.assertEquals(rv.json[1]["change"]["name"]["current"], "version B")
+
+    def test_get(self):
+        query = self.factory.create_query()
+        query.name = "version A"
+        ch1 = query.record_changes(self.factory.user)
+        query.name = "version B"
+        ch2 = query.record_changes(self.factory.user)
+        rv1 = self.make_request("get", "/api/changes/" + str(ch1.id))
+        self.assertEqual(rv1.status_code, 200)
+        self.assertEqual(rv1.json["change"]["name"]["current"], "version A")
+        rv2 = self.make_request("get", "/api/changes/" + str(ch2.id))
+        self.assertEqual(rv2.status_code, 200)
+        self.assertEqual(rv2.json["change"]["name"]["current"], "version B")