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

core: fix MultipleObjectsReturned in views.serve.serve.docs #4356

Closed
wants to merge 1 commit 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
18 changes: 10 additions & 8 deletions readthedocs/core/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,24 +201,26 @@ def serve_docs(
"""Map existing proj, lang, version, filename views to the file format."""
if not version_slug:
version_slug = project.get_default_version()

try:
version = project.versions.public(request.user).get(slug=version_slug)
version = project.versions.get(slug=version_slug)
Copy link
Member

Choose a reason for hiding this comment

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

This change needs to be looked at carefully since .public has a different meaning than .get under our corporate site.

I'm not saying that it's wrong, but just making notes about it when proper review.

except Version.DoesNotExist:
# Properly raise a 404 if the version doesn't exist (or is inactive) and
# a 401 if it does
if project.versions.filter(slug=version_slug, active=True).exists():
return _serve_401(request, project)
raise Http404('Version does not exist.')

# only project admins should see private versions
if not AdminPermission.is_member(user=request.user, obj=project):
if version.privacy_level == constants.PRIVATE:
return _serve_401(request, project)
if not version.active:
raise Http404('Version does not exist.')

filename = resolve_path(
subproject or project, # Resolve the subproject if it exists
version_slug=version_slug,
language=lang_slug,
filename=filename,
subdomain=True, # subdomain will make it a "full" path without a URL prefix
)
if (version.privacy_level == constants.PRIVATE and
not AdminPermission.is_member(user=request.user, obj=project)):
return _serve_401(request, project)
return _serve_symlink_docs(
request,
filename=filename,
Expand Down
24 changes: 21 additions & 3 deletions readthedocs/rtd_tests/tests/test_redirects.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging

from django.http import Http404
from django.http import Http404, HttpResponse
from django.contrib.auth.models import User
from django.test import TestCase
from django.test.utils import override_settings
from django_dynamic_fixture import fixture, get
Expand Down Expand Up @@ -45,8 +46,25 @@ def test_proper_url(self):
r = self.client.get('/docs/pip/')
self.assertEqual(r.status_code, 302)
self.assertEqual(
r['Location'], 'http://readthedocs.org/docs/pip/en/latest/',
)
r['Location'], 'http://readthedocs.org/docs/pip/en/latest/')

def test_owner_with_multiple_projects_can_serve_the_requested_version(self):
# GH #4350
eric = User.objects.get(username='eric')
notpip = eric.projects.exclude(slug='pip').first()
notpip.versions.create_latest()

self.client.login(username='eric', password='test')
with patch('readthedocs.core.views.serve._serve_symlink_docs') as _serve_docs:
_serve_docs.return_value = HttpResponse()
self.client.get('/docs/pip/en/latest/')
self.assertTrue(_serve_docs.called)

def test_inactive_docs_should_return_404_if_the_user_is_not_the_project_admin(self):
pip = Project.objects.get(slug='pip')
pip.versions.filter(slug='latest').update(active=False)
response = self.client.get('/docs/pip/en/latest/')
self.assertEqual(response.status_code, 404)

# Specific Page Redirects
def test_proper_page_on_main_site(self):
Expand Down