diff --git a/onadata/apps/api/models/organization_profile.py b/onadata/apps/api/models/organization_profile.py index 82eac7bca5..fc58c95b81 100644 --- a/onadata/apps/api/models/organization_profile.py +++ b/onadata/apps/api/models/organization_profile.py @@ -26,50 +26,61 @@ def org_profile_post_delete_callback(sender, instance, **kwargs): safe_delete('{}{}'.format(IS_ORG, instance.pk)) -def create_owner_team_and_permissions(sender, instance, created, **kwargs): +def create_owner_team_and_assign_permissions(org): """ - Signal handler that creates the Owner team and assigns group and user - permissions. + Creates an Owner Team for a given organization and + assigns the group and user permissions """ - if created: - team = Team.objects.create( - name=Team.OWNER_TEAM_NAME, organization=instance.user, - created_by=instance.created_by) - content_type = ContentType.objects.get( - app_label='api', model='organizationprofile') - permission, created = Permission.objects.get_or_create( - codename="is_org_owner", name="Organization Owner", - content_type=content_type) - team.permissions.add(permission) - instance.creator.groups.add(team) - - for perm in get_perms_for_model(instance.__class__): - assign_perm(perm.codename, instance.user, instance) - - if instance.creator: - assign_perm(perm.codename, instance.creator, instance) - - if instance.created_by and instance.created_by != instance.creator: - assign_perm(perm.codename, instance.created_by, instance) - - if instance.userprofile_ptr: - for perm in get_perms_for_model( - instance.userprofile_ptr.__class__): + team = Team.objects.create( + name=Team.OWNER_TEAM_NAME, organization=org.user, + created_by=org.created_by) + content_type = ContentType.objects.get( + app_label='api', model='organizationprofile') + # pylint: disable=unpacking-non-sequence + permission, _ = Permission.objects.get_or_create( + codename="is_org_owner", name="Organization Owner", + content_type=content_type) # pylint: disable= + team.permissions.add(permission) + org.creator.groups.add(team) + + for perm in get_perms_for_model(org.__class__): + assign_perm(perm.codename, org.user, org) + + if org.creator: + assign_perm(perm.codename, org.creator, org) + + if org.created_by and org.created_by != org.creator: + assign_perm(perm.codename, org.created_by, org) + + if org.userprofile_ptr: + for perm in get_perms_for_model( + org.userprofile_ptr.__class__): + assign_perm( + perm.codename, org.user, org.userprofile_ptr) + + if org.creator: assign_perm( - perm.codename, instance.user, instance.userprofile_ptr) + perm.codename, + org.creator, + org.userprofile_ptr) + + if org.created_by and\ + org.created_by != org.creator: + assign_perm( + perm.codename, + org.created_by, + org.userprofile_ptr) + + return team - if instance.creator: - assign_perm( - perm.codename, - instance.creator, - instance.userprofile_ptr) - if instance.created_by and\ - instance.created_by != instance.creator: - assign_perm( - perm.codename, - instance.created_by, - instance.userprofile_ptr) +def _post_save_create_owner_team(sender, instance, created, **kwargs): + """ + Signal handler that creates the Owner team and assigns group and user + permissions. + """ + if created: + create_owner_team_and_assign_permissions(instance) @python_2_unicode_compatible @@ -124,7 +135,7 @@ def is_organization_owner(self, user): post_save.connect( - create_owner_team_and_permissions, sender=OrganizationProfile, + _post_save_create_owner_team, sender=OrganizationProfile, dispatch_uid='create_owner_team_and_permissions') post_delete.connect(org_profile_post_delete_callback, diff --git a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py index 2c98d774fc..fae9c63dc1 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -11,7 +11,7 @@ from onadata.apps.api.viewsets.user_profile_viewset import UserProfileViewSet from onadata.apps.api.viewsets.project_viewset import ProjectViewSet from onadata.libs.permissions import OwnerRole -from onadata.apps.api.tools import (get_organization_owners_team, +from onadata.apps.api.tools import (get_or_create_organization_owners_team, add_user_to_organization) from onadata.apps.api.models.organization_profile import OrganizationProfile from onadata.apps.main.models import UserProfile @@ -759,7 +759,7 @@ def test_add_members_to_owner_role(self): self.assertEqual(response.data['users'][1]['user'], 'aboy') self.assertEqual(response.data['users'][1]['role'], 'owner') - owner_team = get_organization_owners_team(self.organization) + owner_team = get_or_create_organization_owners_team(self.organization) self.assertIn(aboy, owner_team.user_set.all()) @@ -772,7 +772,7 @@ def test_add_members_to_owner_role(self): response = view(request, user='denoinc') self.assertEqual(response.status_code, 200) - owner_team = get_organization_owners_team(self.organization) + owner_team = get_or_create_organization_owners_team(self.organization) self.assertNotIn(aboy, owner_team.user_set.all()) diff --git a/onadata/apps/api/tests/viewsets/test_project_viewset.py b/onadata/apps/api/tests/viewsets/test_project_viewset.py index ba26f28821..3ff91d0e61 100644 --- a/onadata/apps/api/tests/viewsets/test_project_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_project_viewset.py @@ -17,7 +17,7 @@ from onadata.apps.api import tools from onadata.apps.api.tests.viewsets.test_abstract_viewset import \ TestAbstractViewSet -from onadata.apps.api.tools import get_organization_owners_team +from onadata.apps.api.tools import get_or_create_organization_owners_team from onadata.apps.api.viewsets.organization_profile_viewset import \ OrganizationProfileViewSet from onadata.apps.api.viewsets.project_viewset import ProjectViewSet @@ -774,7 +774,7 @@ def test_form_transfer_when_org_creator_creates_project( response = view(request, user=self.organization.user.username) self.assertEqual(response.status_code, 201) - owners_team = get_organization_owners_team(self.organization) + owners_team = get_or_create_organization_owners_team(self.organization) self.assertIn(alice_profile.user, owners_team.user_set.all()) # let bob create a project in org @@ -853,7 +853,7 @@ def test_form_transfer_when_org_admin_not_creator_creates_project( response = view(request, user=self.organization.user.username) self.assertEqual(response.status_code, 201) - owners_team = get_organization_owners_team(self.organization) + owners_team = get_or_create_organization_owners_team(self.organization) self.assertIn(alice_profile.user, owners_team.user_set.all()) # let alice create a project in org diff --git a/onadata/apps/api/tests/viewsets/test_team_viewset.py b/onadata/apps/api/tests/viewsets/test_team_viewset.py index 5abb909804..2505c42be9 100644 --- a/onadata/apps/api/tests/viewsets/test_team_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_team_viewset.py @@ -7,7 +7,7 @@ from onadata.apps.api.tests.viewsets.test_abstract_viewset import \ TestAbstractViewSet from onadata.apps.api.tools import add_user_to_team -from onadata.apps.api.tools import get_organization_owners_team, \ +from onadata.apps.api.tools import get_or_create_organization_owners_team, \ get_organization_members_team from onadata.apps.api.viewsets.metadata_viewset import MetaDataViewSet from onadata.apps.api.viewsets.organization_profile_viewset import \ @@ -399,7 +399,7 @@ def test_non_owners_should_be_able_to_change_member_permissions(self): self.assertEqual(response.status_code, 201) - owners_team = get_organization_owners_team(self.organization) + owners_team = get_or_create_organization_owners_team(self.organization) self.assertIn(chuck_profile.user, owners_team.user_set.all()) alice_data = {'username': 'alice', 'email': 'alice@localhost.com'} diff --git a/onadata/apps/api/tests/viewsets/test_widget_viewset.py b/onadata/apps/api/tests/viewsets/test_widget_viewset.py index 4427554910..f12be5164f 100644 --- a/onadata/apps/api/tests/viewsets/test_widget_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_widget_viewset.py @@ -11,7 +11,7 @@ from onadata.libs.permissions import ReadOnlyRole from onadata.libs.permissions import DataEntryOnlyRole from onadata.libs.permissions import OwnerRole -from onadata.apps.api.tools import get_organization_owners_team +from onadata.apps.api.tools import get_or_create_organization_owners_team from onadata.apps.api.viewsets.organization_profile_viewset import\ OrganizationProfileViewSet @@ -717,7 +717,7 @@ def test_widget_create_by_org_admin(self): self.assertEqual(response.status_code, 201) - owners_team = get_organization_owners_team(self.organization) + owners_team = get_or_create_organization_owners_team(self.organization) self.assertIn(chuck_profile.user, owners_team.user_set.all()) extra = { diff --git a/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py b/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py index bf8dca52ae..3a45e41682 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py @@ -1005,7 +1005,7 @@ def test_retrieve_form_using_pk(self): request = self.factory.get( f'/enketo/{self.xform.pk}/formList') response = self.view( - request, form_pk=self.xform.pk) + request, xform_pk=self.xform.pk) self.assertEqual(response.status_code, 401) # Set require auth to false for form owner @@ -1030,7 +1030,7 @@ def test_retrieve_form_using_pk(self): f'/enketo/{self.xform.pk}/formList') request.META.update(auth(request.META, response)) response = self.view( - request, form_pk=self.xform.pk) + request, xform_pk=self.xform.pk) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.data), 1) self.assertEqual( @@ -1041,7 +1041,7 @@ def test_retrieve_form_using_pk(self): request = self.factory.get( f'/enketo/{self.xform.pk}/formList') response = self.view( - request, form_pk=self.xform.pk) + request, xform_pk=self.xform.pk) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.data), 1) self.assertEqual( diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 1e4cc4e0fc..7e9439a841 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -29,7 +29,8 @@ from rest_framework import exceptions from taggit.forms import TagField -from onadata.apps.api.models.organization_profile import OrganizationProfile +from onadata.apps.api.models.organization_profile import ( + OrganizationProfile, create_owner_team_and_assign_permissions) from onadata.apps.api.models.team import Team from onadata.apps.logger.models import DataView, Instance, Project, XForm from onadata.apps.main.forms import QuickConverter @@ -182,22 +183,32 @@ def get_organization_members_team(organization): return team -def get_organization_owners_team(org): +# pylint: disable=invalid-name +def get_or_create_organization_owners_team(org): """ Get the owners team of an organization :param org: organization :return: Owners team of the organization """ - return Team.objects.get( - name="{}#{}".format(org.user.username, Team.OWNER_TEAM_NAME), - organization=org.user) + team_name = f'{org.user.username}#{Team.OWNER_TEAM_NAME}' + try: + team = Team.objects.get(name=team_name, organization=org.user) + except Team.DoesNotExist: + from multidb.pinning import use_master # pylint: disable=import-error + with use_master: + queryset = Team.objects.filter( + name=team_name, organization=org.user) + if queryset.count() > 0: + return queryset.first() # pylint: disable=no-member + return create_owner_team_and_assign_permissions(org) + return team def remove_user_from_organization(organization, user): """Remove a user from an organization""" team = get_organization_members_team(organization) remove_user_from_team(team, user) - owners_team = get_organization_owners_team(organization) + owners_team = get_or_create_organization_owners_team(organization) remove_user_from_team(owners_team, user) role = get_role_in_org(user, organization) @@ -225,7 +236,8 @@ def remove_user_from_team(team, user): # if team is owners team remove more perms if team.name.find(Team.OWNER_TEAM_NAME) > 0: - owners_team = get_organization_owners_team(team.organization.profile) + owners_team = get_or_create_organization_owners_team( + team.organization.profile) members_team = get_organization_members_team(team.organization.profile) for perm in get_perms_for_model(Team): remove_perm(perm.codename, user, owners_team) @@ -254,7 +266,7 @@ def add_user_to_team(team, user): def _assign_organization_team_perms(organization, user): - owners_team = get_organization_owners_team(organization.profile) + owners_team = get_or_create_organization_owners_team(organization.profile) members_team = get_organization_members_team(organization.profile) for perm in get_perms_for_model(Team): assign_perm(perm.codename, user, owners_team) @@ -270,7 +282,7 @@ def get_organization_members(organization): def get_organization_owners(organization): """Get owners team user queryset""" - team = get_organization_owners_team(organization) + team = get_or_create_organization_owners_team(organization) return team.user_set.all() @@ -279,7 +291,8 @@ def _get_owners(organization): return [ user - for user in get_organization_owners_team(organization).user_set.all() + for user in get_or_create_organization_owners_team( + organization).user_set.all() if get_role_in_org(user, organization) == 'owner' and organization.user != user ] diff --git a/onadata/apps/api/viewsets/xform_list_viewset.py b/onadata/apps/api/viewsets/xform_list_viewset.py index 3f4ef5dc6c..8ede9b6500 100644 --- a/onadata/apps/api/viewsets/xform_list_viewset.py +++ b/onadata/apps/api/viewsets/xform_list_viewset.py @@ -73,7 +73,7 @@ def get_renderers(self): def filter_queryset(self, queryset): username = self.kwargs.get('username') - form_pk = self.kwargs.get('form_pk') + form_pk = self.kwargs.get('xform_pk') if (not username and not form_pk) and \ self.request.user.is_anonymous: # raises a permission denied exception, forces authentication @@ -85,8 +85,8 @@ def filter_queryset(self, queryset): UserProfile, user__username=username) elif form_pk: queryset = queryset.filter(pk=form_pk) - form_owner = queryset.first().user - profile = form_owner.profile + if queryset.first(): + profile = queryset.first().user.profile if profile: if profile.require_auth and self.request.user.is_anonymous: diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index ce104186d1..5859a22835 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -224,7 +224,7 @@ re_path(r'^(?P\w+)/formList$', XFormListViewSet.as_view({'get': 'list', 'head': 'list'}), name='form-list'), - re_path(r'^enketo/(?P\w+)/formList$', + re_path(r'^enketo/(?P\w+)/formList$', XFormListViewSet.as_view({'get': 'list', 'head': 'list'}), name='form-list'), re_path(r'^(?P\w+)/(?P\d+)/formList$', @@ -258,7 +258,7 @@ re_path(r'^(?P\w+)/submission$', XFormSubmissionViewSet.as_view({'post': 'create', 'head': 'create'}), name='submissions'), - re_path(r'^enketo/(?P\w+)/submission$', + re_path(r'^enketo/(?P\w+)/submission$', XFormSubmissionViewSet.as_view({'post': 'create', 'head': 'create'}), name='submissions'), re_path(r'^(?P\w+)/(?P\d+)/submission$', diff --git a/onadata/libs/filters.py b/onadata/libs/filters.py index cae1389db2..742946dbe2 100644 --- a/onadata/libs/filters.py +++ b/onadata/libs/filters.py @@ -25,10 +25,6 @@ def filter_queryset(self, request, queryset, view): view.lookup_field, view.kwargs.get('xform_pk')) lookup_field = view.lookup_field - if view.kwargs.get('form_pk'): - form_id = view.kwargs.get('form_pk') - lookup_field = 'pk' - queryset = queryset.filter(deleted_at=None) if request.user.is_anonymous: return queryset diff --git a/onadata/libs/serializers/organization_member_serializer.py b/onadata/libs/serializers/organization_member_serializer.py index 5c8c3a3775..4a0fb0baad 100644 --- a/onadata/libs/serializers/organization_member_serializer.py +++ b/onadata/libs/serializers/organization_member_serializer.py @@ -10,7 +10,7 @@ from onadata.libs.permissions import OwnerRole from onadata.libs.permissions import is_organization from onadata.apps.api.tools import add_user_to_organization -from onadata.apps.api.tools import get_organization_owners_team +from onadata.apps.api.tools import get_or_create_organization_owners_team from onadata.apps.api.tools import add_user_to_team from onadata.apps.api.tools import remove_user_from_team from onadata.apps.api.tools import _get_owners @@ -37,7 +37,7 @@ def _set_organization_role_to_user(organization, user, role): role_cls = ROLES.get(role) role_cls.add(user, organization) - owners_team = get_organization_owners_team(organization) + owners_team = get_or_create_organization_owners_team(organization) # add the owner to owners team if role == OwnerRole.name: diff --git a/onadata/libs/serializers/project_serializer.py b/onadata/libs/serializers/project_serializer.py index cc9cd526e6..6f48c03751 100644 --- a/onadata/libs/serializers/project_serializer.py +++ b/onadata/libs/serializers/project_serializer.py @@ -14,7 +14,7 @@ from onadata.apps.api.models import OrganizationProfile from onadata.apps.api.tools import (get_organization_members_team, - get_organization_owners_team) + get_or_create_organization_owners_team) from onadata.apps.logger.models import Project, XForm from onadata.libs.permissions import (OwnerRole, ReadOnlyRole, get_role, is_organization) @@ -401,7 +401,8 @@ def update(self, instance, validated_data): set_owners_permission(owner, instance) if is_organization(owner.profile): - owners_team = get_organization_owners_team(owner.profile) + owners_team = get_or_create_organization_owners_team( + owner.profile) members_team = get_organization_members_team(owner.profile) OwnerRole.add(owners_team, instance) ReadOnlyRole.add(members_team, instance)