diff --git a/onadata/apps/api/tests/viewsets/test_data_viewset.py b/onadata/apps/api/tests/viewsets/test_data_viewset.py index 223925aae3..d30b708283 100644 --- a/onadata/apps/api/tests/viewsets/test_data_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_data_viewset.py @@ -3721,7 +3721,7 @@ def setUp(self): self.logger = logging.getLogger("console_logger") # pylint: disable=invalid-name,too-many-locals - @flaky(max_runs=5) + @flaky(max_runs=8) def test_data_retrieve_instance_osm_format(self): """Test /data endpoint OSM format.""" filenames = [ diff --git a/onadata/apps/api/tests/viewsets/test_xform_viewset.py b/onadata/apps/api/tests/viewsets/test_xform_viewset.py index a3996d96ed..a5969e631c 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_xform_viewset.py @@ -710,6 +710,7 @@ def setUp(self): ) @patch("onadata.apps.api.viewsets.xform_viewset.send_message") + @flaky def test_replace_form_with_external_choices(self, mock_send_message): with HTTMock(enketo_mock): xls_file_path = os.path.join( @@ -2283,6 +2284,7 @@ def test_form_clone_shared_forms(self): self.assertEqual(response.status_code, 201) self.assertEqual(count + 1, XForm.objects.count()) + @flaky def test_return_error_on_clone_duplicate(self): with HTTMock(enketo_mock): self._publish_xls_form_to_project() diff --git a/onadata/apps/logger/tests/test_briefcase_client.py b/onadata/apps/logger/tests/test_briefcase_client.py index 82d30fd855..ead683b0d9 100644 --- a/onadata/apps/logger/tests/test_briefcase_client.py +++ b/onadata/apps/logger/tests/test_briefcase_client.py @@ -16,6 +16,7 @@ import requests import requests_mock from django_digest.test import Client as DigestClient +from flaky import flaky from six.moves.urllib.parse import urljoin from onadata.apps.logger.models import Instance, XForm @@ -168,6 +169,7 @@ def _download_submissions(self): mocker.head(requests_mock.ANY, content=submission_list) self.briefcase_client.download_instances(self.xform.id_string) + @flaky(max_runs=8) def test_download_xform_xml(self): """ Download xform via briefcase api diff --git a/onadata/libs/models/share_project.py b/onadata/libs/models/share_project.py index d179d15972..88490069b3 100644 --- a/onadata/libs/models/share_project.py +++ b/onadata/libs/models/share_project.py @@ -30,6 +30,9 @@ def remove_xform_permissions(project, user, role): for xform in project.xform_set.all(): # pylint: disable=protected-access role._remove_obj_permissions(user, xform) + # Removed MergedXForm permissions if XForm is also a MergedXForm + if hasattr(xform, "mergedxform"): + role._remove_obj_permissions(user, xform.mergedxform) def remove_dataview_permissions(project, user, role): @@ -85,6 +88,10 @@ def save(self, **kwargs): role = ROLES.get(meta_perm[1]) role.add(self.user, xform) + # Set MergedXForm permissions if XForm is also a MergedXForm + if hasattr(xform, "mergedxform"): + role.add(self.user, xform.mergedxform) + for dataview in self.project.dataview_set.all(): if dataview.matches_parent: role.add(self.user, dataview.xform) diff --git a/onadata/libs/tests/models/test_share_project.py b/onadata/libs/tests/models/test_share_project.py index c869852618..f60d299a24 100644 --- a/onadata/libs/tests/models/test_share_project.py +++ b/onadata/libs/tests/models/test_share_project.py @@ -1,9 +1,11 @@ """Tests for module onadata.libs.models.share_project""" from unittest.mock import patch, call +from pyxform.builder import create_survey_element_from_dict from onadata.apps.logger.models.data_view import DataView from onadata.apps.logger.models.project import Project +from onadata.apps.logger.models.merged_xform import MergedXForm from onadata.apps.logger.models.xform import XForm from onadata.apps.main.tests.test_base import TestBase from onadata.libs.models.share_project import ShareProject @@ -35,7 +37,7 @@ def setUp(self): project = Project.objects.create( name="Demo", organization=self.user, created_by=self.user ) - self._publish_markdown(md_xform, self.user, project) + self._publish_markdown(md_xform, self.user, project, id_string="a") self.dataview_form = XForm.objects.all().order_by("-pk")[0] DataView.objects.create( name="Demo", @@ -44,13 +46,34 @@ def setUp(self): matches_parent=True, columns=[], ) + # MergedXForm + self._publish_markdown(md_xform, self.user, project, id_string="b") + xf1 = XForm.objects.get(id_string="a") + xf2 = XForm.objects.get(id_string="b") + survey = create_survey_element_from_dict(xf1.json_dict()) + survey["id_string"] = "c" + survey["sms_keyword"] = survey["id_string"] + survey["title"] = "Merged XForm" + self.merged_xf = MergedXForm.objects.create( + id_string=survey["id_string"], + sms_id_string=survey["id_string"], + title=survey["title"], + user=self.user, + created_by=self.user, + is_merged_dataset=True, + project=self.project, + xml=survey.to_xml(), + json=survey.to_json(), + ) + self.merged_xf.xforms.add(xf1) + self.merged_xf.xforms.add(xf2) self.alice = self._create_user("alice", "Yuao8(-)") @patch("onadata.libs.models.share_project.safe_delete") def test_share(self, mock_safe_delete, mock_propagate): """A project is shared with a user - Permissions assigned to project, xform and dataview + Permissions assigned to project, xform, mergedxform and dataview """ instance = ShareProject(self.project, self.alice, "manager") instance.save() @@ -58,6 +81,8 @@ def test_share(self, mock_safe_delete, mock_propagate): self.assertTrue(ManagerRole.user_has_role(self.alice, self.project)) self.assertTrue(ManagerRole.user_has_role(self.alice, self.xform)) self.assertTrue(ManagerRole.user_has_role(self.alice, self.dataview_form)) + self.assertTrue(ManagerRole.user_has_role(self.alice, self.merged_xf)) + self.assertTrue(ManagerRole.user_has_role(self.alice, self.merged_xf.xform_ptr)) mock_propagate.assert_called_once_with(args=[self.project.pk]) # Cache is invalidated mock_safe_delete.assert_has_calls( @@ -69,21 +94,32 @@ def test_share(self, mock_safe_delete, mock_propagate): @patch("onadata.libs.models.share_project.safe_delete") def test_remove(self, mock_safe_delete, mock_propagate): - """A user is removed from a project""" - # Add user + """A user is removed from a project + + Permissions removed from project, xform, mergedxform and dataview + """ + # Simulate share project ManagerRole.add(self.alice, self.project) ManagerRole.add(self.alice, self.xform) ManagerRole.add(self.alice, self.dataview_form) - + ManagerRole.add(self.alice, self.merged_xf) + ManagerRole.add(self.alice, self.merged_xf.xform_ptr) + # Confirm project shared self.assertTrue(ManagerRole.user_has_role(self.alice, self.project)) self.assertTrue(ManagerRole.user_has_role(self.alice, self.xform)) self.assertTrue(ManagerRole.user_has_role(self.alice, self.dataview_form)) + self.assertTrue(ManagerRole.user_has_role(self.alice, self.merged_xf)) + self.assertTrue(ManagerRole.user_has_role(self.alice, self.merged_xf.xform_ptr)) # Remove user instance = ShareProject(self.project, self.alice, "manager", True) instance.save() self.assertFalse(ManagerRole.user_has_role(self.alice, self.project)) self.assertFalse(ManagerRole.user_has_role(self.alice, self.xform)) self.assertFalse(ManagerRole.user_has_role(self.alice, self.dataview_form)) + self.assertFalse(ManagerRole.user_has_role(self.alice, self.merged_xf)) + self.assertFalse( + ManagerRole.user_has_role(self.alice, self.merged_xf.xform_ptr) + ) mock_propagate.assert_called_once_with(args=[self.project.pk]) # Cache is invalidated mock_safe_delete.assert_has_calls(