Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional validation when changing the project language #3790

Merged
merged 11 commits into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,29 @@ 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.'
)
format_msg = msg.format(lang=language, proj=project.slug)
if project.translations.filter(language=language).exists():
raise forms.ValidationError(format_msg)
main_project = project.main_language_project
if main_project:
if main_project.language == language:
raise forms.ValidationError(format_msg)
siblings = (main_project.translations
.filter(language=language)
.exclude(pk=project.pk)
.exists())
if siblings:
raise forms.ValidationError(format_msg)
return language


class ProjectRelationshipForm(forms.ModelForm):

Expand Down
66 changes: 66 additions & 0 deletions readthedocs/rtd_tests/tests/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
81 changes: 80 additions & 1 deletion readthedocs/rtd_tests/tests/test_project_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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'])
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, we should test for sibling projects.


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())