Fix merged dataset permissions not applied on share (#2598)

* fix merged dataset permissions not applied on share

* add testcase

* update comment

* update docstring

* add decorator for flaky test

* add flaky decorator to test

* mark flaky test

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 = [

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

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

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)

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,20 +46,43 @@ 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()
         self.alice.refresh_from_db()
         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(