Show active and pending users separately (for admins)

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,36 @@ 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 +94,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,72 @@ def test_returns_400_when_email_taken_case_insensitive(self):
 
 
 class TestUserListGet(BaseTestCase):
+    def make_request_and_check_users(self, method, path, expected_users, unexpected_users, *args, **kwargs):
+        rv = self.make_request(method, path, *args, **kwargs)
+        user_ids = map(lambda u: u['id'], rv.json['results'])
+        for user in expected_users:
+            self.assertIn(user.id, user_ids)
+        for user in unexpected_users:
+            self.assertNotIn(user.id, user_ids)
+
     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)
+        self.make_request_and_check_users('get', '/api/users', [user1, user2], [user3])
+
+    def test_returns_users_by_filter(self):
+        now = models.db.func.now()
+        user_enabled_active1 = self.factory.create_user(disabled_at=None, is_invitation_pending=None)
+        user_enabled_active2 = self.factory.create_user(disabled_at=None, is_invitation_pending=False)
+        user_enabled_pending = self.factory.create_user(disabled_at=None, is_invitation_pending=True)
+        user_disabled_active1 = self.factory.create_user(disabled_at=now, is_invitation_pending=None)
+        user_disabled_active2 = self.factory.create_user(disabled_at=now, is_invitation_pending=False)
+        user_disabled_pending = self.factory.create_user(disabled_at=now, is_invitation_pending=True)
+
+        # get all enabled
+        self.make_request_and_check_users(
+            'get', '/api/users',
+            [user_enabled_active1, user_enabled_active2, user_enabled_pending],
+            [user_disabled_active1, user_disabled_active2, user_disabled_pending]
+        )
+
+        # get all disabled
+        self.make_request_and_check_users(
+            'get', '/api/users?disabled=true',
+            [user_disabled_active1, user_disabled_active2, user_disabled_pending],
+            [user_enabled_active1, user_enabled_active2, user_enabled_pending]
+        )
+
+        # get all enabled and active
+        self.make_request_and_check_users(
+            'get', '/api/users?pending=false',
+            [user_enabled_active1, user_enabled_active2],
+            [user_enabled_pending, user_disabled_active1, user_disabled_active2, user_disabled_pending]
+        )
+
+        # get all enabled and pending
+        self.make_request_and_check_users(
+            'get', '/api/users?pending=true',
+            [user_enabled_pending],
+            [user_enabled_active1, user_enabled_active2, user_disabled_active1, user_disabled_active2, user_disabled_pending]
+        )
+
+        # get all disabled and active
+        self.make_request_and_check_users(
+            'get', '/api/users?disabled=true&pending=false',
+            [user_disabled_active1, user_disabled_active2],
+            [user_disabled_pending, user_enabled_active1, user_enabled_active2, user_enabled_pending]
+        )
+
+        # get all disabled and pending
+        self.make_request_and_check_users(
+            'get', '/api/users?disabled=true&pending=true',
+            [user_disabled_pending],
+            [user_disabled_active1, user_disabled_active2, user_enabled_active1, user_enabled_active2, user_enabled_pending]
+        )
 
 
 class TestUserResourceGet(BaseTestCase):