Repository: readthedocs/readthedocs.org
Issue: Can't update subproject alias


* Add a subproject with alias
* Edit new subproject
* Change alias and save

The alias should update, but instead an error is thrown about the "Child is already a subproject of another project"

Also, that error is engineer speak. The error should be a user friend "Cannot add subproject, it is already a subproject of another project" or something similar.

Changed files:
readthedocs/projects/forms.py, readthedocs/rtd_tests/tests/test_subprojects.py

diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py
index 853470838..9d5bb0744 100644
--- a/readthedocs/projects/forms.py
+++ b/readthedocs/projects/forms.py
@@ -375,17 +375,26 @@ class ProjectRelationshipBaseForm(forms.ModelForm):
         return self.project
 
     def clean_child(self):
-        child = self.cleaned_data['child']
+        """
+        Validate child is a valid subproject.
 
-        child.is_valid_as_subproject(
-            self.project, forms.ValidationError
-        )
+        Validation is done on creation only,
+        when editing users can't change the child.
+        """
+        child = self.cleaned_data['child']
+        if self.instance.pk is None:
+            child.is_valid_as_subproject(
+                self.project, forms.ValidationError
+            )
         return child
 
     def clean_alias(self):
         alias = self.cleaned_data['alias']
-        subproject = self.project.subprojects.filter(
-            alias=alias).exclude(id=self.instance.pk)
+        subproject = (
+            self.project.subprojects
+            .filter(alias=alias)
+            .exclude(id=self.instance.pk)
+        )
 
         if subproject.exists():
             raise forms.ValidationError(
diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py
index 5da03be42..b2deaca2e 100644
--- a/readthedocs/rtd_tests/tests/test_subprojects.py
+++ b/readthedocs/rtd_tests/tests/test_subprojects.py
@@ -1,6 +1,6 @@
-# -*- coding: utf-8 -*-
-import django_dynamic_fixture as fixture
 from unittest import mock
+
+import django_dynamic_fixture as fixture
 from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -12,6 +12,13 @@ from readthedocs.rtd_tests.utils import create_user
 
 class SubprojectFormTests(TestCase):
 
+    def setUp(self):
+        self.user = fixture.get(User)
+        self.project = fixture.get(Project, users=[self.user])
+        self.subproject = fixture.get(Project, users=[self.user])
+        self.project.add_subproject(self.subproject, 'subproject')
+        self.relation = self.subproject.superprojects.first()
+
     def test_empty_child(self):
         user = fixture.get(User)
         project = fixture.get(Project, slug='mainproject')
@@ -254,3 +261,53 @@ class SubprojectFormTests(TestCase):
             [proj_id for (proj_id, __) in form.fields['child'].choices],
             ['', relation.child.id],
         )
+
+    def test_change_alias(self):
+        subproject_2 = fixture.get(Project, users=[self.user])
+        self.project.add_subproject(subproject_2, 'another-subproject')
+
+        relation = subproject_2.superprojects.first()
+
+        # Change alias to an existing alias
+        form = ProjectRelationshipForm(
+            {
+                'child': subproject_2.id,
+                'alias': 'subproject'
+            },
+            instance=relation,
+            project=self.project,
+            user=self.user,
+        )
+        self.assertFalse(form.is_valid())
+        error_msg = 'A subproject with this alias already exists'
+        self.assertDictEqual(form.errors, {'alias': [error_msg]})
+
+        # Change alias to a new alias
+        form = ProjectRelationshipForm(
+            {
+                'child': subproject_2.id,
+                'alias': 'other-subproject'
+            },
+            instance=relation,
+            project=self.project,
+            user=self.user,
+        )
+        self.assertTrue(form.is_valid())
+        form.save()
+        relation.refresh_from_db()
+        self.assertEqual(relation.alias, 'other-subproject')
+
+    def test_change_alias_to_same_alias(self):
+        form = ProjectRelationshipForm(
+            {
+                'child': self.subproject.id,
+                'alias': 'subproject'
+            },
+            instance=self.relation,
+            project=self.project,
+            user=self.user,
+        )
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.relation.refresh_from_db()
+        self.assertEqual(self.relation.alias, 'subproject')