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

Jkmarx/add group member api #3309

Merged
merged 33 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6553d89
Extend group api to update group perms and retrieve all group info.
jkmarx Mar 26, 2019
e8d769e
Add unit tests.
jkmarx Mar 26, 2019
397603e
Refactor and add utils method.
jkmarx Mar 26, 2019
621bd48
Fix method name and comments for clarity.
jkmarx Mar 26, 2019
3fb5ac8
Refactor other view sets to use helper. (#3286)
jkmarx Mar 28, 2019
65722cf
Resolve merge conflict.
jkmarx Mar 28, 2019
964bd37
Refactor email address in tests.
jkmarx Mar 28, 2019
8e52d31
Update return statuses for clarity
jkmarx Mar 28, 2019
4c5aabd
Limit group returns by user membership.
jkmarx Mar 28, 2019
b987154
Remove unused groupManagementResource.
jkmarx Mar 28, 2019
694872c
Extend api to create groups.
jkmarx Mar 29, 2019
faa52d1
Resolve merge conflict.
jkmarx Mar 29, 2019
8dc2e47
Add unit tests.
jkmarx Mar 29, 2019
30e4eca
Merge branch 'develop' of https://github.com/refinery-platform/refine…
jkmarx Mar 29, 2019
88446c8
Extend api to return all member groups.
jkmarx Mar 29, 2019
e423606
Add unit tests.
jkmarx Mar 29, 2019
9deb3ed
Merge branch 'develop' of https://github.com/refinery-platform/refine…
jkmarx Mar 29, 2019
c40d450
Add unit tests.
jkmarx Mar 30, 2019
b5774a5
Extend group API to destroy.
jkmarx Apr 1, 2019
cc3ed54
Add unit tests.
jkmarx Apr 1, 2019
41d9a0a
Fix typo.
jkmarx Apr 1, 2019
0ec7315
Update client.
jkmarx Apr 1, 2019
9dde810
Extend api to return can_edit field.
jkmarx Apr 1, 2019
27f9405
Add group unit tests.
jkmarx Apr 1, 2019
b260db1
Avoid unneccessary api call.
jkmarx Apr 1, 2019
8c1d0e5
Adjust fields to accomodate current client.
jkmarx Apr 2, 2019
b9d12f2
Add unit tests.
jkmarx Apr 3, 2019
34c4f56
Resolve merge conflicts.
jkmarx Apr 3, 2019
1688a71
Move helper to model method.
jkmarx Apr 3, 2019
c07b901
Add unit tests.
jkmarx Apr 3, 2019
55f0969
Fix flake issue with line break.
jkmarx Apr 3, 2019
b03f733
Update client to group memebers api v2. (#3310)
jkmarx Apr 3, 2019
be9d803
Fix method name.
jkmarx Apr 3, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions refinery/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,12 @@ def get_managed_group(self):
except:
return None

def is_user_a_group_manager(self, user):
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like it should be a method on the User model.

if self.is_manager_group():
return user in self.user_set.all()
else:
return user in self.manager_group.user_set.all()


# automatic creation of a managed group when an extended group is created:
def create_manager_group(sender, instance, created, **kwargs):
Expand Down
24 changes: 21 additions & 3 deletions refinery/core/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def partial_update(self, instance, validated_data):


class ExtendedGroupSerializer(serializers.ModelSerializer):
manager_group_uuid = serializers.SerializerMethodField()
member_list = serializers.SerializerMethodField()
perm_list = serializers.SerializerMethodField()
can_edit = serializers.SerializerMethodField()
Expand All @@ -128,6 +129,11 @@ class ExtendedGroupSerializer(serializers.ModelSerializer):
validators=[UniqueValidator(queryset=ExtendedGroup.objects.all())]
)

def get_manager_group_uuid(self, group):
Copy link
Member

Choose a reason for hiding this comment

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

Seems redundant. Doesn't appear to be used anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a field the client expects.

if group.is_manager_group():
return ''
return group.manager_group.uuid

def get_can_edit(self, group):
user = self.context.get('user')
if user is None:
Expand All @@ -145,7 +151,18 @@ def get_member_list(self, group):
users = group.user_set.all().filter(is_active=True).exclude(
username=settings.ANONYMOUS_USER_NAME
)
return UserSerializer(users, many=True).data
user_data = UserSerializer(users, many=True).data
for user in user_data:
if group.is_manager_group():
user['is_manager'] = user.get('id') in \
group.user_set.all().values_list(
'id', flat=True
)
else:
user['is_manager'] = user.get('id') in \
group.manager_group.user_set.all()\
.values_list('id', flat=True)
return user_data
return []

def get_perm_list(self, group):
Expand All @@ -162,7 +179,8 @@ def get_uuid(self, group):

class Meta:
model = ExtendedGroup
fields = ('can_edit', 'name', 'id', 'uuid', 'member_list', 'perm_list')
fields = ('can_edit', 'name', 'id', 'uuid', 'manager_group_uuid',
'member_list', 'perm_list')


class SiteVideoSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -248,7 +266,7 @@ class UserSerializer(serializers.ModelSerializer):

class Meta:
model = User
fields = ('first_name', 'last_name', 'profile', 'username')
fields = ('first_name', 'id', 'last_name', 'profile', 'username')


class WorkflowSerializer(serializers.HyperlinkedModelSerializer):
Expand Down
25 changes: 25 additions & 0 deletions refinery/core/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,31 @@ def test_is_clean_if_data_set_is_shared(self):
self.assertTrue(data_set.is_clean())


class ExtendedGroupModelTests(TestCase):
def setUp(self):
self.group = ExtendedGroup.objects.create(name="Test Group")
self.user = User.objects.create_user('managerUser')
self.non_manager = User.objects.create_user('regularUser')
self.group.user_set.add(self.user)
self.group.manager_group.user_set.add(self.user)

def test_is_user_a_group_manager_returns_true_for_group(self):
self.assertTrue(self.group.is_user_a_group_manager(self.user))

def test_is_user_a_group_manager_returns_true_for_manager_group(self):
self.assertTrue(
self.group.manager_group.is_user_a_group_manager(self.user)
)

def test_is_user_a_group_manager_returns_false_for_group(self):
self.assertFalse(self.group.is_user_a_group_manager(self.non_manager))

def test_is_user_a_group_manager_returns_false_for_manager_group(self):
self.assertFalse(
self.group.manager_group.is_user_a_group_manager(self.non_manager)
)


class EventTests(TestCase):

def setUp(self):
Expand Down
160 changes: 157 additions & 3 deletions refinery/core/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@
from .serializers import DataSetSerializer, UserSerializer

from .views import (AnalysisViewSet, DataSetsViewSet, EventViewSet,
GroupViewSet, ObtainAuthTokenValidSession,
SiteProfileViewSet, UserProfileViewSet, WorkflowViewSet,
user)
GroupViewSet, GroupMemberAPIView,
ObtainAuthTokenValidSession, SiteProfileViewSet,
UserProfileViewSet, WorkflowViewSet, user)

