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

Fix errors encountered when utilizing a master-replica database setup #1813

Merged
merged 6 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 51 additions & 40 deletions onadata/apps/api/models/organization_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())

Expand 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())

Expand Down
6 changes: 3 additions & 3 deletions onadata/apps/api/tests/viewsets/test_project_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/api/tests/viewsets/test_team_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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': '[email protected]'}
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/api/tests/viewsets/test_widget_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 = {
Expand Down
6 changes: 3 additions & 3 deletions onadata/apps/api/tests/viewsets/test_xform_list_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand Down
33 changes: 23 additions & 10 deletions onadata/apps/api/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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()


Expand 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
]
Expand Down
6 changes: 3 additions & 3 deletions onadata/apps/api/viewsets/xform_list_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/main/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@
re_path(r'^(?P<username>\w+)/formList$',
XFormListViewSet.as_view({'get': 'list', 'head': 'list'}),
name='form-list'),
re_path(r'^enketo/(?P<form_pk>\w+)/formList$',
re_path(r'^enketo/(?P<xform_pk>\w+)/formList$',
XFormListViewSet.as_view({'get': 'list', 'head': 'list'}),
name='form-list'),
re_path(r'^(?P<username>\w+)/(?P<xform_pk>\d+)/formList$',
Expand Down Expand Up @@ -258,7 +258,7 @@
re_path(r'^(?P<username>\w+)/submission$',
XFormSubmissionViewSet.as_view({'post': 'create', 'head': 'create'}),
name='submissions'),
re_path(r'^enketo/(?P<form_pk>\w+)/submission$',
re_path(r'^enketo/(?P<xform_pk>\w+)/submission$',
XFormSubmissionViewSet.as_view({'post': 'create', 'head': 'create'}),
name='submissions'),
re_path(r'^(?P<username>\w+)/(?P<xform_pk>\d+)/submission$',
Expand Down
4 changes: 0 additions & 4 deletions onadata/libs/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions onadata/libs/serializers/organization_member_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions onadata/libs/serializers/project_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down