From 5162f8524777491e01427d6c42d4db08461e72f8 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 22 Aug 2018 14:47:37 -0700 Subject: [PATCH 1/5] Use STATIC_URL for badges --- readthedocs/projects/views/public.py | 9 ++++----- readthedocs/rtd_tests/tests/test_project_views.py | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index 6a6d7d95ddd..093d9b24ded 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -15,7 +15,6 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth.models import User -from django.contrib.staticfiles.templatetags.staticfiles import static from django.core.cache import cache from django.core.urlresolvers import reverse from django.http import Http404, HttpResponse, HttpResponseRedirect @@ -121,18 +120,18 @@ def project_badge(request, project_slug): version = Version.objects.public(request.user).get( project__slug=project_slug, slug=version_slug) except Version.DoesNotExist: - url = static(badge_path % 'unknown') + url = settings.STATIC_URL + (badge_path % 'unknown') return HttpResponseRedirect(url) version_builds = version.builds.filter(type='html', state='finished').order_by('-date') if not version_builds.exists(): - url = static(badge_path % 'unknown') + url = settings.STATIC_URL + (badge_path % 'unknown') return HttpResponseRedirect(url) last_build = version_builds[0] if last_build.success: - url = static(badge_path % 'passing') + url = settings.STATIC_URL + (badge_path % 'passing') else: - url = static(badge_path % 'failing') + url = settings.STATIC_URL + (badge_path % 'failing') return HttpResponseRedirect(url) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 0615bf4aed9..d49258aa1b8 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -3,8 +3,8 @@ from mock import patch from django.test import TestCase +from django.conf import settings from django.contrib.auth.models import User -from django.contrib.staticfiles.templatetags.staticfiles import static from django.contrib.messages import constants as message_const from django.core.urlresolvers import reverse from django.http.response import HttpResponseRedirect @@ -426,7 +426,7 @@ class TestBadges(TestCase): # To set `flat` as default style as done in code. def get_badge_path(self, version, style='flat'): - return static(self.BADGE_PATH % (version, style)) + return settings.STATIC_URL + (self.BADGE_PATH % (version, style)) def setUp(self): self.BADGE_PATH = 'projects/badges/%s-%s.svg' From e1f06bff5b30c3066356300ecc752be7763a171c Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 22 Aug 2018 15:53:52 -0700 Subject: [PATCH 2/5] Respond directly with badges rather than redirecting --- readthedocs/projects/views/public.py | 34 ++++++++++++------- .../rtd_tests/tests/test_project_views.py | 28 +++++++++------ 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index 093d9b24ded..1a9480a5107 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -19,6 +19,7 @@ from django.core.urlresolvers import reverse from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, render +from django.contrib.staticfiles.storage import staticfiles_storage from django.views.decorators.cache import never_cache from django.views.generic import DetailView, ListView from taggit.models import Tag @@ -116,23 +117,30 @@ def project_badge(request, project_slug): style = "flat" badge_path = 'projects/badges/%s-' + style + '.svg' version_slug = request.GET.get('version', LATEST) + version = None + url = badge_path % 'unknown' + try: version = Version.objects.public(request.user).get( project__slug=project_slug, slug=version_slug) except Version.DoesNotExist: - url = settings.STATIC_URL + (badge_path % 'unknown') - return HttpResponseRedirect(url) - version_builds = version.builds.filter(type='html', - state='finished').order_by('-date') - if not version_builds.exists(): - url = settings.STATIC_URL + (badge_path % 'unknown') - return HttpResponseRedirect(url) - last_build = version_builds[0] - if last_build.success: - url = settings.STATIC_URL + (badge_path % 'passing') - else: - url = settings.STATIC_URL + (badge_path % 'failing') - return HttpResponseRedirect(url) + pass + + if version: + version_builds = version.builds.filter(type='html', + state='finished').order_by('-date') + if version_builds.exists(): + last_build = version_builds[0] + if last_build.success: + url = badge_path % 'passing' + else: + url = badge_path % 'failing' + + with staticfiles_storage.open(url) as fd: + return HttpResponse( + fd.read(), + content_type='image/svg+xml', + ) def project_downloads(request, project_slug): diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index d49258aa1b8..2faa080d998 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -436,32 +436,38 @@ def setUp(self): def test_unknown_badge(self): res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('unknown') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'unknown') + + # Unknown project + unknown_project_url = reverse('project_badge', args=['fake-project']) + res = self.client.get(unknown_project_url, {'version': 'latest'}) + self.assertContains(res, 'unknown') def test_passing_badge(self): get(Build, project=self.project, version=self.version, success=True) res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('passing') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'passing') def test_failing_badge(self): get(Build, project=self.project, version=self.version, success=False) res = self.client.get(self.badge_url, {'version': self.version.slug}) - static_badge = self.get_badge_path('failing') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'failing') def test_plastic_failing_badge(self): get(Build, project=self.project, version=self.version, success=False) res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'plastic'}) - static_badge = self.get_badge_path('failing', 'plastic') - self.assertEquals(res.url, static_badge) + self.assertContains(res, 'failing') + + # The plastic badge has slightly more rounding + self.assertContains(res, 'rx="4"') def test_social_passing_badge(self): get(Build, project=self.project, version=self.version, success=True) - res = self.client.get(self.badge_url, {'version': self.version.slug , 'style': 'social'}) - static_badge = self.get_badge_path('passing', 'social') - self.assertEquals(res.url, static_badge) + res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'social'}) + self.assertContains(res, 'passing') + + # The social badge (but not the other badges) has this element + self.assertContains(res, 'rlink') class TestTags(TestCase): From cfad6703c33122342cba5a7a0f9ddf6e49aa00f1 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 22 Aug 2018 16:00:13 -0700 Subject: [PATCH 3/5] Remove unnecessary function --- readthedocs/rtd_tests/tests/test_project_views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 2faa080d998..6a30ea0e21f 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -3,7 +3,6 @@ from mock import patch from django.test import TestCase -from django.conf import settings from django.contrib.auth.models import User from django.contrib.messages import constants as message_const from django.core.urlresolvers import reverse @@ -424,10 +423,6 @@ def get_project_queryset(self): class TestBadges(TestCase): """Test a static badge asset is served for each build.""" - # To set `flat` as default style as done in code. - def get_badge_path(self, version, style='flat'): - return settings.STATIC_URL + (self.BADGE_PATH % (version, style)) - def setUp(self): self.BADGE_PATH = 'projects/badges/%s-%s.svg' self.project = get(Project, slug='badgey') From b9abaddd3a1d7f61a0b27e991820c36be2dc98bd Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 22 Aug 2018 16:54:34 -0700 Subject: [PATCH 4/5] Mock for opening files in storage --- readthedocs/rtd_tests/tests/test_project_views.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 6a30ea0e21f..0d9e80f9d6a 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -8,8 +8,8 @@ from django.core.urlresolvers import reverse from django.http.response import HttpResponseRedirect from django.views.generic.base import ContextMixin -from django_dynamic_fixture import get -from django_dynamic_fixture import new +from django.contrib.staticfiles import finders +from django_dynamic_fixture import get, new import six @@ -420,6 +420,15 @@ def get_project_queryset(self): self.assertEqual(view.get_context_data()['project'], self.project) +def staticfiles_test_open(fp): + local_path = finders.find(fp) + if local_path: + return open(local_path) + else: + raise RuntimeError(u'Could not open file {}'.format(fp)) + + +@patch('django.contrib.staticfiles.storage.staticfiles_storage.open', staticfiles_test_open) class TestBadges(TestCase): """Test a static badge asset is served for each build.""" From e7ab5114b065f9262ba88b6870767a63af65ae75 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 22 Aug 2018 17:20:45 -0700 Subject: [PATCH 5/5] More places required mocking --- readthedocs/rtd_tests/tests/test_privacy_urls.py | 6 +++++- readthedocs/rtd_tests/tests/test_project_views.py | 11 ++--------- readthedocs/rtd_tests/utils.py | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index e02be755ed9..03c78068293 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -18,6 +18,8 @@ from readthedocs.oauth.models import RemoteRepository, RemoteOrganization from readthedocs.rtd_tests.utils import create_user +from ..utils import staticfiles_test_open + class URLAccessMixin(object): @@ -167,6 +169,7 @@ def setUp(self): } +@mock.patch('django.contrib.staticfiles.storage.staticfiles_storage.open', staticfiles_test_open) class PublicProjectMixin(ProjectMixin): request_data = { @@ -177,7 +180,7 @@ class PublicProjectMixin(ProjectMixin): response_data = { # Public '/projects/pip/downloads/pdf/latest/': {'status_code': 302}, - '/projects/pip/badge/': {'status_code': 302}, + '/projects/pip/badge/': {'status_code': 200}, } def test_public_urls(self): @@ -185,6 +188,7 @@ def test_public_urls(self): self._test_url(urlpatterns) +@mock.patch('django.contrib.staticfiles.storage.staticfiles_storage.open', staticfiles_test_open) class PrivateProjectMixin(ProjectMixin): def test_private_urls(self): diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 0d9e80f9d6a..63275e53b5e 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -8,7 +8,6 @@ from django.core.urlresolvers import reverse from django.http.response import HttpResponseRedirect from django.views.generic.base import ContextMixin -from django.contrib.staticfiles import finders from django_dynamic_fixture import get, new import six @@ -23,6 +22,8 @@ from readthedocs.projects.views.mixins import ProjectRelationMixin from readthedocs.projects import tasks +from ..utils import staticfiles_test_open + @patch('readthedocs.projects.views.private.trigger_build', lambda x: None) class TestProfileMiddleware(RequestFactoryTestMixin, TestCase): @@ -420,14 +421,6 @@ def get_project_queryset(self): self.assertEqual(view.get_context_data()['project'], self.project) -def staticfiles_test_open(fp): - local_path = finders.find(fp) - if local_path: - return open(local_path) - else: - raise RuntimeError(u'Could not open file {}'.format(fp)) - - @patch('django.contrib.staticfiles.storage.staticfiles_storage.open', staticfiles_test_open) class TestBadges(TestCase): """Test a static badge asset is served for each build.""" diff --git a/readthedocs/rtd_tests/utils.py b/readthedocs/rtd_tests/utils.py index 3f01431c247..015b1702ae1 100644 --- a/readthedocs/rtd_tests/utils.py +++ b/readthedocs/rtd_tests/utils.py @@ -13,6 +13,7 @@ from tempfile import mkdtemp from django.contrib.auth.models import User +from django.contrib.staticfiles import finders from django_dynamic_fixture import new from readthedocs.doc_builder.base import restoring_chdir @@ -173,3 +174,16 @@ def create_user(username, password, **kwargs): user.set_password(password) user.save() return user + + +def staticfiles_test_open(fp): + """ + Open a static file which may not be collected + + Typically used with ``mock.patch`` + """ + local_path = finders.find(fp) + if local_path: + return open(local_path) + else: + raise RuntimeError(u'Could not open file {}'.format(fp))