Merge pull request #3790 from stsewd/validate-language-change

Additional validation when changing the project language

readthedocs/projects/forms.py

@@ -228,6 +228,36 @@ class Meta(object):
             'tags',
         )
 
+    def clean_language(self):
+        language = self.cleaned_data['language']
+        project = self.instance
+        if project:
+            msg = _(
+                'There is already a "{lang}" translation '
+                'for the {proj} project.'
+            )
+            if project.translations.filter(language=language).exists():
+                raise forms.ValidationError(
+                    msg.format(lang=language, proj=project.slug)
+                )
+            main_project = project.main_language_project
+            if main_project:
+                if main_project.language == language:
+                    raise forms.ValidationError(
+                        msg.format(lang=language, proj=main_project.slug)
+                    )
+                siblings = (
+                    main_project.translations
+                    .filter(language=language)
+                    .exclude(pk=project.pk)
+                    .exists()
+                )
+                if siblings:
+                    raise forms.ValidationError(
+                        msg.format(lang=language, proj=main_project.slug)
+                    )
+        return language
+
 
 class ProjectRelationshipBaseForm(forms.ModelForm):
 

readthedocs/rtd_tests/tests/test_project.py

@@ -6,6 +6,7 @@
 import json
 
 from django.contrib.auth.models import User
+from django.forms.models import model_to_dict
 from django.test import TestCase
 from django_dynamic_fixture import get
 from mock import patch
@@ -258,6 +259,71 @@ def test_user_cant_delete_other_user_translations(self):
         self.assertEqual(resp.status_code, 404)
         self.assertIn(project_b, project_a.translations.all())
 
+    def test_user_cant_change_lang_to_translation_lang(self):
+        user_a = User.objects.get(username='eric')
+        project_a = Project.objects.get(slug='read-the-docs')
+        project_b = get(
+            Project, users=[user_a],
+            language='es', main_language_project=None
+        )
+
+        project_a.translations.add(project_b)
+        project_a.save()
+
+        # User tries to change the language
+        # to the same of the translation
+        self.client.login(username=user_a.username, password='test')
+        self.assertIn(project_b, project_a.translations.all())
+        self.assertEqual(project_a.language, 'en')
+        self.assertEqual(project_b.language, 'es')
+        data = model_to_dict(project_a)
+        data['language'] = 'es'
+        resp = self.client.post(
+            reverse(
+                'projects_edit',
+                args=[project_a.slug]
+            ),
+            data=data,
+            follow=True
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertContains(resp, 'error')
+        self.assertContains(
+            resp,
+            'There is already a "es" translation '
+            'for the read-the-docs project'
+        )
+
+    def test_user_can_change_project_whith_same_lang(self):
+        user_a = User.objects.get(username='eric')
+        project_a = Project.objects.get(slug='read-the-docs')
+        project_b = get(
+            Project, users=[user_a],
+            language='es', main_language_project=None
+        )
+
+        project_a.translations.add(project_b)
+        project_a.save()
+
+        # User save the project with no modifications
+        self.client.login(username=user_a.username, password='test')
+        self.assertIn(project_b, project_a.translations.all())
+        self.assertEqual(project_a.language, 'en')
+        self.assertEqual(project_b.language, 'es')
+        data = model_to_dict(project_a)
+        # Same languge
+        data['language'] = 'en'
+        resp = self.client.post(
+            reverse(
+                'projects_edit',
+                args=[project_a.slug]
+            ),
+            data=data,
+            follow=True
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertNotContains(resp, 'error')
+
     def test_token(self):
         r = self.client.get('/api/v2/project/6/token/', {})
         resp = json.loads(r.content)

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -12,7 +12,7 @@
 
 from readthedocs.projects.exceptions import ProjectSpamError
 from readthedocs.projects.forms import (
-    ProjectBasicsForm, ProjectExtraForm, TranslationForm)
+    ProjectBasicsForm, ProjectExtraForm, TranslationForm, UpdateProjectForm)
 from readthedocs.projects.models import Project
 
 
@@ -260,3 +260,82 @@ def test_not_already_translation(self):
             'is already a translation',
             ''.join(form.errors['project'])
         )
+
+    def test_cant_change_language_to_translation_lang(self):
+        self.project_a_es.translations.add(self.project_b_en)
+        self.project_a_es.translations.add(self.project_c_br)
+        self.project_a_es.save()
+
+        # Parent project tries to change lang
+        form = UpdateProjectForm(
+            {
+                'documentation_type': 'sphinx',
+                'language': 'en',
+            },
+            instance=self.project_a_es
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn(
+            'There is already a "en" translation',
+            ''.join(form.errors['language'])
+        )
+
+        # Translation tries to change lang
+        form = UpdateProjectForm(
+            {
+                'documentation_type': 'sphinx',
+                'language': 'es',
+            },
+            instance=self.project_b_en
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn(
+            'There is already a "es" translation',
+            ''.join(form.errors['language'])
+        )
+
+        # Translation tries to change lang
+        # to the same as its sibling
+        form = UpdateProjectForm(
+            {
+                'documentation_type': 'sphinx',
+                'language': 'br',
+            },
+            instance=self.project_b_en
+        )
+        self.assertFalse(form.is_valid())
+        self.assertIn(
+            'There is already a "br" translation',
+            ''.join(form.errors['language'])
+        )
+
+    def test_can_change_language_to_self_lang(self):
+        self.project_a_es.translations.add(self.project_b_en)
+        self.project_a_es.translations.add(self.project_c_br)
+        self.project_a_es.save()
+
+        # Parent project tries to change lang
+        form = UpdateProjectForm(
+            {
+                'repo': 'https://github.com/test/test',
+                'repo_type': self.project_a_es.repo_type,
+                'name': self.project_a_es.name,
+                'documentation_type': 'sphinx',
+                'language': 'es',
+            },
+            instance=self.project_a_es
+        )
+        self.assertTrue(form.is_valid())
+
+        # Translation tries to change lang
+        form = UpdateProjectForm(
+            {
+                'repo': 'https://github.com/test/test',
+                'repo_type': self.project_b_en.repo_type,
+                'name': self.project_b_en.name,
+                'documentation_type': 'sphinx',
+                'language': 'en',
+            },
+            instance=self.project_b_en
+        )
+        self.assertTrue(form.is_valid())