Merge pull request #1547 from getredash/drafts

Improve drafts UX

client/app/pages/dashboards/dashboard-list.html

@@ -25,6 +25,7 @@ <h3 class='list-group-item m-0'>Tags</h3>
           <td>
             <a href="dashboard/{{ dashboard.slug }}">
               <span class="label label-primary m-2" ng-bind="tag" ng-repeat="tag in dashboard.tags"></span> {{ dashboard.untagged_name }}
+              <span class="label label-warning" ng-if="dashboard.is_draft">Unpublished</span>
             </a>
           </td>
           <td>{{ dashboard.created_at | dateTime }}</td>

client/app/pages/dashboards/dashboard.html

@@ -1,7 +1,7 @@
 <div class="container">
-  <div class="row bg-white p-10 p-l-15 p-r-15 m-b-10">
+  <div class="row bg-white p-t-10 p-b-10 m-b-10">
     <div class="col-sm-9">
-      <h3>{{$ctrl.dashboard.name}} <span class="label label-warning" ng-if="$ctrl.dashboard.is_draft">Draft</span></h3>
+      <h3>{{$ctrl.dashboard.name}} <span class="label label-warning" ng-if="$ctrl.dashboard.is_draft">Unpublished</span></h3>
     </div>
     <div class="col-sm-3 text-right">
       <h3>

client/app/pages/home/home.html

@@ -15,7 +15,7 @@
             <p class="f-500 m-b-20 c-black">Recent Dashboards</p>
             <div class="list-group">
               <a ng-href="dashboard/{{dashboard.slug}}" class="list-group-item" ng-repeat="dashboard in $ctrl.recentDashboards">
-                {{dashboard.name}}
+                {{dashboard.name}} <span class="label label-warning" ng-if="dashboard.is_draft">Unpublished</span>
               </a>
             </div>
           </div>
@@ -24,7 +24,7 @@
             <p class="f-500 m-b-20 c-black">Recent Queries</p>
             <div class="list-group">
               <a ng-href="queries/{{query.id}}" class="list-group-item"
-                 ng-repeat="query in $ctrl.recentQueries">{{query.name}}</a>
+                 ng-repeat="query in $ctrl.recentQueries">{{query.name}} <span class="label label-warning" ng-if="query.is_draft">Unpublished</span></a>
             </div>
           </div>
       </div>

client/app/pages/queries-list/index.js

@@ -16,11 +16,6 @@ class QueriesListCtrl {
         Title.set('Queries');
         this.resource = Query.query;
         break;
-      case '/queries/drafts':
-        Title.set('Draft Queries');
-        this.resource = Query.myQueries;
-        this.defaultOptions.drafts = true;
-        break;
       case '/queries/my':
         Title.set('My Queries');
         this.resource = Query.myQueries;
@@ -51,7 +46,6 @@ class QueriesListCtrl {
     this.tabs = [
       { name: 'My Queries', path: 'queries/my' },
       { path: 'queries', name: 'All Queries', isActive: path => path === '/queries' },
-      { path: 'queries/drafts', name: 'Drafts' },
     ];
   }
 }
@@ -70,6 +64,5 @@ export default function (ngModule) {
   return {
     '/queries': route,
     '/queries/my': route,
-    '/queries/drafts': route,
   };
 }

client/app/pages/queries-list/queries-list.html

@@ -16,7 +16,7 @@
       </thead>
       <tbody>
       <tr ng-repeat="query in $ctrl.paginator.getPageRows()">
-        <td><a href="queries/{{query.id}}">{{query.name}}</a></td>
+        <td><a href="queries/{{query.id}}">{{query.name}}</a> <span class="label label-warning" ng-if="query.is_draft">Unpublished</span></td>
         <td>{{query.user.name}}</td>
         <td>{{query.created_at | dateTime}}</td>
         <td>{{query.runtime | durationHumanize}}</td>

client/app/pages/queries/queries-search-results-page.html

@@ -21,7 +21,7 @@
         </thead>
         <tbody>
           <tr ng-repeat="query in $ctrl.paginator.getPageRows()">
-            <td><a href="queries/{{query.id}}">{{query.name}}</a></td>
+            <td><a href="queries/{{query.id}}">{{query.name}}</a> <span class="label label-warning" ng-if="query.is_draft">Unpublished</span></td>
             <td>{{query.user.name}}</td>
             <td>{{query.created_at | dateTime}}</td>
             <td>{{query.schedule | scheduleHumanize}}</td>

client/app/pages/queries/queries-search-results-page.js

