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

Fix the form for adopting a project #4721

Merged
merged 16 commits into from
Oct 17, 2018
Merged
Show file tree
Hide file tree
Changes from 12 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
13 changes: 13 additions & 0 deletions readthedocs/gold/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django import forms

from readthedocs.payments.forms import StripeModelForm, StripeResourceMixin
from readthedocs.projects.models import Project

from .models import LEVEL_CHOICES, GoldUser

Expand Down Expand Up @@ -88,6 +89,18 @@ def __init__(self, *args, **kwargs):
self.projects = kwargs.pop('projects', None)
super(GoldProjectForm, self).__init__(*args, **kwargs)

def clean_project(self):
cleaned_data = super(GoldProjectForm, self).clean()
Copy link
Member

Choose a reason for hiding this comment

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

You actually don't need to call to this method here, you can just use cleaned_data like this

https://github.com/rtfd/readthedocs.org/blob/90b1e52cfd6a124bf7298a1b3cb1711fddd67ab8/readthedocs/core/forms.py#L59-L65

Copy link
Member Author

Choose a reason for hiding this comment

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

Wokring on it.

project_slug = cleaned_data.get('project', '')
project_instance = Project.objects.filter(slug=project_slug)

# Checking if the project with the entered slug
# is absent or present in the database.
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this comment, is clear what we are checking

Copy link
Member Author

Choose a reason for hiding this comment

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

Working on it.

if not project_instance.exists():
raise forms.ValidationError('No project found.')
Copy link
Member

Choose a reason for hiding this comment

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

We need support translation for this message (using _(msg)) (like the above code)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's it

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks.
though i deleted my comment before seeing your comment.

else:
return project_slug

def clean(self):
cleaned_data = super(GoldProjectForm, self).clean()
if self.projects.count() < self.user.num_supported_projects:
Expand Down
5 changes: 5 additions & 0 deletions readthedocs/rtd_tests/tests/test_gold.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ def test_adding_projects(self):
self.assertEqual(self.golduser.projects.count(), 1)
self.assertEqual(resp.status_code, 302)

def test_incorrect_input_when_adding_projects(self):
self.assertEqual(self.golduser.projects.count(), 0)
resp = self.client.post(reverse('gold_projects'), data={'project': 'random-incorrect-slug'})
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have an assert about that random-incorrect-slug doesn't exists on the db

self.assertFormError(resp, form='form', field='project', errors='No project found.')

def test_too_many_projects(self):
self.project2 = get(Project, slug='test2')

Expand Down