diff --git a/media/css/core.css b/media/css/core.css index 6e921acff5d..4b36e031890 100644 --- a/media/css/core.css +++ b/media/css/core.css @@ -1102,6 +1102,12 @@ div.httpexchange div.highlight pre { font-size: .9em; } +/* Subprojects */ +div.module.project-subprojects div.subproject-meta { + font-size: .9em; + font-style: italic; +} + /* Pygments */ div.highlight pre .hll { background-color: #ffffcc } div.highlight pre .c { color: #60a0b0; font-style: italic } /* Comment */ diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 910b475cbd0..643b2b6fac6 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -21,7 +21,8 @@ from readthedocs.oauth.models import RemoteRepository from readthedocs.projects import constants from readthedocs.projects.exceptions import ProjectSpamError -from readthedocs.projects.models import Project, EmailHook, WebHook, Domain +from readthedocs.projects.models import ( + Project, ProjectRelationship, EmailHook, WebHook, Domain) from readthedocs.redirects.models import Redirect from future import standard_library @@ -236,6 +237,46 @@ class Meta(object): ) +class ProjectRelationshipForm(forms.ModelForm): + + """Form to add/update project relationships""" + + parent = forms.CharField(widget=forms.HiddenInput(), required=False) + + class Meta(object): + model = ProjectRelationship + exclude = [] + + def __init__(self, *args, **kwargs): + self.project = kwargs.pop('project') + self.user = kwargs.pop('user') + super(ProjectRelationshipForm, self).__init__(*args, **kwargs) + # Don't display the update form with an editable child, as it will be + # filtered out from the queryset anyways. + if hasattr(self, 'instance') and self.instance.pk is not None: + self.fields['child'].disabled = True + else: + self.fields['child'].queryset = self.get_subproject_queryset() + + def clean_parent(self): + if self.project.superprojects.exists(): + # This validation error is mostly for testing, users shouldn't see + # this in normal circumstances + raise forms.ValidationError(_("Subproject nesting is not supported")) + return self.project + + def get_subproject_queryset(self): + """Return scrubbed subproject choice queryset + + This removes projects that are either already a subproject of another + project, or are a superproject, as neither case is supported. + """ + queryset = (Project.objects.for_admin_user(self.user) + .exclude(subprojects__isnull=False) + .exclude(superprojects__isnull=False)) + return queryset + + class DualCheckboxWidget(forms.CheckboxInput): """Checkbox with link to the version's built documentation""" @@ -364,44 +405,6 @@ def build_upload_html_form(project): return type('UploadHTMLForm', (BaseUploadHTMLForm,), attrs) -class SubprojectForm(forms.Form): - - """Project subproject form""" - - subproject = forms.CharField() - alias = forms.CharField(required=False) - - def __init__(self, *args, **kwargs): - self.user = kwargs.pop('user') - self.parent = kwargs.pop('parent') - super(SubprojectForm, self).__init__(*args, **kwargs) - - def clean_subproject(self): - """Normalize subproject field - - Does lookup on against :py:class:`Project` to ensure matching project - exists. Return the :py:class:`Project` object instead. - """ - subproject_name = self.cleaned_data['subproject'] - subproject_qs = Project.objects.filter(slug=subproject_name) - if not subproject_qs.exists(): - raise forms.ValidationError((_("Project %(name)s does not exist") - % {'name': subproject_name})) - subproject = subproject_qs.first() - if not AdminPermission.is_admin(self.user, subproject): - raise forms.ValidationError(_( - 'You need to be admin of {name} in order to add it as ' - 'a subproject.'.format(name=subproject_name))) - return subproject - - def save(self): - relationship = self.parent.add_subproject( - self.cleaned_data['subproject'], - alias=self.cleaned_data['alias'], - ) - return relationship - - class UserForm(forms.Form): """Project user association form""" diff --git a/readthedocs/projects/urls/private.py b/readthedocs/projects/urls/private.py index cafe30c4061..7033e71a566 100644 --- a/readthedocs/projects/urls/private.py +++ b/readthedocs/projects/urls/private.py @@ -64,14 +64,6 @@ private.project_delete, name='projects_delete'), - url(r'^(?P[-\w]+)/subprojects/delete/(?P[-\w]+)/$', # noqa - private.project_subprojects_delete, - name='projects_subprojects_delete'), - - url(r'^(?P[-\w]+)/subprojects/$', - private.project_subprojects, - name='projects_subprojects'), - url(r'^(?P[-\w]+)/users/$', private.project_users, name='projects_users'), @@ -165,3 +157,25 @@ ] urlpatterns += integration_urls + +subproject_urls = [ + url(r'^(?P{project_slug})/subprojects/$'.format(**pattern_opts), + private.ProjectRelationshipList.as_view(), + name='projects_subprojects'), + url((r'^(?P{project_slug})/subprojects/create/$' + .format(**pattern_opts)), + private.ProjectRelationshipCreate.as_view(), + name='projects_subprojects_create'), + url((r'^(?P{project_slug})/' + r'subprojects/(?P{project_slug})/edit/$' + .format(**pattern_opts)), + private.ProjectRelationshipUpdate.as_view(), + name='projects_subprojects_update'), + url((r'^(?P{project_slug})/' + r'subprojects/(?P{project_slug})/delete/$' + .format(**pattern_opts)), + private.ProjectRelationshipDelete.as_view(), + name='projects_subprojects_delete'), +] + +urlpatterns += subproject_urls diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 8a25849d3e8..6c09304dc62 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -28,12 +28,13 @@ from readthedocs.core.mixins import ListViewWithForm from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.projects.forms import ( - ProjectBasicsForm, ProjectExtraForm, - ProjectAdvancedForm, UpdateProjectForm, SubprojectForm, + ProjectBasicsForm, ProjectExtraForm, ProjectAdvancedForm, + UpdateProjectForm, ProjectRelationshipForm, build_versions_form, UserForm, EmailHookForm, TranslationForm, RedirectForm, WebHookForm, DomainForm, IntegrationForm, ProjectAdvertisingForm) -from readthedocs.projects.models import Project, EmailHook, WebHook, Domain +from readthedocs.projects.models import ( + Project, ProjectRelationship, EmailHook, WebHook, Domain) from readthedocs.projects.views.base import ProjectAdminMixin, ProjectSpamMixin from readthedocs.projects import tasks from readthedocs.oauth.services import registry @@ -398,7 +399,7 @@ def edit_alias(request, project_slug, alias_id=None): class AliasList(PrivateViewMixin, ListView): model = VersionAlias template_context_name = 'alias' - template_name = 'projects/alias_list.html', + template_name = 'projects/alias_list.html' def get_queryset(self): self.project = get_object_or_404( @@ -407,44 +408,50 @@ def get_queryset(self): return self.project.aliases.all() -@login_required -def project_subprojects(request, project_slug): - """Project subprojects view and form view""" - project = get_object_or_404(Project.objects.for_admin_user(request.user), - slug=project_slug) +class ProjectRelationshipMixin(ProjectAdminMixin, PrivateViewMixin): - form_kwargs = { - 'parent': project, - 'user': request.user, - } - if request.method == 'POST': - form = SubprojectForm(request.POST, **form_kwargs) - if form.is_valid(): - form.save() - broadcast(type='app', task=tasks.symlink_subproject, args=[project.pk]) - project_dashboard = reverse( - 'projects_subprojects', args=[project.slug]) - return HttpResponseRedirect(project_dashboard) - else: - form = SubprojectForm(**form_kwargs) + model = ProjectRelationship + form_class = ProjectRelationshipForm + lookup_field = 'child__slug' + lookup_url_kwarg = 'subproject_slug' - subprojects = project.subprojects.all() + def get_queryset(self): + self.project = self.get_project() + return self.model.objects.filter(parent=self.project) - return render_to_response( - 'projects/project_subprojects.html', - {'form': form, 'project': project, 'subprojects': subprojects}, - context_instance=RequestContext(request) - ) + def get_form(self, data=None, files=None, **kwargs): + kwargs['user'] = self.request.user + return super(ProjectRelationshipMixin, self).get_form(data, files, **kwargs) + def form_valid(self, form): + broadcast(type='app', task=tasks.symlink_subproject, + args=[self.get_project().pk]) + return super(ProjectRelationshipMixin, self).form_valid(form) -@login_required -def project_subprojects_delete(request, project_slug, child_slug): - parent = get_object_or_404(Project.objects.for_admin_user(request.user), slug=project_slug) - child = get_object_or_404(Project.objects.all(), slug=child_slug) - parent.remove_subproject(child) - broadcast(type='app', task=tasks.symlink_subproject, args=[parent.pk]) - return HttpResponseRedirect(reverse('projects_subprojects', - args=[parent.slug])) + def get_success_url(self): + return reverse('projects_subprojects', args=[self.get_project().slug]) + + +class ProjectRelationshipList(ProjectRelationshipMixin, ListView): + + def get_context_data(self, **kwargs): + ctx = super(ProjectRelationshipList, self).get_context_data(**kwargs) + ctx['superproject'] = self.project.superprojects.first() + return ctx + + +class ProjectRelationshipCreate(ProjectRelationshipMixin, CreateView): + pass + + +class ProjectRelationshipUpdate(ProjectRelationshipMixin, UpdateView): + pass + + +class ProjectRelationshipDelete(ProjectRelationshipMixin, DeleteView): + + def get(self, request, *args, **kwargs): + return self.http_method_not_allowed(request, *args, **kwargs) @login_required diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index b0152ef29c8..b9fa7c0eaeb 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -142,6 +142,7 @@ def setUp(self): self.domain = get(Domain, url='http://docs.foobar.com', project=self.pip) self.default_kwargs = { 'project_slug': self.pip.slug, + 'subproject_slug': self.subproject.slug, 'version_slug': self.pip.versions.all()[0].slug, 'filename': 'index.html', 'type_': 'pdf', @@ -227,6 +228,7 @@ class PrivateProjectAdminAccessTest(PrivateProjectMixin, TestCase): '/dashboard/pip/users/delete/': {'status_code': 405}, '/dashboard/pip/notifications/delete/': {'status_code': 405}, '/dashboard/pip/redirects/delete/': {'status_code': 405}, + '/dashboard/pip/subprojects/sub/delete/': {'status_code': 405}, '/dashboard/pip/integrations/sync/': {'status_code': 405}, '/dashboard/pip/integrations/1/sync/': {'status_code': 405}, '/dashboard/pip/integrations/1/delete/': {'status_code': 405}, @@ -255,6 +257,7 @@ class PrivateProjectUserAccessTest(PrivateProjectMixin, TestCase): '/dashboard/pip/users/delete/': {'status_code': 405}, '/dashboard/pip/notifications/delete/': {'status_code': 405}, '/dashboard/pip/redirects/delete/': {'status_code': 405}, + '/dashboard/pip/subprojects/sub/delete/': {'status_code': 405}, '/dashboard/pip/integrations/sync/': {'status_code': 405}, '/dashboard/pip/integrations/1/sync/': {'status_code': 405}, '/dashboard/pip/integrations/1/delete/': {'status_code': 405}, diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 575dec1304c..7db6f696759 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -381,6 +381,21 @@ def test_delete_project(self): task=tasks.remove_dir, args=[project.doc_path]) + def test_subproject_create(self): + project = get(Project, slug='pip', users=[self.user]) + subproject = get(Project, users=[self.user]) + + with patch('readthedocs.projects.views.private.broadcast') as broadcast: + response = self.client.post( + '/dashboard/pip/subprojects/create/', + data={'child': subproject.pk}, + ) + self.assertEqual(response.status_code, 302) + broadcast.assert_called_with( + type='app', + task=tasks.symlink_subproject, + args=[project.pk]) + class TestPrivateMixins(MockBuildTestCase): diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index 757a59891c7..d64d3ee3d6b 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -1,66 +1,147 @@ from __future__ import absolute_import -import mock +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 -from django.contrib.auth.models import User -from readthedocs.projects.forms import SubprojectForm -from readthedocs.projects.models import Project +from readthedocs.projects.forms import ProjectRelationshipForm +from readthedocs.projects.models import Project, ProjectRelationship from readthedocs.rtd_tests.utils import create_user -from django_dynamic_fixture import get - class SubprojectFormTests(TestCase): - def test_name_validation(self): - user = get(User) - project = get(Project, slug='mainproject') - - form = SubprojectForm({}, - parent=project, user=user) + def test_empty_child(self): + user = fixture.get(User) + project = fixture.get(Project, slug='mainproject') + form = ProjectRelationshipForm( + {}, + project=project, + user=user + ) form.full_clean() - self.assertTrue('subproject' in form.errors) - - form = SubprojectForm({'name': 'not-existent'}, - parent=project, user=user) + self.assertEqual(len(form.errors['child']), 1) + self.assertRegexpMatches( + form.errors['child'][0], + r'This field is required.' + ) + + def test_nonexistent_child(self): + user = fixture.get(User) + project = fixture.get(Project, slug='mainproject') + self.assertFalse(Project.objects.filter(pk=9999).exists()) + form = ProjectRelationshipForm( + {'child': 9999}, + project=project, + user=user + ) form.full_clean() - self.assertTrue('subproject' in form.errors) + self.assertEqual(len(form.errors['child']), 1) + self.assertRegexpMatches( + form.errors['child'][0], + r'Select a valid choice.' + ) def test_adding_subproject_fails_when_user_is_not_admin(self): - # Make sure that a user cannot add a subproject that he is not the - # admin of. - - user = get(User) - project = get(Project, slug='mainproject') + user = fixture.get(User) + project = fixture.get(Project, slug='mainproject') project.users.add(user) - subproject = get(Project, slug='subproject') - - form = SubprojectForm({'subproject': subproject.slug}, - parent=project, user=user) - # Fails because user does not own subproject. + subproject = fixture.get(Project, slug='subproject') + self.assertQuerysetEqual( + Project.objects.for_admin_user(user), + [project], + transform=lambda n: n, + ) + form = ProjectRelationshipForm( + {'child': subproject.pk}, + project=project, + user=user + ) form.full_clean() - self.assertTrue('subproject' in form.errors) - - def test_admin_of_subproject_can_add_it(self): - user = get(User) - project = get(Project, slug='mainproject') + self.assertEqual(len(form.errors['child']), 1) + self.assertRegexpMatches( + form.errors['child'][0], + r'Select a valid choice.' + ) + + def test_adding_subproject_passes_when_user_is_admin(self): + user = fixture.get(User) + project = fixture.get(Project, slug='mainproject') project.users.add(user) - subproject = get(Project, slug='subproject') + subproject = fixture.get(Project, slug='subproject') subproject.users.add(user) - - # Works now as user is admin of subproject. - form = SubprojectForm({'subproject': subproject.slug}, - parent=project, user=user) - # Fails because user does not own subproject. + self.assertQuerysetEqual( + Project.objects.for_admin_user(user), + [project, subproject], + transform=lambda n: n, + ) + form = ProjectRelationshipForm( + {'child': subproject.pk}, + project=project, + user=user + ) form.full_clean() self.assertTrue(form.is_valid()) form.save() - self.assertEqual( [r.child for r in project.subprojects.all()], - [subproject]) + [subproject] + ) + + def test_subproject_form_cant_create_sub_sub_project(self): + user = fixture.get(User) + project = fixture.get(Project, users=[user]) + subproject = fixture.get(Project, users=[user]) + subsubproject = fixture.get(Project, users=[user]) + relation = fixture.get( + ProjectRelationship, parent=project, child=subproject + ) + self.assertQuerysetEqual( + Project.objects.for_admin_user(user), + [project, subproject, subsubproject], + transform=lambda n: n, + ) + form = ProjectRelationshipForm( + {'child': subsubproject.pk}, + project=subproject, + user=user + ) + # The subsubproject is valid here, as far as the child check is + # concerned, but the parent check should fail. + self.assertEqual( + [proj_id for (proj_id, __) in form.fields['child'].choices], + ['', subsubproject.pk], + ) + form.full_clean() + self.assertEqual(len(form.errors['parent']), 1) + self.assertRegexpMatches( + form.errors['parent'][0], + r'Subproject nesting is not supported' + ) + + def test_excludes_existing_subprojects(self): + user = fixture.get(User) + project = fixture.get(Project, users=[user]) + subproject = fixture.get(Project, users=[user]) + relation = fixture.get( + ProjectRelationship, parent=project, child=subproject + ) + self.assertQuerysetEqual( + Project.objects.for_admin_user(user), + [project, subproject], + transform=lambda n: n, + ) + form = ProjectRelationshipForm( + {'child': subproject.pk}, + project=project, + user=user + ) + self.assertEqual( + [proj_id for (proj_id, __) in form.fields['child'].choices], + [''], + ) @override_settings(PUBLIC_DOMAIN='readthedocs.org') @@ -70,11 +151,13 @@ def setUp(self): with mock.patch('readthedocs.projects.models.broadcast'): self.owner = create_user(username='owner', password='test') self.tester = create_user(username='tester', password='test') - self.pip = get(Project, slug='pip', users=[self.owner], main_language_project=None) - self.subproject = get(Project, slug='sub', language='ja', users=[ - self.owner], main_language_project=None) - self.translation = get(Project, slug='trans', language='ja', users=[ - self.owner], main_language_project=None) + self.pip = fixture.get(Project, slug='pip', users=[self.owner], main_language_project=None) + self.subproject = fixture.get(Project, slug='sub', language='ja', + users=[ self.owner], + main_language_project=None) + self.translation = fixture.get(Project, slug='trans', language='ja', + users=[ self.owner], + main_language_project=None) self.pip.add_subproject(self.subproject) self.pip.translations.add(self.translation) diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index 8d9256c74da..a3fdad5f4d8 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -113,8 +113,13 @@ def test_project_delete(self): self.assertRedirectToLogin(response) def test_subprojects_delete(self): + # This URL doesn't exist anymore, 404 response = self.client.get( '/dashboard/pip/subprojects/delete/a-subproject/') + self.assertEqual(response.status_code, 404) + # New URL + response = self.client.get( + '/dashboard/pip/subprojects/a-subproject/delete/') self.assertRedirectToLogin(response) def test_subprojects(self): @@ -213,7 +218,18 @@ def test_admins_can_delete_subprojects(self): self.project.users.add(self.user) self.subproject.users.add(self.user) - response = self.client.get('/dashboard/my-mainproject/subprojects/delete/my-subproject/') + # URL doesn't exist anymore, 404 + response = self.client.get( + '/dashboard/my-mainproject/subprojects/delete/my-subproject/') + self.assertEqual(response.status_code, 404) + # This URL still doesn't accept GET, 405 + response = self.client.get( + '/dashboard/my-mainproject/subprojects/my-subproject/delete/') + self.assertEqual(response.status_code, 405) + self.assertTrue(self.subproject in [r.child for r in self.project.subprojects.all()]) + # Test POST + response = self.client.post( + '/dashboard/my-mainproject/subprojects/my-subproject/delete/') self.assertEqual(response.status_code, 302) self.assertTrue(self.subproject not in [r.child for r in self.project.subprojects.all()]) @@ -221,6 +237,7 @@ def test_project_admins_can_delete_subprojects_that_they_are_not_admin_of(self): self.project.users.add(self.user) self.assertFalse(AdminPermission.is_admin(self.user, self.subproject)) - response = self.client.get('/dashboard/my-mainproject/subprojects/delete/my-subproject/') + response = self.client.post( + '/dashboard/my-mainproject/subprojects/my-subproject/delete/') self.assertEqual(response.status_code, 302) self.assertTrue(self.subproject not in [r.child for r in self.project.subprojects.all()]) diff --git a/readthedocs/templates/projects/project_subprojects.html b/readthedocs/templates/projects/project_subprojects.html deleted file mode 100644 index eb07b52c6d1..00000000000 --- a/readthedocs/templates/projects/project_subprojects.html +++ /dev/null @@ -1,54 +0,0 @@ -{% extends "projects/project_edit_base.html" %} - -{% load i18n %} - -{% block title %}{% trans "Edit Subprojects" %}{% endblock %} - -{% block nav-dashboard %} class="active"{% endblock %} - -{% block editing-option-edit-proj %}class="active"{% endblock %} - -{% block project-subprojects-active %}active{% endblock %} -{% block project_edit_content_header %}{% trans "Subprojects" %}{% endblock %} - -{% block project_edit_content %} -

- {% trans "This allows you to add subprojects to your project. This allows them to live in the same namespace in the URLConf for a subdomain or CNAME." %} -

- -

{% trans "Existing Subprojects" %}

-

-

-

- {% trans "Choose which project you would like to add as a subproject." %} -

-
{% csrf_token %} - {{ form.as_p }} -

- -

-
-{% endblock %} - - -{% block footerjs %} - $('#id_subproject').autocomplete({ - source: '{% url "search_autocomplete" %}', - minLength: 2, - open: function(event, ui) { - ac_top = $('.ui-autocomplete').css('top'); - $('.ui-autocomplete').css({'width': '233px', 'top': ac_top + 10 }); - } - }); - -{% endblock %} diff --git a/readthedocs/templates/projects/projectrelationship_form.html b/readthedocs/templates/projects/projectrelationship_form.html new file mode 100644 index 00000000000..566b5fde8d0 --- /dev/null +++ b/readthedocs/templates/projects/projectrelationship_form.html @@ -0,0 +1,41 @@ +{% extends "projects/project_edit_base.html" %} + +{% load i18n %} + +{% block title %}{% trans "Subprojects" %}{% endblock %} + +{% block nav-dashboard %} class="active"{% endblock %} + +{% block editing-option-edit-proj %}class="active"{% endblock %} + +{% block project-subprojects-active %}active{% endblock %} +{% block project_edit_content_header %}{% trans "Subprojects" %}{% endblock %} + +{% block project_edit_content %} + {% if object %} +
+ {% csrf_token %} + {{ form.as_p }} + +
+ +
+ {% csrf_token %} + +
+ {% else %} +
+ {% csrf_token %} + {{ form.as_p }} + +
+ {% endif %} +{% endblock %} diff --git a/readthedocs/templates/projects/projectrelationship_list.html b/readthedocs/templates/projects/projectrelationship_list.html new file mode 100644 index 00000000000..a5aca6a2753 --- /dev/null +++ b/readthedocs/templates/projects/projectrelationship_list.html @@ -0,0 +1,78 @@ +{% extends "projects/project_edit_base.html" %} + +{% load i18n %} + +{% block title %}{% trans "Subprojects" %}{% endblock %} + +{% block nav-dashboard %} class="active"{% endblock %} + +{% block editing-option-edit-proj %}class="active"{% endblock %} + +{% block project-subprojects-active %}active{% endblock %} +{% block project_edit_content_header %}{% trans "Subprojects" %}{% endblock %} + +{% block project_edit_content %} +

+ {% blocktrans trimmed %} + Subprojects are projects hosted from the same URL as their parent project. + This is useful for organizing multiple projects under a custom domain. + {% endblocktrans %} +

+ + {% if superproject %} +

+ {% blocktrans trimmed with project=superproject.parent.name %} + This project is already configured as a subproject of {{ project }}. + Nested subprojects are not currently supported. + {% endblocktrans %} +

+ + + {% blocktrans trimmed with project=superproject.parent.name %} + View subprojects of {{ project }} + {% endblocktrans %} + + {% else %} + + +
+
+ +
+
+ {% endif %} +{% endblock %}