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

Cache contributors views #3091

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
23 changes: 14 additions & 9 deletions pontoon/contributors/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,21 +235,26 @@ def test_default_period(


@pytest.mark.django_db
def test_invalid_period(
member, mock_contributors_render, mock_users_translations_counts
):
def test_invalid_period(member, mock_users_translations_counts):
"""
Checks how view handles invalid period, it result in period being None - displays all data.
"""
# If period parameter is invalid value
member.client.get("/contributors/?period=invalidperiod")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None
with patch.object(
views.ContributorsView, "render_to_response", return_value=HttpResponse("")
) as mock_contributors_render:
member.client.get("/contributors/?period=invalidperiod")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None

# Period shouldn't be negative integer
member.client.get("/contributors/?period=-6")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None
# The ContributorsView URL is cached, so we need a new mock
with patch.object(
views.ContributorsView, "render_to_response", return_value=HttpResponse("")
) as mock_contributors_render:
member.client.get("/contributors/?period=-6")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None


@pytest.mark.django_db
Expand Down
5 changes: 4 additions & 1 deletion pontoon/contributors/urls.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from django.urls import path, register_converter
from django.urls.converters import StringConverter
from django.views.decorators.cache import cache_page
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Django docs recommend to specify the per-view cache in the URLconf rather than next to the view functions themselves.

from django.views.generic import RedirectView

from pontoon.settings import PER_VIEW_CACHE_TIMEOUT

from . import views


Expand All @@ -25,7 +28,7 @@ class UsernameConverter(StringConverter):
# List contributors
path(
"contributors/",
views.ContributorsView.as_view(),
cache_page(PER_VIEW_CACHE_TIMEOUT)(views.ContributorsView.as_view()),
name="pontoon.contributors",
),
# Contributor profile by username
Expand Down
6 changes: 5 additions & 1 deletion pontoon/localizations/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.urls import include, path, re_path
from django.views.decorators.cache import cache_page

from pontoon.projects import views as projects_views
from pontoon.settings import PER_VIEW_CACHE_TIMEOUT
from pontoon.teams import views as teams_views

from . import views
Expand Down Expand Up @@ -66,7 +68,9 @@
# Localization contributors
path(
"contributors/",
views.LocalizationContributorsView.as_view(),
cache_page(PER_VIEW_CACHE_TIMEOUT)(
views.LocalizationContributorsView.as_view()
),
name="pontoon.localizations.ajax.contributors",
),
# Project insights
Expand Down
5 changes: 5 additions & 0 deletions pontoon/projects/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ def test_project_top_contributors(client, project_a, project_b):
project_a_contributor
]

# The ProjectContributorsView URL is cached, so we need a new mock
with patch(
"pontoon.projects.views.ProjectContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/projects/{project_b.slug}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
Expand Down
7 changes: 6 additions & 1 deletion pontoon/projects/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from django.urls import include, path
from django.views.decorators.cache import cache_page
from django.views.generic import RedirectView

from pontoon.settings import PER_VIEW_CACHE_TIMEOUT

from . import views

urlpatterns = [
Expand Down Expand Up @@ -70,7 +73,9 @@
# Project contributors
path(
"contributors/",
views.ProjectContributorsView.as_view(),
cache_page(PER_VIEW_CACHE_TIMEOUT)(
views.ProjectContributorsView.as_view()
),
name="pontoon.projects.ajax.contributors",
),
# Project insights
Expand Down
3 changes: 3 additions & 0 deletions pontoon/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,9 @@ def _default_from_email():
}
}

# Default timeout for the per-view cache, in seconds.
PER_VIEW_CACHE_TIMEOUT = 60 * 60 * 24 # 1 day

# Site ID is used by Django's Sites framework.
SITE_ID = 1

Expand Down
45 changes: 23 additions & 22 deletions pontoon/teams/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,28 +170,29 @@ def test_users_permissions_for_ajax_permissions_view(


@pytest.mark.django_db
@patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
)
def test_locale_top_contributors(mock_render, client, translation_a, locale_b):
def test_locale_top_contributors(client, translation_a, locale_b):
"""
Tests if the view returns top contributors specific for given locale.
"""
client.get(
f"/{translation_a.locale.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)

response_context = mock_render.call_args[0][0]
assert response_context["locale"] == translation_a.locale
assert list(response_context["contributors"]) == [translation_a.user]

client.get(
f"/{locale_b.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)

response_context = mock_render.call_args[0][0]
assert response_context["locale"] == locale_b
assert list(response_context["contributors"]) == []
with patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/{translation_a.locale.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
assert mock_render.call_args[0][0]["locale"] == translation_a.locale
assert list(mock_render.call_args[0][0]["contributors"]) == [translation_a.user]

# The LocaleContributorsView URL is cached, so we need a new mock
with patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/{locale_b.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
assert mock_render.call_args[0][0]["locale"] == locale_b
assert list(mock_render.call_args[0][0]["contributors"]) == []
7 changes: 6 additions & 1 deletion pontoon/teams/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from django.urls import include, path
from django.views.decorators.cache import cache_page
from django.views.generic import RedirectView

from pontoon.settings import PER_VIEW_CACHE_TIMEOUT

from . import views


Expand Down Expand Up @@ -70,7 +73,9 @@
# Team contributors
path(
"contributors/",
Copy link
Collaborator Author

@mathjazz mathjazz Feb 10, 2024

Choose a reason for hiding this comment

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

As per the docs, if multiple URLs point at the same view, each URL will be cached separately.

In this example, /de/contributors/ and /it/contributors/ will be cached separately, as you may expect. But once a particular URL has been requested, subsequent requests to that URL will use the cache.

views.LocaleContributorsView.as_view(),
cache_page(PER_VIEW_CACHE_TIMEOUT)(
views.LocaleContributorsView.as_view()
),
name="pontoon.teams.ajax.contributors",
),
# Team insights
Expand Down
Loading