Refactor Subproject validation to use it for Forms and API

readthedocs/api/v3/serializers.py

@@ -572,17 +572,9 @@ def validate_child(self, value):
                 _('You do not have permissions on the child project'),
             )
 
-        # Check the child project is not a subproject already
-        if value.superprojects.exists():
-            raise serializers.ValidationError(
-                _('Child is already a subproject of another project'),
-            )
-
-        # Check the child project is already a superproject
-        if value.subprojects.exists():
-            raise serializers.ValidationError(
-                _('Child is already a superproject'),
-            )
+        value.is_valid_as_subproject(
+            self.parent_project, serializers.ValidationError
+        )
         return value
 
     def validate_alias(self, value):
@@ -596,17 +588,9 @@ def validate_alias(self, value):
 
     # pylint: disable=arguments-differ
     def validate(self, data):
-        # Check the parent and child are not the same project
-        if data['child'].slug == self.parent_project.slug:
-            raise serializers.ValidationError(
-                _('Project can not be subproject of itself'),
-            )
-
-        # Check the parent project is not a subproject already
-        if self.parent_project.superprojects.exists():
-            raise serializers.ValidationError(
-                _('Subproject nesting is not supported'),
-            )
+        self.parent_project.is_valid_as_superproject(
+            serializers.ValidationError
+        )
         return data
 
 

readthedocs/api/v3/tests/test_subprojects.py

@@ -108,7 +108,7 @@ def test_projects_subprojects_list_post_with_subproject_of_itself(self):
         self.assertEqual(response.status_code, 400)
         self.assertIn(
             'Project can not be subproject of itself',
-            response.json()['non_field_errors'],
+            response.json()['child'],
         )
         self.assertEqual(newproject.subprojects.count(), 0)
 

readthedocs/projects/forms.py

@@ -360,20 +360,17 @@ def __init__(self, *args, **kwargs):
             self.fields['child'].queryset = self.get_subproject_queryset()
 
     def clean_parent(self):
-        if self.project.superprojects.exists():
-            # This validation error is mostly for testing, users shouldn't see
-            # this in normal circumstances
-            raise forms.ValidationError(
-                _('Subproject nesting is not supported'),
-            )
+        self.project.is_valid_as_superproject(
+            forms.ValidationError
+        )
         return self.project
 
     def clean_child(self):
         child = self.cleaned_data['child']
-        if child == self.project:
-            raise forms.ValidationError(
-                _('A project can not be a subproject of itself'),
-            )
+
+        child.is_valid_as_subproject(
+            self.project, forms.ValidationError
+        )
         return child
 
     def clean_alias(self):

readthedocs/projects/models.py

@@ -1154,6 +1154,44 @@ def environment_variables(self):
             for variable in self.environmentvariable_set.all()
         }
 
+    def is_valid_as_superproject(self, error_class):
+        """
+        Checks if the project can be a superproject.
+
+        This is used to handle form and serializer validations
+        if check fails returns ValidationError using to the error_class passed
+        """
+        # Check the parent project is not a subproject already
+        if self.superprojects.exists():
+            raise error_class(
+                _('Subproject nesting is not supported'),
+            )
+
+    def is_valid_as_subproject(self, parent, error_class):
+        """
+        Checks if the project can be a subproject.
+
+        This is used to handle form and serializer validations
+        if check fails returns ValidationError using to the error_class passed
+        """
+        # Check the child project is not a subproject already
+        if self.superprojects.exists():
+            raise error_class(
+                _('Child is already a subproject of another project'),
+            )
+
+        # Check the child project is already a superproject
+        if self.subprojects.exists():
+            raise error_class(
+                _('Child is already a superproject'),
+            )
+
+        # Check the parent and child are not the same project
+        if parent.slug == self.slug:
+            raise error_class(
+                _('Project can not be subproject of itself'),
+            )
+
 
 class APIProject(Project):