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.

Sign up for GitHub

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

Show active and pending users separately (for admins) #3400

Merged
merged 12 commits into from
Feb 7, 2019
Merged

Show active and pending users separately (for admins) #3400

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
96c9ef1
Show active and pending users separately (for admins)
kravets-levko Feb 5, 2019
c7493e0
Merge branch 'master' into feature/active-and-pending-users
kravets-levko Feb 5, 2019
c12cbba
Show sidebar both for admins and non-admins
kravets-levko Feb 6, 2019
9191a5f
Refine users list API
kravets-levko Feb 6, 2019
3226d54
Fix users list page: apply right policy checks
kravets-levko Feb 6, 2019
49becef
Merge branch 'master' into feature/active-and-pending-users
kravets-levko Feb 6, 2019
055c3b7
Tests
kravets-levko Feb 6, 2019
1b30408
Fix code style
kravets-levko Feb 6, 2019
5e93402
List pages: debounce search
kravets-levko Feb 6, 2019
b6397dc
Refine tests
kravets-levko Feb 6, 2019
42bdb97
Refine tests
kravets-levko Feb 6, 2019
8518daf
Merge branch 'master' into feature/active-and-pending-users
kravets-levko Feb 7, 2019
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
62 changes: 36 additions & 26 deletions tests/handlers/test_users.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,75 +81,85 @@ class PlainObject(object):
result = PlainObject()
now = models.db.func.now()

result.enabled_active1 = self.factory.create_user(disabled_at=None, is_invitation_pending=None)
result.enabled_active2 = self.factory.create_user(disabled_at=None, is_invitation_pending=False)
result.enabled_pending = self.factory.create_user(disabled_at=None, is_invitation_pending=True)
result.disabled_active1 = self.factory.create_user(disabled_at=now, is_invitation_pending=None)
result.disabled_active2 = self.factory.create_user(disabled_at=now, is_invitation_pending=False)
result.disabled_pending = self.factory.create_user(disabled_at=now, is_invitation_pending=True)
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_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 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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why you care about unexpected ids. If you work with a set, you test complete, unordered equality. No need for intersection stuff as well.

Set([1,2,3]) == Set([1,3,2])
Out: True

Set([1,2,3]) == Set([1,3,2,4])
Out: False

You can just get rid of assertUsersListMatches and assert for set equality in the tests. i.e:

def test_gets_all_enabled(self, users): # using a fixture here
    actual = self.make_request_and_return_ids('get', '/api/users')
    self.assertSetEquals(actual, Set([users.enabled_active1, users.enabled_pending]))

Copy link
Collaborator Author

@kravets-levko kravets-levko Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to explain. I'm creating all possible combinations of users for my tests. But there may be other users in DB that will match particular conditions. So for example I create users 1,2,3 and do some request. Users 1,2 will match filter, user 3 not. But it's possible that pre-existed users will also be there, and, say, they'll also match filter, so result will be 1,2,8 instead of 1,2. If I'll compare for equality, test will fail. Therefore I check exactly what I have to check: which of users I expect to see in output and which shouldn't be there for sure.


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)

self.make_request_and_check_users('get', '/api/users', [user1, user2], [user3])
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()
self.make_request_and_check_users(
'get', '/api/users',
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()
self.make_request_and_check_users(
'get', '/api/users?disabled=true',
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()
self.make_request_and_check_users(
'get', '/api/users?pending=false',
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()
self.make_request_and_check_users(
'get', '/api/users?pending=true',
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()
self.make_request_and_check_users(
'get', '/api/users?disabled=true&pending=false',
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()
self.make_request_and_check_users(
'get', '/api/users?disabled=true&pending=true',
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]
)
Expand Down