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

Solve intermittent bug where form permissions are not applied for new forms #2470

Merged
merged 6 commits into from
Aug 17, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from django.contrib.auth.models import User, timezone
from django.core.cache import cache
from django.test.utils import override_settings

from guardian.shortcuts import get_perms
from mock import patch
Expand All @@ -28,8 +29,12 @@
from onadata.apps.api.viewsets.user_profile_viewset import UserProfileViewSet
from onadata.apps.logger.models.project import Project
from onadata.apps.main.models import UserProfile
from onadata.libs.permissions import DataEntryRole, OwnerRole, EditorRole
from onadata.libs.utils.cache_tools import PROJ_OWNER_CACHE, PROJ_PERM_CACHE, PROJ_TEAM_USERS_CACHE
from onadata.libs.permissions import DataEntryRole, OwnerRole
from onadata.libs.utils.cache_tools import (
PROJ_OWNER_CACHE,
PROJ_PERM_CACHE,
PROJ_TEAM_USERS_CACHE,
)


# pylint: disable=too-many-public-methods
Expand Down Expand Up @@ -796,29 +801,34 @@ def test_add_members_to_owner_role(self):

self.assertNotIn(aboy, owner_team.user_set.all())

@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
def test_org_members_added_to_projects(self):
# create org
self._org_create()
view = OrganizationProfileViewSet.as_view(
{"post": "members", "get": "retrieve", "put": "members"}
)
# create a proj
project_data = {"owner": self.company_data["user"]}
self._project_create(project_data)

with self.captureOnCommitCallbacks(execute=True):
self._publish_xls_form_to_project()

# create aboy
self.profile_data["username"] = "aboy"
aboy = self._create_user_profile().user

data = {"username": "aboy", "role": "owner"}
request = self.factory.post(
"/", data=json.dumps(data), content_type="application/json", **self.extra
"/",
data=json.dumps(data),
content_type="application/json",
**self.extra,
)
response = view(request, user="denoinc")
self.assertEqual(response.status_code, 201)

# create a proj
project_data = {"owner": self.company_data["user"]}
self._project_create(project_data)
self._publish_xls_form_to_project()

# create alice
self.profile_data["username"] = "alice"
alice = self._create_user_profile().user
Expand All @@ -833,6 +843,8 @@ def test_org_members_added_to_projects(self):
self.assertEqual(response.status_code, 201)

# Assert that user added in org is added to teams in proj
aboy.refresh_from_db()
alice.refresh_from_db()
self.assertTrue(OwnerRole.user_has_role(aboy, self.project))
self.assertTrue(OwnerRole.user_has_role(alice, self.project))
self.assertTrue(OwnerRole.user_has_role(aboy, self.xform))
Expand Down
40 changes: 26 additions & 14 deletions onadata/apps/api/tests/viewsets/test_project_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -777,13 +777,18 @@ def test_project_manager_can_assign_form_to_project_no_perm(self):
self.assertEqual(response.status_code, 403)

# pylint: disable=invalid-name
@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
def test_project_users_get_readonly_role_on_add_form(self):
self._project_create()
alice_data = {"username": "alice", "email": "[email protected]"}
alice_profile = self._create_user_profile(alice_data)
ReadOnlyRole.add(alice_profile.user, self.project)
self.assertTrue(ReadOnlyRole.user_has_role(alice_profile.user, self.project))
self._publish_xls_form_to_project()

with self.captureOnCommitCallbacks(execute=True):
self._publish_xls_form_to_project()

alice_profile.refresh_from_db()
self.assertTrue(ReadOnlyRole.user_has_role(alice_profile.user, self.xform))
self.assertFalse(OwnerRole.user_has_role(alice_profile.user, self.xform))

Expand Down Expand Up @@ -1253,6 +1258,7 @@ def test_project_share_endpoint(self, mock_send_mail):
role_class._remove_obj_permissions(alice_profile.user, self.project)

