diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index 1883327a441..5cd264110b6 100644 --- a/readthedocs/api/v3/serializers.py +++ b/readthedocs/api/v3/serializers.py @@ -569,7 +569,19 @@ def validate_child(self, value): user = self.context['request'].user if user not in value.users.all(): raise serializers.ValidationError( - 'You do not have permissions on the child project', + _('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'), ) return value @@ -578,7 +590,7 @@ def validate_alias(self, value): subproject = self.parent_project.subprojects.filter(alias=value) if subproject.exists(): raise serializers.ValidationError( - 'A subproject with this alias already exists', + _('A subproject with this alias already exists'), ) return value @@ -587,13 +599,13 @@ 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', + _('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', + _('Subproject nesting is not supported'), ) return data diff --git a/readthedocs/api/v3/tests/test_subprojects.py b/readthedocs/api/v3/tests/test_subprojects.py index 460adf32ef9..21c2589a28a 100644 --- a/readthedocs/api/v3/tests/test_subprojects.py +++ b/readthedocs/api/v3/tests/test_subprojects.py @@ -89,17 +89,18 @@ def test_projects_subprojects_list_post_with_others_as_child(self): self.assertEqual(self.project.subprojects.count(), 1) def test_projects_subprojects_list_post_with_subproject_of_itself(self): - self.assertEqual(self.project.subprojects.count(), 1) + newproject = self._create_new_project() + self.assertEqual(newproject.subprojects.count(), 0) self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') data = { - 'child': self.project.slug, + 'child': newproject.slug, 'alias': 'subproject-alias', } response = self.client.post( reverse( 'projects-subprojects-list', kwargs={ - 'parent_lookup_parent__slug': self.project.slug, + 'parent_lookup_parent__slug': newproject.slug, }, ), data, @@ -109,7 +110,57 @@ def test_projects_subprojects_list_post_with_subproject_of_itself(self): 'Project can not be subproject of itself', response.json()['non_field_errors'], ) - self.assertEqual(self.project.subprojects.count(), 1) + self.assertEqual(newproject.subprojects.count(), 0) + + def test_projects_subprojects_list_post_with_child_already_superproject(self): + newproject = self._create_new_project() + self.assertEqual(newproject.subprojects.count(), 0) + self.assertTrue(self.project.subprojects.exists()) + self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') + data = { + 'child': self.project.slug, + 'alias': 'subproject-alias', + } + response = self.client.post( + reverse( + 'projects-subprojects-list', + kwargs={ + 'parent_lookup_parent__slug': newproject.slug, + }, + ), + data, + ) + self.assertEqual(response.status_code, 400) + self.assertIn( + 'Child is already a superproject', + response.json()['child'], + ) + self.assertEqual(newproject.subprojects.count(), 0) + + def test_projects_subprojects_list_post_with_child_already_subproject(self): + newproject = self._create_new_project() + self.assertEqual(newproject.subprojects.count(), 0) + self.assertTrue(self.subproject.superprojects.exists()) + self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}') + data = { + 'child': self.subproject.slug, + 'alias': 'subproject-alias', + } + response = self.client.post( + reverse( + 'projects-subprojects-list', + kwargs={ + 'parent_lookup_parent__slug': newproject.slug, + }, + ), + data, + ) + self.assertEqual(response.status_code, 400) + self.assertIn( + 'Child is already a subproject of another project', + response.json()['child'], + ) + self.assertEqual(newproject.subprojects.count(), 0) def test_projects_subprojects_list_post_nested_subproject(self): newproject = self._create_new_project() diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index 9262b746f16..a86ad933683 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -150,6 +150,57 @@ def test_excludes_existing_subprojects(self): [''], ) + def test_subproject_cant_be_subproject(self): + user = fixture.get(User) + project = fixture.get(Project, users=[user]) + another_project = fixture.get(Project, users=[user]) + subproject = fixture.get(Project, users=[user]) + relation = fixture.get( + ProjectRelationship, parent=project, child=subproject, + ) + + form = ProjectRelationshipForm( + {'child': subproject.pk}, + project=project, + user=user, + ) + self.assertFalse(form.is_valid()) + self.assertRegex( + form.errors['child'][0], + 'Select a valid choice', + ) + + form = ProjectRelationshipForm( + {'child': subproject.pk}, + project=another_project, + user=user, + ) + self.assertFalse(form.is_valid()) + self.assertRegex( + form.errors['child'][0], + 'Select a valid choice', + ) + + def test_superproject_cant_be_subproject(self): + user = fixture.get(User) + project = fixture.get(Project, users=[user]) + another_project = fixture.get(Project, users=[user]) + subproject = fixture.get(Project, users=[user]) + relation = fixture.get( + ProjectRelationship, parent=project, child=subproject, + ) + + form = ProjectRelationshipForm( + {'child': project.pk}, + project=another_project, + user=user, + ) + self.assertFalse(form.is_valid()) + self.assertRegex( + form.errors['child'][0], + 'Select a valid choice', + ) + def test_exclude_self_project_as_subproject(self): user = fixture.get(User) project = fixture.get(Project, users=[user])