Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve drafts UX #1547

Merged
merged 5 commits into from
Jan 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/app/pages/dashboards/dashboard-list.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions client/app/pages/dashboards/dashboard.html
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
4 changes: 2 additions & 2 deletions client/app/pages/home/home.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
7 changes: 0 additions & 7 deletions client/app/pages/queries-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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' },
];
}
}
Expand All @@ -70,6 +64,5 @@ export default function (ngModule) {
return {
'/queries': route,
'/queries/my': route,
'/queries/drafts': route,
};
}
2 changes: 1 addition & 1 deletion client/app/pages/queries-list/queries-list.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/queries/queries-search-results-page.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/queries/queries-search-results-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions client/app/pages/queries/query.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions redash/handlers/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down
25 changes: 15 additions & 10 deletions redash/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand All @@ -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)))
Expand All @@ -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(
Expand All @@ -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))))
Expand Down Expand Up @@ -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
Expand All @@ -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)))
Expand Down
14 changes: 7 additions & 7 deletions tests/models/test_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 4 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ var config = {
target: redashBackend + '/',
secure: false
},
'/invite': {
target: redashBackend + '/',
secure: false
},
'/setup': {
target: redashBackend + '/',
secure: false
Expand Down