Show active and pending users separately (for admins)

client/app/components/items-list/LiveItemsList.jsx

@@ -1,4 +1,4 @@
-import { isString, isNil, isFunction, map, each } from 'lodash';
+import { isString, isNil, isFunction, map, each, debounce } from 'lodash';
 import React from 'react';
 import PropTypes from 'prop-types';
 import hoistNonReactStatics from 'hoist-non-react-statics';
@@ -90,7 +90,7 @@ export function wrap(WrappedComponent, { defaultOrderBy, getRequest, doRequest,
         paginator.orderBy(orderByField); // fetch data
         savedOrderByField = paginator.orderByField;
       };
-      this.state.updateSearch = (searchTerm) => {
+      this.state.updateSearch = debounce((searchTerm) => {
         // here we update state directly, but later `fetchData` will update it properly
         this.state.searchTerm = searchTerm;
         // in search mode ignore the ordering and use the ranking order
@@ -102,7 +102,7 @@ export function wrap(WrappedComponent, { defaultOrderBy, getRequest, doRequest,
           paginator.orderByField = null;
         }
         paginator.setPage(1); // fetch data
-      };
+      }, 200);
       this.state.updateSelectedTags = (selectedTags) => {
         // here we update state directly, but later `fetchData` will update it properly
         this.state.selectedTags = selectedTags;

client/app/components/items-list/components/Sidebar.jsx

@@ -15,6 +15,7 @@ export function SearchInput({ placeholder, value, showIcon, onChange }) {
   return (
     <div className="m-b-10">
       <InputControl
+        className="form-control"
         placeholder={placeholder}
         defaultValue={value}
         onChange={event => onChange(event.target.value)}

client/app/pages/users/UsersList.jsx

@@ -61,10 +61,16 @@ class UsersList extends React.Component {
       href: 'users',
       title: 'Active Users',
     },
+    {
+      key: 'pending',
+      href: 'users/pending',
+      title: 'Pending Invitations',
+    },
     {
       key: 'disabled',
       href: 'users/disabled',
       title: 'Disabled Users',
+      isAvailable: () => policy.canCreateUser(),
     },
   ];
 
@@ -75,7 +81,6 @@ class UsersList extends React.Component {
         <div>
           <a href={'users/' + user.id} className="{'text-muted': user.is_disabled}">{user.name}</a>
           <div className="text-muted">{user.email}</div>
-          {user.is_invitation_pending && <span className="label label-tag-archived">Invitation Pending</span>}
         </div>
       </div>
     ), {
@@ -103,16 +108,18 @@ class UsersList extends React.Component {
     }),
     Columns.custom((text, user) => <UsersListActions user={user} actions={this.props.controller.actions} />, {
       width: '1%',
-      isAvailable: () => currentUser.isAdmin,
+      isAvailable: () => policy.canCreateUser(),
     }),
   ];
 
   onTableRowClick = (event, item) => navigateTo('users/' + item.id);
 
-  renderPageHeader(isAdminView) {
-    const { controller } = this.props;
-    return isAdminView ? (
-      // Admin
+  // eslint-disable-next-line class-methods-use-this
+  renderPageHeader() {
+    if (!policy.canCreateUser()) {
+      return null;
+    }
+    return (
       <div className="m-b-10">
         <a
           href="users/new"
@@ -123,31 +130,10 @@ class UsersList extends React.Component {
         </a>
         <DynamicComponent name="UsersListExtra" />
       </div>
-    ) : (
-      // Non-admin
-      <div className="row m-b-10">
-        <div className="col-xs-9 p-r-0">
-          <Sidebar.SearchInput
-            value={controller.searchTerm}
-            showIcon
-            onChange={controller.updateSearch}
-          />
-        </div>
-        <div className="col-xs-3">
-          <Sidebar.PageSizeSelect
-            options={controller.pageSizeOptions}
-            value={controller.itemsPerPage}
-            onChange={itemsPerPage => controller.updatePagination({ itemsPerPage })}
-          />
-        </div>
-      </div>
     );
   }
 
-  renderSidebar(isAdminView) {
-    if (!isAdminView) {
-      return null;
-    }
+  renderSidebar() {
     const { controller } = this.props;
     return (
       <React.Fragment>
@@ -166,15 +152,14 @@ class UsersList extends React.Component {
   }
 
   render() {
-    const isAdminView = policy.canCreateUser();
-    const sidebar = this.renderSidebar(isAdminView);
+    const sidebar = this.renderSidebar();
     const { controller } = this.props;
     return (
       <React.Fragment>
-        {this.renderPageHeader(isAdminView)}
+        {this.renderPageHeader()}
         <div className="row">
-          {isAdminView && <div className="col-md-3 list-control-t">{sidebar}</div>}
-          <div className={isAdminView ? 'list-content col-md-9' : 'col-md-12'}>
+          <div className="col-md-3 list-control-t">{sidebar}</div>
+          <div className="list-content col-md-9">
             {!controller.isLoaded && <LoadingState className="" />}
             {controller.isLoaded && controller.isEmpty && <EmptyState className="" />}
             {
@@ -199,7 +184,7 @@ class UsersList extends React.Component {
               )
             }
           </div>
-          {isAdminView && <div className="col-md-3 list-control-r-b">{sidebar}</div>}
+          <div className="col-md-3 list-control-r-b">{sidebar}</div>
         </div>
       </React.Fragment>
     );
@@ -218,8 +203,17 @@ export default function init(ngModule) {
   ngModule.component('pageUsersList', react2angular(liveItemsList(UsersList, {
     defaultOrderBy: '-created_at',
     getRequest(request, { currentPage }) {
-      if (currentPage === 'disabled') {
-        request.disabled = true;
+      switch (currentPage) {
+        case 'active':
+          request.pending = false;
+          break;
+        case 'pending':
+          request.pending = true;
+          break;
+        case 'disabled':
+          request.disabled = true;
+          break;
+        // no default
       }
       return request;
     },
@@ -256,6 +250,11 @@ export default function init(ngModule) {
       title: 'Users',
       key: 'active',
     },
+    {
+      path: '/users/pending',
+      title: 'Pending Invitations',
+      key: 'pending',
+    },
     {
       path: '/users/disabled',
       title: 'Disabled Users',

redash/handlers/users.py

@@ -14,6 +14,7 @@
 from redash.handlers.base import BaseResource, require_fields, get_object_or_404, paginate, order_results as _order_results
 
 from redash.authentication.account import invite_link_for_user, send_invite_email, send_password_reset_email
+from redash.settings import parse_boolean
 
 
 # Ordering map for relationships
@@ -41,6 +42,35 @@ def invite_user(org, inviter, user):
 
 
 class UserListResource(BaseResource):
+    def get_users(self, disabled, pending, search_term):
+        if disabled:
+            users = models.User.all_disabled(self.current_org)
+        else:
+            users = models.User.all(self.current_org)
+
+        if pending is not None:
+            users = models.User.pending(users, pending)
+
+        if search_term:
+            users = models.User.search(users, search_term)
+            self.record_event({
+                'action': 'search',
+                'object_type': 'user',
+                'term': search_term,
+                'pending': pending,
+            })
+        else:
+            self.record_event({
+                'action': 'list',
+                'object_type': 'user',
+                'pending': pending,
+            })
+
+        # order results according to passed order parameter,
+        # special-casing search queries where the database
+        # provides an order by search rank
+        return order_results(users, fallback=bool(search_term))
+
     @require_permission('list_users')
     def get(self):
         page = request.args.get('page', 1, type=int)
@@ -63,30 +93,16 @@ def serialize_user(user):
 
         search_term = request.args.get('q', '')
 
-        if request.args.get('disabled', None) is not None:
-            users = models.User.all_disabled(self.current_org)
-        else:
-            users = models.User.all(self.current_org)
+        disabled = request.args.get('disabled', 'false')  # get enabled users by default
+        disabled = parse_boolean(disabled)
 
-        if search_term:
-            users = models.User.search(users, search_term)
-            self.record_event({
-                'action': 'search',
-                'object_type': 'user',
-                'term': search_term,
-            })
-        else:
-            self.record_event({
-                'action': 'list',
-                'object_type': 'user',
-            })
+        pending = request.args.get('pending', None)  # get both active and pending by default
+        if pending is not None:
+            pending = parse_boolean(pending)
 
-        # order results according to passed order parameter,
-        # special-casing search queries where the database
-        # provides an order by search rank
-        ordered_users = order_results(users, fallback=bool(search_term))
+        users = self.get_users(disabled, pending, search_term)
 
-        return paginate(ordered_users, page, page_size, serialize_user)
+        return paginate(users, page, page_size, serialize_user)
 
     @require_admin
     def post(self):

redash/models/users.py

@@ -190,6 +190,10 @@ def get_by_api_key_and_org(cls, api_key, org):
     def all(cls, org):
         return cls.get_by_org(org).filter(cls.disabled_at.is_(None))
 
+    @classmethod
+    def all_disabled(cls, org):
+        return cls.get_by_org(org).filter(cls.disabled_at.isnot(None))
+
     @classmethod
     def search(cls, base_query, term):
         term = u'%{}%'.format(term)
@@ -198,8 +202,11 @@ def search(cls, base_query, term):
         return base_query.filter(search_filter)
 
     @classmethod
-    def all_disabled(cls, org):
-        return cls.get_by_org(org).filter(cls.disabled_at.isnot(None))
+    def pending(cls, base_query, pending):
+        if pending:
+            return base_query.filter(cls.is_invitation_pending.is_(True))
+        else:
+            return base_query.filter(cls.is_invitation_pending.isnot(True))  # check for both `false`/`null`
 
     @classmethod
     def find_by_email(cls, email):

tests/handlers/test_users.py

@@ -74,17 +74,95 @@ def test_returns_400_when_email_taken_case_insensitive(self):
 
 
 class TestUserListGet(BaseTestCase):
+    def create_filters_fixtures(self):
+        class PlainObject(object):
+            pass
+
+        result = PlainObject()
+        now = models.db.func.now()
+
+        result.enabled_active1 = self.factory.create_user(disabled_at=None, is_invitation_pending=None).id
+        result.enabled_active2 = self.factory.create_user(disabled_at=None, is_invitation_pending=False).id
+        result.enabled_pending = self.factory.create_user(disabled_at=None, is_invitation_pending=True).id
+        result.disabled_active1 = self.factory.create_user(disabled_at=now, is_invitation_pending=None).id
+        result.disabled_active2 = self.factory.create_user(disabled_at=now, is_invitation_pending=False).id
+        result.disabled_pending = self.factory.create_user(disabled_at=now, is_invitation_pending=True).id
+
+        return result
+
+    def make_request_and_return_ids(self, *args, **kwargs):
+        rv = self.make_request(*args, **kwargs)
+        return map(lambda u: u['id'], rv.json['results'])
+
+    def assertUsersListMatches(self, actual_ids, expected_ids, unexpected_ids):
+        actual_ids = set(actual_ids)
+        expected_ids = set(expected_ids)
+        unexpected_ids = set(unexpected_ids)
+        self.assertSetEqual(actual_ids.intersection(expected_ids), expected_ids)
+        self.assertSetEqual(actual_ids.intersection(unexpected_ids), set())
+
     def test_returns_users_for_given_org_only(self):
         user1 = self.factory.user
         user2 = self.factory.create_user()
         org = self.factory.create_org()
         user3 = self.factory.create_user(org=org)
 
-        rv = self.make_request('get', "/api/users")
-        user_ids = map(lambda u: u['id'], rv.json['results'])
-        self.assertIn(user1.id, user_ids)
-        self.assertIn(user2.id, user_ids)
-        self.assertNotIn(user3.id, user_ids)
+        user_ids = self.make_request_and_return_ids('get', '/api/users')
+        self.assertUsersListMatches(user_ids, [user1.id, user2.id], [user3.id])
+
+    def test_gets_all_enabled(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.enabled_active1, users.enabled_active2, users.enabled_pending],
+            [users.disabled_active1, users.disabled_active2, users.disabled_pending]
+        )
+
+    def test_gets_all_disabled(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users?disabled=true')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.disabled_active1, users.disabled_active2, users.disabled_pending],
+            [users.enabled_active1, users.enabled_active2, users.enabled_pending]
+        )
+
+    def test_gets_all_enabled_and_active(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users?pending=false')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.enabled_active1, users.enabled_active2],
+            [users.enabled_pending, users.disabled_active1, users.disabled_active2, users.disabled_pending]
+        )
+
+    def test_gets_all_enabled_and_pending(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users?pending=true')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.enabled_pending],
+            [users.enabled_active1, users.enabled_active2, users.disabled_active1, users.disabled_active2, users.disabled_pending]
+        )
+
+    def test_gets_all_disabled_and_active(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users?disabled=true&pending=false')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.disabled_active1, users.disabled_active2],
+            [users.disabled_pending, users.enabled_active1, users.enabled_active2, users.enabled_pending]
+        )
+
+    def test_gets_all_disabled_and_pending(self):
+        users = self.create_filters_fixtures()
+        user_ids = self.make_request_and_return_ids('get', '/api/users?disabled=true&pending=true')
+        self.assertUsersListMatches(
+            user_ids,
+            [users.disabled_pending],
+            [users.disabled_active1, users.disabled_active2, users.enabled_active1, users.enabled_active2, users.enabled_pending]
+        )
 
 
 class TestUserResourceGet(BaseTestCase):