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

Remove project's description field #5482

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions docs/api/v2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Project details
"default_branch": "master",
"repo_type": "git",
"repo": "https://github.com/pypa/pip",
"description": "Pip Installs Packages.",
"description": "",
"language": "en",
"documentation_type": "sphinx_htmldir",
"canonical_url": "http://pip.pypa.io/en/stable/",
Expand All @@ -109,7 +109,7 @@ Project details
:>json string default_branch: The default version control branch
:>json string repo_type: Version control repository of the project
:>json string repo: The repository URL for the project
:>json string description: An RST description of the project
:>json string description: An empty string. This will be removed in future.
:>json string language: The language code of this project
:>json string documentation_type: An RST description of the project
:>json string canonical_url: The canonical URL of the default docs
Expand Down
10 changes: 0 additions & 10 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@
from django.utils.safestring import mark_safe
from django.utils.translation import ugettext_lazy as _
from guardian.shortcuts import assign
from textclassifier.validators import ClassifierValidator

from readthedocs.builds.constants import TAG
from readthedocs.core.utils import slugify, trigger_build
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.integrations.models import Integration
from readthedocs.oauth.models import RemoteRepository
from readthedocs.projects import constants
from readthedocs.projects.exceptions import ProjectSpamError
from readthedocs.projects.models import (
Domain,
EmailHook,
Expand Down Expand Up @@ -165,20 +163,13 @@ class ProjectExtraForm(ProjectForm):
class Meta:
model = Project
fields = (
'description',
'documentation_type',
'language',
'programming_language',
'tags',
'project_url',
)

description = forms.CharField(
validators=[ClassifierValidator(raises=ProjectSpamError)],
required=False,
widget=forms.Textarea,
)

def clean_tags(self):
tags = self.cleaned_data.get('tags', [])
for tag in tags:
Expand Down Expand Up @@ -299,7 +290,6 @@ class Meta:
'repo',
'repo_type',
# Extra
'description',
'language',
'programming_language',
'project_url',
Expand Down
3 changes: 1 addition & 2 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,7 @@ def post(self, request, *args, **kwargs):
for key in ['name', 'repo', 'repo_type', 'remote_repository']:
initial_data['basics'][key] = request.POST.get(key)
initial_data['extra'] = {}
for key in ['description', 'project_url']:
initial_data['extra'][key] = request.POST.get(key)
initial_data['extra']['project_url'] = request.POST.get(key)
request.method = 'GET'
return self.wizard_class.as_view(initial_dict=initial_data)(request)

Expand Down
9 changes: 9 additions & 0 deletions readthedocs/restapi/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@

class ProjectSerializer(serializers.ModelSerializer):
canonical_url = serializers.ReadOnlyField(source='get_docs_url')
description = serializers.SerializerMethodField()

def get_description(self, obj):
"""
Forcing the description field to be an empty string.

See https://github.com/rtfd/readthedocs.org/issues/3689.
Copy link
Member Author

Choose a reason for hiding this comment

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

Forcing the description to be an empty string in the API response. This was done as per this comment.

"""
return ''

class Meta:
model = Project
Expand Down
16 changes: 0 additions & 16 deletions readthedocs/rtd_tests/tests/test_project_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from django.test import TestCase
from django.test.utils import override_settings
from django_dynamic_fixture import get
from textclassifier.validators import ClassifierValidator

from readthedocs.builds.constants import LATEST
from readthedocs.builds.models import Version
Expand All @@ -14,7 +13,6 @@
REPO_TYPE_GIT,
REPO_TYPE_HG,
)
from readthedocs.projects.exceptions import ProjectSpamError
from readthedocs.projects.forms import (
EnvironmentVariableForm,
ProjectAdvancedForm,
Expand All @@ -30,20 +28,6 @@

class TestProjectForms(TestCase):

@mock.patch.object(ClassifierValidator, '__call__')
def test_form_spam(self, mocked_validator):
"""Form description field fails spam validation."""
mocked_validator.side_effect = ProjectSpamError

data = {
'description': 'foo',
'documentation_type': 'sphinx',
'language': 'en',
}
form = ProjectExtraForm(data)
with self.assertRaises(ProjectSpamError):
form.is_valid()

def test_import_repo_url(self):
"""Validate different type of repository URLs on importing a Project."""

Expand Down
50 changes: 0 additions & 50 deletions readthedocs/rtd_tests/tests/test_project_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def setUp(self):
'repo_type': 'git',
},
'extra': {
'description': 'Describe foobar',
'language': 'en',
'documentation_type': 'sphinx',
},
Expand Down Expand Up @@ -151,7 +150,6 @@ def setUp(self):
super().setUp()
self.step_data['basics']['advanced'] = True
self.step_data['extra'] = {
'description': 'Describe foobar',
'language': 'en',
'documentation_type': 'sphinx',
'tags': 'foo, bar, baz',
Expand Down Expand Up @@ -205,54 +203,6 @@ def test_remote_repository_is_added(self):
self.assertIsNotNone(proj)
self.assertEqual(proj.remote_repository, remote_repo)

@patch(
'readthedocs.projects.views.private.ProjectExtraForm.clean_description',
create=True,
)
def test_form_spam(self, mocked_validator):
"""Don't add project on a spammy description."""
self.user.date_joined = timezone.now() - timedelta(days=365)
self.user.save()
mocked_validator.side_effect = ProjectSpamError

with self.assertRaises(Project.DoesNotExist):
proj = Project.objects.get(name='foobar')

resp = self.post_step('basics')
self.assertWizardResponse(resp, 'extra')
resp = self.post_step('extra', session=list(resp._request.session.items()))
self.assertIsInstance(resp, HttpResponseRedirect)
self.assertEqual(resp.status_code, 302)
self.assertEqual(resp['location'], '/')

with self.assertRaises(Project.DoesNotExist):
proj = Project.objects.get(name='foobar')
self.assertFalse(self.user.profile.banned)

@patch(
'readthedocs.projects.views.private.ProjectExtraForm.clean_description',
create=True,
)
def test_form_spam_ban_user(self, mocked_validator):
"""Don't add spam and ban new user."""
self.user.date_joined = timezone.now()
self.user.save()
mocked_validator.side_effect = ProjectSpamError

with self.assertRaises(Project.DoesNotExist):
proj = Project.objects.get(name='foobar')

resp = self.post_step('basics')
self.assertWizardResponse(resp, 'extra')
resp = self.post_step('extra', session=list(resp._request.session.items()))
self.assertIsInstance(resp, HttpResponseRedirect)
self.assertEqual(resp.status_code, 302)
self.assertEqual(resp['location'], '/')

with self.assertRaises(Project.DoesNotExist):
proj = Project.objects.get(name='foobar')
self.assertTrue(self.user.profile.banned)


class TestImportDemoView(MockBuildTestCase):
"""Test project import demo view."""
Expand Down
1 change: 0 additions & 1 deletion readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ def INSTALLED_APPS(self): # noqa
'django_gravatar',
'rest_framework',
'corsheaders',
'textclassifier',
'annoying',
'django_extensions',
'crispy_forms',
Expand Down
9 changes: 0 additions & 9 deletions readthedocs/templates/core/project_details.html
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,6 @@ <h3>{% trans "Build a version" %}</h3>
{% endif %}
{% endblock %}

{% if project.description %}
<div id="project_description">
<h3>{% trans "Description" %}</h3>
<p>
{{ project.description|restructuredtext }}
</p>
</div>
{% endif %}

</div>{# END .module #}

<div class="project_details">
Expand Down
11 changes: 0 additions & 11 deletions readthedocs/templates/projects/index.rst.html

This file was deleted.

1 change: 0 additions & 1 deletion requirements/pip.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ django-crispy-forms==1.7.2
# https://github.com/rtfd/readthedocs.org/issues/3999
docker==3.1.3 # pyup: ignore

django-textclassifier==1.0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not needed anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is used for spam project detection. I do worry that we'd get more spam if we don't run any detection. Perhaps we could run it on the name of the project. @agjohnson?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidfischer
There's an open PR for this. #5038

django-annoying==0.10.4
django-messages-extends==0.6.0
djangorestframework-jsonp==1.0.2
Expand Down