From 65a6385380a7ed1f7779a4198a9b8430fa3bf887 Mon Sep 17 00:00:00 2001 From: Allen Short Date: Fri, 14 Oct 2016 17:25:30 -0500 Subject: [PATCH] Make draft status for queries and dashboards toggleable. --- migrations/0027_add_draft_toggle.py | 18 ++++++ rd_ui/app/scripts/controllers/dashboard.js | 11 ++++ rd_ui/app/scripts/controllers/query_view.js | 8 ++- rd_ui/app/views/dashboard.html | 5 ++ rd_ui/app/views/query.html | 5 ++ redash/handlers/dashboards.py | 16 +++--- redash/handlers/queries.py | 1 + redash/models.py | 63 +++++++++++++-------- tests/factories.py | 2 + tests/handlers/test_queries.py | 2 +- tests/test_models.py | 26 +++++++++ 11 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 migrations/0027_add_draft_toggle.py diff --git a/migrations/0027_add_draft_toggle.py b/migrations/0027_add_draft_toggle.py new file mode 100644 index 0000000000..0a07b14abe --- /dev/null +++ b/migrations/0027_add_draft_toggle.py @@ -0,0 +1,18 @@ +from playhouse.migrate import PostgresqlMigrator, migrate + +from redash.models import db +from redash import models + +if __name__ == '__main__': + db.connect_db() + migrator = PostgresqlMigrator(db.database) + + with db.database.transaction(): + migrate( + migrator.add_column('queries', 'is_draft', models.Query.is_draft) + ) + migrate( + migrator.add_column('dashboards', 'is_draft', models.Query.is_draft) + ) + db.database.execute_sql("UPDATE queries SET is_draft = (name = 'New Query')") + db.close_db(None) diff --git a/rd_ui/app/scripts/controllers/dashboard.js b/rd_ui/app/scripts/controllers/dashboard.js index 7c51b1085c..977050e042 100644 --- a/rd_ui/app/scripts/controllers/dashboard.js +++ b/rd_ui/app/scripts/controllers/dashboard.js @@ -129,6 +129,17 @@ }); }; + $scope.togglePublished = function () { + Events.record(currentUser, "toggle_published", "dashboard", $scope.dashboard.id); + $scope.dashboard.is_draft = !$scope.dashboard.is_draft; + $scope.saveInProgress = true; + Dashboard.save({slug: $scope.dashboard.id, name: $scope.dashboard.name, + layout: JSON.stringify($scope.dashboard.layout), + is_draft: $scope.dashboard.is_draft}, + function() {$scope.saveInProgress = false;}); + + }; + $scope.toggleFullscreen = function() { $scope.isFullscreen = !$scope.isFullscreen; $('body').toggleClass('headless'); diff --git a/rd_ui/app/scripts/controllers/query_view.js b/rd_ui/app/scripts/controllers/query_view.js index 6182bde93b..462bbc13b3 100644 --- a/rd_ui/app/scripts/controllers/query_view.js +++ b/rd_ui/app/scripts/controllers/query_view.js @@ -132,7 +132,7 @@ data.id = $scope.query.id; data.version = $scope.query.version; } else { - data = _.pick($scope.query, ["schedule", "query", "id", "description", "name", "data_source_id", "options", "latest_query_data_id", "version"]); + data = _.pick($scope.query, ["schedule", "query", "id", "description", "name", "data_source_id", "options", "latest_query_data_id", "version", "is_draft"]); } options = _.extend({}, { @@ -163,6 +163,12 @@ $scope.saveQuery(undefined, {'name': $scope.query.name}); }; + $scope.togglePublished = function() { + Events.record(currentUser, 'toggle_published', 'query', $scope.query.id); + $scope.query.is_draft = !$scope.query.is_draft; + $scope.saveQuery(undefined, {'is_draft': $scope.query.is_draft}); + }; + $scope.executeQuery = function() { if (!$scope.canExecuteQuery()) { return; diff --git a/rd_ui/app/views/dashboard.html b/rd_ui/app/views/dashboard.html index 055903b435..a4b093afda 100644 --- a/rd_ui/app/views/dashboard.html +++ b/rd_ui/app/views/dashboard.html @@ -22,6 +22,8 @@
  • Edit Dashboard
  • Add Widget
  • Manage Permissions
    • +
  • Unpublish Dashboard
    • +
  • Publish Dashboard
  • Archive Dashboard
    • @@ -29,6 +31,9 @@
    This dashboard is archived and won't appear in the dashboards list or search results.
    +
    + This dashboard is a draft. +
    diff --git a/rd_ui/app/views/query.html b/rd_ui/app/views/query.html index ed8ead1361..a5f6e738bf 100644 --- a/rd_ui/app/views/query.html +++ b/rd_ui/app/views/query.html @@ -91,6 +91,9 @@

    This query is archived and can't be used in dashboards, and won't appear in search results.
    +
    + This query is a draft. +

    @@ -126,6 +129,8 @@

    diff --git a/redash/handlers/dashboards.py b/redash/handlers/dashboards.py index 01b36f6843..39109af670 100644 --- a/redash/handlers/dashboards.py +++ b/redash/handlers/dashboards.py @@ -31,13 +31,12 @@ def get(self): @require_permission('create_dashboard') def post(self): dashboard_properties = request.get_json(force=True) - dashboard = models.Dashboard.create(name=dashboard_properties['name'], - org=self.current_org, - user=self.current_user, - layout='[]') - - result = dashboard.to_dict() - return result + dashboard = models.Dashboard(name=dashboard_properties['name'], + org=self.current_org, + user=self.current_user, + is_draft=True, + layout='[]') + return dashboard.to_dict() class DashboardResource(BaseResource): @@ -63,7 +62,8 @@ def post(self, dashboard_slug): require_object_modify_permission(dashboard, self.current_user) - updates = project(dashboard_properties, ('name', 'layout', 'version')) + updates = project(dashboard_properties, ('name', 'layout', 'version', + 'is_draft')) updates['changed_by'] = self.current_user try: diff --git a/redash/handlers/queries.py b/redash/handlers/queries.py index 4879e63f0b..38d4251229 100644 --- a/redash/handlers/queries.py +++ b/redash/handlers/queries.py @@ -62,6 +62,7 @@ def post(self): query_def['user'] = self.current_user query_def['data_source'] = data_source query_def['org'] = self.current_org + query_def['is_draft'] = True query = models.Query.create(**query_def) self.record_event({ diff --git a/redash/models.py b/redash/models.py index 4dd77eacb8..54ae72e51a 100644 --- a/redash/models.py +++ b/redash/models.py @@ -702,6 +702,7 @@ class Query(ChangeTrackingMixin, ModelTimestampsMixin, BaseVersionedModel, Belon user = peewee.ForeignKeyField(User) last_modified_by = peewee.ForeignKeyField(User, null=True, related_name="modified_queries") is_archived = peewee.BooleanField(default=False, index=True) + is_draft = peewee.BooleanField(default=True, index=True) schedule = peewee.CharField(max_length=10, null=True) options = JSONField(default={}) @@ -719,6 +720,7 @@ def to_dict(self, with_stats=False, with_visualizations=False, with_user=True, w 'schedule': self.schedule, 'api_key': self.api_key, 'is_archived': self.is_archived, + 'is_draft': self.is_draft, 'updated_at': self.updated_at, 'created_at': self.created_at, 'data_source_id': self.data_source_id, @@ -771,10 +773,9 @@ def all_queries(cls, groups, drafts=False): .order_by(cls.created_at.desc()) if drafts: - q = q.where(Query.name == 'New Query') + q = q.where(Query.is_draft == True) else: - q = q.where(Query.name != 'New Query') - + q = q.where(Query.is_draft == False) return q @classmethod @@ -818,17 +819,22 @@ def search(cls, term, groups): @classmethod def recent(cls, groups, user_id=None, limit=20): - query = cls.select(Query, User).where(Event.created_at > peewee.SQL("current_date - 7")).\ - join(Event, on=(Query.id == Event.object_id.cast('integer'))). \ - join(DataSourceGroup, on=(Query.data_source==DataSourceGroup.data_source)). \ - switch(Query).join(User).\ - where(Event.action << ('edit', 'execute', 'edit_name', 'edit_description', 'view_source')).\ - where(~(Event.object_id >> None)).\ - where(Event.object_type == 'query'). \ - where(DataSourceGroup.group << groups).\ - where(cls.is_archived == False).\ - group_by(Event.object_id, Query.id, User.id).\ - order_by(peewee.SQL("count(0) desc")) + query = ( + cls.select(Query, User) + .where(Event.created_at > peewee.SQL("current_date - 7")) + .join(Event, on=(Query.id == Event.object_id.cast('integer'))) + .join(DataSourceGroup, on=(Query.data_source==DataSourceGroup.data_source)) + .switch(Query).join(User) + .where(Event.action << ('edit', 'execute', 'edit_name', + 'edit_description', 'toggle_published', + 'view_source')) + .where(~(Event.object_id >> None)) + .where(Event.object_type == 'query') + .where(DataSourceGroup.group << groups) + .where(cls.is_archived == False) + .where(cls.is_draft == False) + .group_by(Event.object_id, Query.id, User.id) + .order_by(peewee.SQL("count(0) desc"))) if user_id: query = query.where(Event.user == user_id) @@ -1077,6 +1083,7 @@ class Dashboard(ChangeTrackingMixin, ModelTimestampsMixin, BaseVersionedModel, B layout = peewee.TextField() dashboard_filters_enabled = peewee.BooleanField(default=False) is_archived = peewee.BooleanField(default=False, index=True) + is_draft = peewee.BooleanField(default=False, index=True) class Meta: db_table = 'dashboards' @@ -1129,6 +1136,7 @@ def to_dict(self, with_widgets=False, user=None): 'dashboard_filters_enabled': self.dashboard_filters_enabled, 'widgets': widgets_layout, 'is_archived': self.is_archived, + 'is_draft': self.is_draft, 'updated_at': self.updated_at, 'created_at': self.created_at, 'version': self.version @@ -1136,17 +1144,21 @@ def to_dict(self, with_widgets=False, user=None): @classmethod def all(cls, org, groups, user_id): - query = cls.select().\ - join(Widget, peewee.JOIN_LEFT_OUTER, on=(Dashboard.id == Widget.dashboard)). \ - join(Visualization, peewee.JOIN_LEFT_OUTER, on=(Widget.visualization == Visualization.id)). \ - join(Query, peewee.JOIN_LEFT_OUTER, on=(Visualization.query == Query.id)). \ - join(DataSourceGroup, peewee.JOIN_LEFT_OUTER, on=(Query.data_source == DataSourceGroup.data_source)). \ - where(Dashboard.is_archived == False). \ - where((DataSourceGroup.group << groups) | - (Dashboard.user == user_id) | - (~(Widget.dashboard >> None) & (Widget.visualization >> None))). \ - where(Dashboard.org == org). \ - group_by(Dashboard.id) + query = (cls.select() + .join(Widget, peewee.JOIN_LEFT_OUTER, + on=(Dashboard.id == Widget.dashboard)) + .join(Visualization, peewee.JOIN_LEFT_OUTER, + on=(Widget.visualization == Visualization.id)) + .join(Query, peewee.JOIN_LEFT_OUTER, + on=(Visualization.query == Query.id)) + .join(DataSourceGroup, peewee.JOIN_LEFT_OUTER, + on=(Query.data_source == DataSourceGroup.data_source)) + .where(Dashboard.is_archived == False) + .where((DataSourceGroup.group << groups & (Dashboard.is_draft != True)) | + (Dashboard.user == user_id) | + (~(Widget.dashboard >> None) & (Widget.visualization >> None))) + .where(Dashboard.org == org) + .group_by(Dashboard.id)) return query @@ -1162,6 +1174,7 @@ def recent(cls, org, groups, user_id, for_user=False, limit=20): where(~(Event.object_id >> None)). \ where(Event.object_type == 'dashboard'). \ where(Dashboard.is_archived == False). \ + where(Dashboard.is_draft == False). \ where(Dashboard.org == org). \ where((DataSourceGroup.group << groups) | (Dashboard.user == user_id) | diff --git a/tests/factories.py b/tests/factories.py index 524b4bf115..fe6c5a1b9e 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -69,6 +69,7 @@ def __call__(self): query='SELECT 1', user=user_factory.create, is_archived=False, + is_draft=False, schedule=None, data_source=data_source_factory.create, org=1) @@ -79,6 +80,7 @@ def __call__(self): query='SELECT {{param1}}', user=user_factory.create, is_archived=False, + is_draft=False, schedule=None, data_source=data_source_factory.create, org=1) diff --git a/tests/handlers/test_queries.py b/tests/handlers/test_queries.py index 75049926d1..ce7e24484c 100644 --- a/tests/handlers/test_queries.py +++ b/tests/handlers/test_queries.py @@ -90,7 +90,6 @@ def test_works_for_non_owner_with_permission(self): self.assertEqual(rv.json['last_modified_by']['id'], user.id) - class TestQueryListResourcePost(BaseTestCase): def test_create_query(self): query_data = { @@ -110,6 +109,7 @@ def test_create_query(self): query = models.Query.get_by_id(rv.json['id']) self.assertEquals(len(list(query.visualizations)), 1) + self.assertTrue(query.is_draft) class QueryRefreshTest(BaseTestCase): diff --git a/tests/test_models.py b/tests/test_models.py index a3814924b8..18eeaeacfe 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -129,6 +129,22 @@ def test_global_recent(self): self.assertIn(q1, recent) self.assertNotIn(q2, recent) + def test_recent_excludes_drafts(self): + q1 = self.factory.create_query() + q2 = self.factory.create_query(is_draft=True) + + models.Event.create(org=self.factory.org, user=self.factory.user, + action="edit", object_type="query", + object_id=q1.id) + models.Event.create(org=self.factory.org, user=self.factory.user, + action="edit", object_type="query", + object_id=q2.id) + + recent = models.Query.recent([self.factory.default_group]) + + self.assertIn(q1, recent) + self.assertNotIn(q2, recent) + def test_recent_for_user(self): q1 = self.factory.create_query() q2 = self.factory.create_query() @@ -657,6 +673,16 @@ def test_returns_recent_dashboards_basic(self): self.assertNotIn(self.w2.dashboard, models.Dashboard.recent(self.u1.org, self.u1.groups, None)) self.assertNotIn(self.w1.dashboard, models.Dashboard.recent(self.u1.org, self.u2.groups, None)) + def test_recent_excludes_drafts(self): + models.Event.create(org=self.factory.org, user=self.u1, action="view", + object_type="dashboard", object_id=self.w1.dashboard.id) + models.Event.create(org=self.factory.org, user=self.u1, action="view", + object_type="dashboard", object_id=self.w2.dashboard.id) + + self.w2.dashboard.update_instance(is_draft=True) + self.assertIn(self.w1.dashboard, models.Dashboard.recent(self.u1.org, self.u1.groups, None)) + self.assertNotIn(self.w2.dashboard, models.Dashboard.recent(self.u1.org, self.u1.groups, None)) + def test_returns_recent_dashboards_created_by_user(self): d1 = self.factory.create_dashboard(user=self.u1) models.Event.create(org=self.factory.org, user=self.u1, action="view",