Skip to content

Commit

Permalink
Proxito: define dummy dashboard URLs for addons serializers (#11227)
Browse files Browse the repository at this point in the history
* Proxito: define dummy dashboard URLs for addons serializers

This PR defines dummy dashboard URLs in El Proxito to allow calling `reverse()`
for APIv3 serializers used under `/_/addons/` and return `*.urls` fields for
resources.

Closes readthedocs/readthedocs-ops#1323

* Proxito: define dummy dashboard URLs for addons serializers

This PR defines dummy dashboard URLs in El Proxito to allow calling `reverse()`
for APIv3 serializers used under `/_/addons/` and return `*.urls` fields for
resources.

Closes readthedocs/readthedocs-ops#1323

* Update tests

* "I'm a teapot" view for dummy URLs

* Update HTTP status code

* Reduce number of DB queries
  • Loading branch information
humitos committed Mar 21, 2024
1 parent 5315a7e commit 2d62224
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 17 deletions.
8 changes: 8 additions & 0 deletions readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ def get_context_data(self, **kwargs):
return context


class TeapotView(TemplateView):
template_name = "core/teapot.html"

def get(self, request, *args, **kwargs):
context = self.get_context_data(**kwargs)
return self.render_to_response(context, status=418)


def server_error_500(request, template_name="500.html"):
"""A simple 500 handler so we get media."""
r = render(request, template_name)
Expand Down
18 changes: 18 additions & 0 deletions readthedocs/proxito/tests/responses/v0.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@
"subproject_of": null,
"tags": ["project", "tag", "test"],
"translation_of": null,
"urls": {
"builds": "https://readthedocs.org/project/builds/",
"documentation": "https://project.dev.readthedocs.io/en/latest/",
"home": "https://readthedocs.org/project/",
"versions": "https://readthedocs.org/project/versions/"
},
"users": [
{
"username": "testuser"
Expand All @@ -49,6 +55,13 @@
"ref": null,
"slug": "latest",
"type": "tag",
"urls": {
"dashboard": {
"edit": "https://readthedocs.org/project/version/latest/edit/"
},
"documentation": "https://project.dev.readthedocs.io/en/latest/",
"vcs": "https://github.com/readthedocs/project/tree/master/"
},
"verbose_name": "latest"
}
},
Expand All @@ -66,6 +79,11 @@
"name": "Finished"
},
"success": true,
"urls": {
"build": "https://readthedocs.org/project/builds/1/",
"project": "https://readthedocs.org/project/",
"version": "https://readthedocs.org/project/version/latest/edit/"
},
"version": "latest"
}
},
Expand Down
8 changes: 4 additions & 4 deletions readthedocs/proxito/tests/test_hosting.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ def test_number_of_queries_project_version_slug(self):
active=True,
)

with self.assertNumQueries(20):
with self.assertNumQueries(21):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down Expand Up @@ -730,7 +730,7 @@ def test_number_of_queries_url(self):
active=True,
)

with self.assertNumQueries(20):
with self.assertNumQueries(22):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down Expand Up @@ -766,7 +766,7 @@ def test_number_of_queries_url_subproject(self):
active=True,
)

with self.assertNumQueries(24):
with self.assertNumQueries(28):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand All @@ -792,7 +792,7 @@ def test_number_of_queries_url_translations(self):
language=language,
)

with self.assertNumQueries(24):
with self.assertNumQueries(26):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down
49 changes: 47 additions & 2 deletions readthedocs/proxito/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from django.views import defaults

from readthedocs.constants import pattern_opts
from readthedocs.core.views import HealthCheckView
from readthedocs.core.views import HealthCheckView, TeapotView
from readthedocs.projects.views.public import ProjectDownloadMedia
from readthedocs.proxito.views.hosting import ReadTheDocsConfigJson
from readthedocs.proxito.views.serve import (
Expand Down Expand Up @@ -151,7 +151,52 @@
re_path(r"^(?P<path>.*)$", ServeDocs.as_view(), name="docs_detail"),
]

urlpatterns = health_check_urls + proxied_urls + core_urls + docs_urls

# Declare dummy "dashboard URLs" in El Proxito to be able to ``reverse()`` them
# from API ``/_/addons/`` endpoint. Mainly for the the ``*.urls`` fields. We
# cannot resolve ``*._links`` fields properly yet, but they are not required at
# this point. We can come back later here if we need them.
# See https://github.com/readthedocs/readthedocs-ops/issues/1323
dummy_dashboard_urls = [
# /dashboard/<project_slug>/
re_path(
r"^(?P<project_slug>{project_slug})/$".format(**pattern_opts),
TeapotView.as_view(),
name="projects_detail",
),
# /dashboard/<project_slug>/builds/
re_path(
(r"^(?P<project_slug>{project_slug})/builds/$".format(**pattern_opts)),
TeapotView.as_view(),
name="builds_project_list",
),
# /dashboard/<project_slug>/versions/
re_path(
r"^(?P<project_slug>{project_slug})/versions/$".format(**pattern_opts),
TeapotView.as_view(),
name="project_version_list",
),
# /dashboard/<project_slug>/builds/<build_id>/
re_path(
(
r"^(?P<project_slug>{project_slug})/builds/(?P<build_pk>\d+)/$".format(
**pattern_opts
)
),
TeapotView.as_view(),
name="builds_detail",
),
# /dashboard/<project_slug>/version/<version_slug>/
re_path(
r"^(?P<project_slug>[-\w]+)/version/(?P<version_slug>[^/]+)/edit/$",
TeapotView.as_view(),
name="project_version_detail",
),
]

urlpatterns = (
health_check_urls + proxied_urls + core_urls + docs_urls + dummy_dashboard_urls
)

# Use Django default error handlers to make things simpler
handler404 = proxito_404_page_handler
Expand Down
27 changes: 16 additions & 11 deletions readthedocs/proxito/views/hosting.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,20 @@ def _resolve_resources(self):

else:
project = Project.objects.filter(slug=project_slug).first()
version = Version.objects.filter(slug=version_slug, project=project).first()
version = (
Version.objects.filter(slug=version_slug, project=project)
.select_related("project")
.first()
)
if version:
build = version.builds.filter(
success=True,
state=BUILD_STATE_FINISHED,
).first()
build = (
version.builds.filter(
success=True,
state=BUILD_STATE_FINISHED,
)
.select_related("project", "version")
.first()
)

return project, version, build, filename

Expand Down Expand Up @@ -195,10 +203,7 @@ class NoLinksMixin:

"""Mixin to remove conflicting fields from serializers."""

FIELDS_TO_REMOVE = (
"_links",
"urls",
)
FIELDS_TO_REMOVE = ("_links",)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand All @@ -213,8 +218,8 @@ def __init__(self, *args, **kwargs):

# NOTE: the following serializers are required only to remove some fields we
# can't expose yet in this API endpoint because it's running under El Proxito
# which cannot resolve some dashboard URLs because they are not defined on El
# Proxito.
# which cannot resolve URLs pointing to the APIv3 because they are not defined
# on El Proxito.
#
# See https://github.com/readthedocs/readthedocs-ops/issues/1323
class ProjectSerializerNoLinks(NoLinksMixin, ProjectSerializer):
Expand Down
11 changes: 11 additions & 0 deletions readthedocs/templates/core/teapot.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends "errors/base.html" %}
{% load core_tags %}
{% load i18n %}

{% block title %}
{% trans "418. I'm a teapot" %}
{% endblock %}

{% block content %}
<h1>{% trans "418. I'm a teapot" %}</h1>
{% endblock %}

0 comments on commit 2d62224

Please sign in to comment.