cache = memcache.Client(["127.0.0.1:11211"])

Expand Down Expand Up @@ -1018,6 +1018,19 @@ def test_get_groups_with_data_set_uuid_has_can_edit_false(self):
get_response = self.view(get_request)
self.assertEqual(get_response.data[0].get('can_edit'), False)

def test_get_groups_with_data_set_uuid_has_manager_group_uuid(self):
new_user = User.objects.create_user('Non-owner',
'user@example.com',
self.password)
self.group.user_set.add(new_user)
ExtendedGroup.objects.public_group().user_set.remove(new_user)
get_request = self.factory.get(self.url_root,
{'dataSetUuid': self.data_set.uuid})
force_authenticate(get_request, user=new_user)
get_response = self.view(get_request)
self.assertEqual(get_response.data[0].get('manager_group_uuid'),
self.group.manager_group.uuid)

def test_get_groups_with_data_set_uuid_has_correct_perms_field(self):
self.data_set.unshare(self.group_2)
get_request = self.factory.get(self.url_root,
Expand Down Expand Up @@ -1248,6 +1261,147 @@ def test_post_groups_returns_400_for_duplicate_name(self):
self.assertEqual(post_response.status_code, 400)


class GroupMemberApiV2Tests(APIV2TestCase):
def setUp(self):
self.group = ExtendedGroup.objects.create(name="Test Group")
super(GroupMemberApiV2Tests, self).setUp(
api_base_name='groups/' + self.group.uuid + '/members/',
view=GroupMemberAPIView.as_view()
)
self.data_set = create_dataset_with_necessary_models(user=self.user)
self.group.manager_group.user_set.add(self.user)
self.group.user_set.add(self.user)
self.public_group = ExtendedGroup.objects.public_group()
self.public_group.manager_group.user_set.add(self.user)
self.non_manager = User.objects.create_user('Non-owner',
'user@example.com',
self.password)
self.group.user_set.add(self.non_manager)

def test_delete_member_returns_403_for_non_self_and_manager(self):
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.user.id) + '/')
)
force_authenticate(delete_request, user=self.non_manager)
delete_response = self.view(delete_request,
self.group.uuid,
self.user.id)
self.assertEqual(delete_response.status_code, 403)

