Allow setting any readable model version as parent of a model version, not just versions from the same model

closes #1708 (closed)

arkindex/training/tests/test_model_api.py

@@ -269,7 +269,7 @@ class TestModelAPI(FixtureAPITestCase):
     @patch("arkindex.project.aws.s3.meta.client.generate_presigned_url")
     def test_create_model_version_any_parent_model_version(self, s3_presigned_url_mock):
         """
-        Any readable model version can be set as parent of a model version, not just version of the same model
+        Any readable model version can be set as parent of a model version, not just versions of the same model
         """
         self.client.force_login(self.user1)
         s3_presigned_url_mock.return_value = "http://s3/upload_put_url"
@@ -312,6 +312,36 @@ class TestModelAPI(FixtureAPITestCase):
             }
         )
 
+    @patch("arkindex.users.managers.BaseACLManager.filter_rights")
+    def test_create_model_version_readable_parent_version(self, filter_rights_mock):
+        """
+        Only model versions the user has read access to can be set as parents of a model version
+        """
+        filter_rights_mock.return_value = Model.objects.filter(id=self.model1.id)
+        self.client.force_login(self.user1)
+        fake_now = timezone.now()
+        # To mock the creation date
+        with patch("django.utils.timezone.now") as mock_now:
+            mock_now.return_value = fake_now
+            with self.assertNumQueries(4):
+                response = self.client.post(
+                    reverse("api:model-versions", kwargs={"pk": str(self.model1.id)}),
+                    {
+                        "tag": "TAG",
+                        "description": "description",
+                        "configuration": {"hello": "this is me"},
+                        # self.model_version3 belongs to self.model2
+                        "parent": str(self.model_version3.id)
+                    },
+                    format="json",
+                )
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(filter_rights_mock.call_count, 1)
+        self.assertEqual(filter_rights_mock.call_args, call(self.user1, Model, Role.Guest.value))
+        self.assertDictEqual(response.json(), {
+            "parent": [f'Invalid pk "{str(self.model_version3.id)}" - object does not exist.']
+        })
+
     def test_retrieve_model_requires_login(self):
         with self.assertNumQueries(0):
             response = self.client.get(reverse("api:model-retrieve", kwargs={"pk": str(self.model2.id)}))
@@ -1015,6 +1045,29 @@ class TestModelAPI(FixtureAPITestCase):
         self.model_version3.refresh_from_db()
         self.assertEqual(self.model_version3.parent_id, self.model_version1.id)
 
+    @patch("arkindex.users.managers.BaseACLManager.filter_rights")
+    def test_partial_update_readable_parent_model_version(self, filter_rights_mock):
+        """
+        A model version can have any model version as a parent, not just a version from the same model
+        """
+        filter_rights_mock.return_value = Model.objects.filter(id__in=[self.model2.id, self.model3.id])
+        self.client.force_login(self.user2)
+
+        with self.assertNumQueries(4):
+            response = self.client.patch(
+                reverse("api:model-version-retrieve", kwargs={"pk": str(self.model_version3.id)}),
+                # self.model_version3 is a version of self.model2, while self.model_version1 belongs to self.model1
+                {"parent": str(self.model_version1.id)},
+                format="json"
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(filter_rights_mock.call_count, 1)
+        self.assertEqual(filter_rights_mock.call_args, call(self.user2, Model, Role.Guest.value))
+        self.assertDictEqual(response.json(), {
+            "parent": [f'Invalid pk "{str(self.model_version1.id)}" - object does not exist.']
+        })
+
     @patch("arkindex.training.api.get_max_level", return_value=None)
     def test_update_model_version_requires_contributor(self, get_max_level_mock):
         """