@@ -8,7 +8,7 @@ function QuerySearchCtrl($location, $filter, currentUser, Events, Query) {
   this.term = $location.search().q;
   this.paginator = new Paginator([], { itemsPerPage: 20 });
 
-  Query.search({ q: this.term }, (results) => {
+  Query.search({ q: this.term, include_drafts: true }, (results) => {
     const queries = results.map((query) => {
       query.created_at = moment(query.created_at);
       return query;

client/app/pages/queries/query.html

@@ -21,7 +21,7 @@
     <div class="col-sm-9">
       <h3>
         <edit-in-place editable="canEdit" done="saveName" ignore-blanks="true" value="query.name"></edit-in-place>
-        <span class="label label-warning" ng-if="query.is_draft">Draft</span>
+        <span class="label label-warning" ng-if="query.is_draft">Unpublished</span>
       </h3>
       <p>
         <em>
@@ -102,6 +102,9 @@ <h3>
                       ng-options="ds.id as ds.name for ds in dataSources"></select>
 
               <div class="pull-right">
+                <button class="btn btn-s btn-default" ng-click="togglePublished()" ng-if="query.is_draft && query.id != undefined && (isQueryOwner || currentUser.hasPermission('admin'))">
+                  <span class="fa fa-paper-plane"></span> Publish
+                </button>
                 <button class="btn btn-s btn-default" ng-click="duplicateQuery()" ng-disabled="query.id === undefined">
                   <span class="zmdi zmdi-arrow-split"></span> Fork
                 </button>
@@ -116,7 +119,6 @@ <h3>
                   <ul class="dropdown-menu pull-right" uib-dropdown-menu>
                     <li ng-if="!query.is_archived && query.id != undefined && (isQueryOwner || currentUser.hasPermission('admin'))"><a ng-click="archiveQuery()">Archive Query</a></li>
                     <li ng-if="!query.is_archived && query.id != undefined && (isQueryOwner || currentUser.hasPermission('admin')) && showPermissionsControl"><a ng-click="showManagePermissionsModal()">Manage Permissions</a></li>
-                    <li ng-if="query.is_draft && query.id != undefined && (isQueryOwner || currentUser.hasPermission('admin'))"><a ng-click="togglePublished()">Publish Query</a></li>
                     <li ng-if="!query.is_draft && query.id != undefined && (isQueryOwner || currentUser.hasPermission('admin'))"><a ng-click="togglePublished()">Unpublish Query</a></li>
                     <li ng-if="query.id != undefined"><a ng-click="showApiKey()">Show API Key</a></li>
                   </ul>

redash/handlers/queries.py

@@ -31,8 +31,9 @@ class QuerySearchResource(BaseResource):
     @require_permission('view_query')
     def get(self):
         term = request.args.get('q', '')
+        include_drafts = request.args.get('include_drafts') is not None
 
-        return [q.to_dict(with_last_modified_by=False) for q in models.Query.search(term, self.current_user.group_ids)]
+        return [q.to_dict(with_last_modified_by=False) for q in models.Query.search(term, self.current_user.group_ids, include_drafts=include_drafts)]
 
 
 class QueryRecentResource(BaseResource):
@@ -77,7 +78,7 @@ def post(self):
 
     @require_permission('view_query')
     def get(self):
-        results = models.Query.all_queries(self.current_user.group_ids)
+        results = models.Query.all_queries(self.current_user.group_ids, self.current_user.id)
         page = request.args.get('page', 1, type=int)
         page_size = request.args.get('page_size', 25, type=int)
         return paginate(results, page, page_size, lambda q: q.to_dict(with_stats=True, with_last_modified_by=False))
@@ -86,8 +87,7 @@ def get(self):
 class MyQueriesResource(BaseResource):
     @require_permission('view_query')
     def get(self):
-        drafts = request.args.get('drafts') is not None
-        results = models.Query.by_user(self.current_user, drafts)
+        results = models.Query.by_user(self.current_user)
         page = request.args.get('page', 1, type=int)
         page_size = request.args.get('page_size', 25, type=int)
         return paginate(results, page, page_size, lambda q: q.to_dict(with_stats=True, with_last_modified_by=False))

redash/models.py

@@ -16,6 +16,7 @@
 from sqlalchemy.orm import object_session, backref
 # noinspection PyUnresolvedReferences
 from sqlalchemy.orm.exc import NoResultFound
+from sqlalchemy import or_
 
 from passlib.apps import custom_app_context as pwd_context
 
@@ -758,7 +759,7 @@ def create(cls, **kwargs):
         return query
 
     @classmethod
-    def all_queries(cls, group_ids, drafts=False):
+    def all_queries(cls, group_ids, user_id=None, drafts=False):
         q = (cls.query.join(User, Query.user_id == User.id)
             .outerjoin(QueryResult)
             .join(DataSourceGroup, Query.data_source_id == DataSourceGroup.data_source_id)
@@ -767,16 +768,14 @@ def all_queries(cls, group_ids, drafts=False):
             .group_by(Query.id, User.id, QueryResult.id, QueryResult.retrieved_at, QueryResult.runtime)
             .order_by(Query.created_at.desc()))
 
-        if drafts:
-            q = q.filter(Query.is_draft == True)
-        else:
-            q = q.filter(Query.is_draft == False)
+        if not drafts:
+            q = q.filter(or_(Query.is_draft == False, Query.user_id == user_id))
 
         return q
 
     @classmethod
-    def by_user(cls, user, drafts):
-        return cls.all_queries(user.group_ids, drafts).filter(Query.user == user)
+    def by_user(cls, user):
+        return cls.all_queries(user.group_ids, user.id).filter(Query.user == user)
 
     @classmethod
     def outdated_queries(cls):
@@ -795,7 +794,7 @@ def outdated_queries(cls):
         return outdated_queries.values()
 
     @classmethod
-    def search(cls, term, group_ids):
+    def search(cls, term, group_ids, include_drafts=False):
         # TODO: This is very naive implementation of search, to be replaced with PostgreSQL full-text-search solution.
         where = (Query.name.ilike(u"%{}%".format(term)) |
                  Query.description.ilike(u"%{}%".format(term)))
@@ -804,6 +803,10 @@ def search(cls, term, group_ids):
             where |= Query.id == term
 
         where &= Query.is_archived == False
+
+        if not include_drafts:
+            where &= Query.is_draft == False
+
         where &= DataSourceGroup.group_id.in_(group_ids)
         query_ids = (
             db.session.query(Query.id).join(
@@ -826,7 +829,7 @@ def recent(cls, group_ids, user_id=None, limit=20):
                      Event.object_id != None,
                      Event.object_type == 'query',
                      DataSourceGroup.group_id.in_(group_ids),
-                     Query.is_draft == False,
+                     or_(Query.is_draft == False, Query.user_id == user_id),
                      Query.is_archived == False)
                  .group_by(Event.object_id, Query.id, User.id)
                  .order_by(db.desc(db.func.count(0))))
@@ -1187,6 +1190,8 @@ def all(cls, org, group_ids, user_id):
                 Dashboard.org == org)
             .group_by(Dashboard.id))
 
+        query = query.filter(or_(Dashboard.user_id == user_id, Dashboard.is_draft == False))
+
         return query
 
     @classmethod
@@ -1204,7 +1209,7 @@ def recent(cls, org, group_ids, user_id, for_user=False, limit=20):
                      Event.object_type == 'dashboard',
                      Dashboard.org == org,
                      Dashboard.is_archived == False,
-                     Dashboard.is_draft == False,
+                     or_(Dashboard.is_draft == False, Dashboard.user_id == user_id),
                      DataSourceGroup.group_id.in_(group_ids) |
                      (Dashboard.user_id == user_id) |
                      ((Widget.dashboard != None) & (Widget.visualization == None)))

tests/models/test_queries.py

@@ -165,17 +165,17 @@ def test_returns_only_users_queries(self):
         q = self.factory.create_query(user=self.factory.user)
         q2 = self.factory.create_query(user=self.factory.create_user())
 
-        queries = Query.by_user(self.factory.user, False)
+        queries = Query.by_user(self.factory.user)
 
         # not using self.assertIn/NotIn because otherwise this fails :O
-        self.assertTrue(q in queries)
-        self.assertFalse(q2 in queries)
+        self.assertTrue(q in list(queries))
+        self.assertFalse(q2 in list(queries))
 
-    def test_returns_drafts_if_asked_to(self):
+    def test_returns_drafts_by_the_user(self):
         q = self.factory.create_query(is_draft=True)
-        q2 = self.factory.create_query(is_draft=False)
+        q2 = self.factory.create_query(is_draft=True, user=self.factory.create_user())
 
-        queries = Query.by_user(self.factory.user, True)
+        queries = Query.by_user(self.factory.user)
 
         # not using self.assertIn/NotIn because otherwise this fails :O
         self.assertTrue(q in queries)
@@ -185,7 +185,7 @@ def test_returns_only_queries_from_groups_the_user_is_member_in(self):
         q = self.factory.create_query()
         q2 = self.factory.create_query(data_source=self.factory.create_data_source(group=self.factory.create_group()))
 
-        queries = Query.by_user(self.factory.user, False)
+        queries = Query.by_user(self.factory.user)
 
         # not using self.assertIn/NotIn because otherwise this fails :O
         self.assertTrue(q in queries)

tests/test_models.py

@@ -284,6 +284,14 @@ def test_returns_only_queries_in_given_groups(self):
         self.assertIn(q1, list(models.Query.all_queries([group1.id, group2.id])))
         self.assertIn(q2, list(models.Query.all_queries([group1.id, group2.id])))
 
+    def test_skips_drafts(self):
+        q = self.factory.create_query(is_draft=True)
+        self.assertNotIn(q, models.Query.all_queries([self.factory.default_group.id]))
+
+    def test_includes_drafts_of_given_user(self):
+        q = self.factory.create_query(is_draft=True)
+        self.assertIn(q, models.Query.all_queries([self.factory.default_group.id], user_id=q.user_id))
+
 
 class TestGroup(BaseTestCase):
     def test_returns_groups_with_specified_names(self):

webpack.config.js

@@ -89,6 +89,10 @@ var config = {
         target: redashBackend + '/',
         secure: false
       },
+      '/invite': {
+        target: redashBackend + '/',
+        secure: false
+      },
       '/setup': {
         target: redashBackend + '/',
         secure: false