def test_delete_member_returns_400_user_leaves_public_group(self):
delete_request = self.factory.delete(
'/groups/' + self.public_group.uuid + '/members/' +
str(self.non_manager.id) + '/'
)
force_authenticate(delete_request, user=self.non_manager)
delete_response = self.view(delete_request,
self.public_group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 400)

def test_delete_member_returns_400_manager_removes_user_from_public(self):
delete_request = self.factory.delete(
'/groups/' + self.public_group.uuid + '/members/' +
str(self.non_manager.id) + '/'
)
force_authenticate(delete_request, user=self.user)
delete_response = self.view(delete_request,
self.public_group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 400)

def test_delete_member_returns_400_last_manager_demotes_self(self):
delete_request = self.factory.delete(
'/groups/' + self.group.manager_group.uuid + '/members/' +
str(self.user.id) + '/'
)
force_authenticate(delete_request, user=self.user)
delete_response = self.view(delete_request,
self.group.uuid,
self.user.id)
self.assertEqual(delete_response.status_code, 400)

def test_delete_member_returns_400_manager_leaves(self):
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.user.id) + '/')
)
force_authenticate(delete_request, user=self.user)
delete_response = self.view(delete_request,
self.group.uuid,
self.user.id)
self.assertEqual(delete_response.status_code, 400)

def test_delete_member_leaves_group_success(self):
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.non_manager.id) + '/')
)
force_authenticate(delete_request, user=self.non_manager)
delete_response = self.view(delete_request,
self.group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 200)
self.assertNotIn(self.non_manager,
self.group.user_set.all())

def test_delete_member_by_manager_removes_member(self):
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.non_manager.id) + '/')
)
force_authenticate(delete_request, user=self.user)
delete_response = self.view(delete_request,
self.group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 200)
self.assertNotIn(self.non_manager,
self.group.user_set.all())

def test_delete_member_by_manager_demotes_member(self):
self.group.manager_group.user_set.add(self.non_manager)
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.non_manager.id) + '/')
)
force_authenticate(delete_request, user=self.user)
delete_response = self.view(delete_request,
self.group.manager_group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 200)
self.assertNotIn(self.non_manager,
self.group.manager_group.user_set.all())

def test_delete_member_by_demoting_self(self):
self.group.manager_group.user_set.add(self.non_manager)
delete_request = self.factory.delete(
urljoin(self.url_root, str(self.non_manager.id) + '/')
)
force_authenticate(delete_request, user=self.non_manager)
delete_response = self.view(delete_request,
self.group.manager_group.uuid,
self.non_manager.id)
self.assertEqual(delete_response.status_code, 200)
self.assertNotIn(self.non_manager,
self.group.manager_group.user_set.all())

def test_post_group_member_returns_403_for_non_managers(self):
post_request = self.factory.post(self.url_root,
{'userId': self.non_manager.id})
force_authenticate(post_request, user=self.non_manager)
post_request = self.view(post_request,
self.group.uuid)
self.assertEqual(post_request.status_code, 403)

def test_post_group_member_promotes_for_user(self):
post_request = self.factory.post(
'/groups/' + self.group.manager_group.uuid + '/members/',
{'userId': self.non_manager.id}
)
force_authenticate(post_request, user=self.user)
post_request = self.view(post_request,
self.group.manager_group.uuid)
self.assertEqual(post_request.status_code, 200)
self.assertIn(self.non_manager,
self.group.manager_group.user_set.all())


class AnalysisApiV2Tests(APIV2TestCase):

def setUp(self):
Expand Down
7 changes: 6 additions & 1 deletion refinery/core/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
from rest_framework.routers import DefaultRouter

from .views import (AnalysisViewSet, DataSetsViewSet, EventViewSet,
GroupViewSet, ObtainAuthTokenValidSession, OpenIDToken,
GroupViewSet, GroupMemberAPIView,
ObtainAuthTokenValidSession, OpenIDToken,
SiteProfileViewSet, UserProfileViewSet, WorkflowViewSet,
site_statistics)

Expand Down Expand Up @@ -66,6 +67,10 @@
core_router.register(r'groups', GroupViewSet, 'groups')
core_router.urls.extend([
url(r'^events/$', EventViewSet.as_view()),
url(r'^groups/(?P<uuid>' + UUID_RE + r')/members/$',
GroupMemberAPIView.as_view()),
url(r'^groups/(?P<uuid>' + UUID_RE + r')/members/(?P<id>\d)/$',
GroupMemberAPIView.as_view()),
url(r'^user_profile/(?P<uuid>' + UUID_RE + r')/$',
UserProfileViewSet.as_view()),
url(r'^analyses/$', AnalysisViewSet.as_view()),
Expand Down