# pylint: disable=invalid-name
@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
@patch("onadata.apps.api.viewsets.project_viewset.send_mail")
def test_project_share_endpoint_form_published_later(self, mock_send_mail):
# create project
Expand Down Expand Up @@ -1280,7 +1286,10 @@ def test_project_share_endpoint_form_published_later(self, mock_send_mail):
self.assertTrue(role_class.user_has_role(alice_profile.user, self.project))

# publish form after project sharing
self._publish_xls_form_to_project()
with self.captureOnCommitCallbacks(execute=True):
self._publish_xls_form_to_project()

alice_profile.user.refresh_from_db()
self.assertTrue(role_class.user_has_role(alice_profile.user, self.xform))
# Reset the mock called value to False
mock_send_mail.called = False
Expand Down Expand Up @@ -1673,6 +1682,7 @@ def test_projects_get_exception(self):
error_msg = "Invalid value for project_id. It must be a positive integer."
self.assertEqual(str(response.data["detail"]), error_msg)

@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
def test_publish_to_public_project(self):
public_project = Project(
name="demo",
Expand All @@ -1682,12 +1692,14 @@ def test_publish_to_public_project(self):
organization=self.user,
)
public_project.save()

self.project = public_project
self._publish_xls_form_to_project(public=True)

self.assertEqual(self.xform.shared, True)
self.assertEqual(self.xform.shared_data, True)
with self.captureOnCommitCallbacks(execute=True):
self._publish_xls_form_to_project(public=True)

self.xform.refresh_from_db()
self.assertTrue(self.xform.shared)
self.assertTrue(self.xform.shared_data)

def test_public_form_private_project(self):
self.project = Project(
Expand Down Expand Up @@ -1738,6 +1750,7 @@ def test_public_form_private_project(self):
self.assertFalse(self.xform.shared_data)
self.assertFalse(self.project.shared)

@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
def test_publish_to_public_project_public_form(self):
public_project = Project(
name="demo",
Expand All @@ -1747,9 +1760,7 @@ def test_publish_to_public_project_public_form(self):
organization=self.user,
)
public_project.save()

self.project = public_project

data = {
"owner": f"http://testserver/api/v1/users/{self.project.organization.username}",
"public": True,
Expand All @@ -1763,10 +1774,13 @@ def test_publish_to_public_project_public_form(self):
"title": "transportation_2011_07_25",
"bamboo_dataset": "",
}
self._publish_xls_form_to_project(publish_data=data, merge=False)

self.assertEqual(self.xform.shared, True)
self.assertEqual(self.xform.shared_data, True)
with self.captureOnCommitCallbacks(execute=True):
self._publish_xls_form_to_project(publish_data=data, merge=False)

self.xform.refresh_from_db()
self.assertTrue(self.xform.shared)
self.assertTrue(self.xform.shared_data)

def test_project_all_users_can_share_remove_themselves(self):
self._publish_xls_form_to_project()
Expand Down Expand Up @@ -2913,9 +2927,7 @@ def test_create_invitation(self, mock_send_mail):
"status": 1,
},
)
mock_send_mail.assert_called_with(
invitation.pk, "https://onadata.com/register"
)
mock_send_mail.assert_called_with(invitation.pk, "https://onadata.com/register")

def test_email_required(self, mock_send_mail):
"""email is required"""
Expand Down
7 changes: 6 additions & 1 deletion onadata/apps/viewer/models/data_dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from django.core.files.uploadedfile import InMemoryUploadedFile
from django.db.models.signals import post_save, pre_save
from django.db import transaction
from django.utils import timezone
from django.utils.translation import gettext as _

Expand Down Expand Up @@ -214,7 +215,11 @@ def set_object_permissions(sender, instance=None, created=False, **kwargs):
) # noqa

try:
set_project_perms_to_xform_async.delay(xform.pk, instance.project.pk)
transaction.on_commit(
lambda: set_project_perms_to_xform_async.delay(
xform.pk, instance.project.pk
)
)
except OperationalError:
# pylint: disable=import-outside-toplevel
from onadata.libs.utils.project_utils import (
Expand Down
Loading