diff --git a/core/templates/core/leaderboard.html b/core/templates/core/leaderboard.html index 1b86655..c6afd1c 100644 --- a/core/templates/core/leaderboard.html +++ b/core/templates/core/leaderboard.html @@ -54,11 +54,12 @@

Classement

{% if profiles %}
{% for p in profiles %} + {% with rank=forloop.counter0|add:page_obj.start_index %} - {% if forloop.counter <= 3 %} - + {% if rank <= 3 %} + {% endif %} - {{ forloop.counter }} + {{ rank }} {% if p.avatar_url %} {% else %} @@ -73,8 +74,44 @@

Classement

contrib.
+ {% endwith %} {% endfor %} + + {% if page_obj.paginator.num_pages > 1 %} + {% with qs=querystring %} + + {% endwith %} + {% endif %} + {% else %}
groups @@ -85,4 +122,4 @@

Personne ici {% endif %}

-{% endblock %} +{% endblock %} \ No newline at end of file diff --git a/core/tests.py b/core/tests.py index 2a300c8..5d92a5b 100644 --- a/core/tests.py +++ b/core/tests.py @@ -273,3 +273,63 @@ def test_explore_clone_size_filter_pagination(client, max_size, htmx): assert response.context["page_obj"].number == 2 assert len(response.context["page_obj"]) == 1 assert b"Excluded boundary" not in response.content + + +@pytest.mark.django_db +def test_leaderboard_pagination(client, monkeypatch): + cache.clear() + # Small page size so we don't need 50+ users. The view reads this module + # constant at request time, so patching it works. + monkeypatch.setattr("core.views.LEADERBOARD_PAGE_SIZE", 2) + + # lbuser{i} has i contributions. Odd i -> Ghana (1, 3, 5), even -> Nigeria (2, 4). + for i in range(1, 6): + user = User.objects.create_user(f"lbuser{i}") + profile = user.profile # created by signal + profile.github_username = f"lbuser{i}" + profile.country = "Ghana" if i % 2 else "Nigeria" + profile.save() + Contribution.objects.bulk_create([ + Contribution( + user=user, repo_full_name="a/b", title=f"pr{j}", + pr_url=f"https://example.com/{i}/{j}", + ) + for j in range(i) + ]) + + url = reverse("leaderboard") + + # Page 1: hero shown, highest contributor first, 2 rows. + r1 = client.get(url) + assert r1.status_code == 200 + assert r1.context["page_obj"].number == 1 + assert [p.github_username for p in r1.context["profiles"]] == ["lbuser5", "lbuser4"] + assert r1.context["top"].github_username == "lbuser5" + + # Page 2: no hero, ranks continue from 3. + r2 = client.get(url, {"page": 2}) + assert r2.status_code == 200 + assert r2.context["top"] is None + assert r2.context["page_obj"].start_index() == 3 + assert [p.github_username for p in r2.context["profiles"]] == ["lbuser3", "lbuser2"] + + # Bad or out-of-range page numbers must not crash. + assert client.get(url, {"page": "abc"}).status_code == 200 + assert client.get(url, {"page": 999}).status_code == 200 + + # Country filter: 3 Ghana profiles -> 2 pages, and every page link keeps ?country=Ghana. + r3 = client.get(url, {"country": "Ghana"}) + assert r3.status_code == 200 + assert r3.context["page_obj"].paginator.num_pages == 2 + links = re.findall(r'href="([^"]+)"', r3.content.decode()) + page_links = [link for link in links if "page=" in link] + assert page_links + for link in page_links: + query = parse_qs(urlsplit(unescape(link)).query) + assert query["country"] == ["Ghana"] + + next_page = next(link for link in page_links if "page=2" in link) + r4 = client.get(url + "?" + urlsplit(unescape(next_page)).query) + assert r4.status_code == 200 + assert r4.context["page_obj"].number == 2 + assert [p.github_username for p in r4.context["profiles"]] == ["lbuser1"] \ No newline at end of file diff --git a/core/views.py b/core/views.py index 4587165..5f78c6f 100644 --- a/core/views.py +++ b/core/views.py @@ -21,6 +21,9 @@ # Number of issues per page in the explore view. PAGE_SIZE = 30 +# Number of contributors per page on the leaderboard. +LEADERBOARD_PAGE_SIZE = 50 + # Fields needed by issue cards / explore list (avoid pulling full body blobs). ISSUE_LIST_FIELDS = ( "id", @@ -207,19 +210,23 @@ def profile_view(request, username): def leaderboard(request): country = request.GET.get("country", "").strip() - lb_key = cache_key("leaderboard", country or "all") - cached = cache_get(lb_key) - if cached is not None: - return HttpResponse(cached) - profiles = ( - Profile.objects.select_related("user") - .annotate(total=Count("user__contributions")) - .filter(total__gt=0) - .order_by("-total") - ) - if country: - profiles = profiles.filter(country__iexact=country) + # Cache only the computed rows (not rendered HTML), so the template is + # rendered per request. This also fixes #38 (anonymous navbar served to + # logged-in users) and means pagination doesn't need page in the key. + rows_key = cache_key("leaderboard", "rows", country.lower() or "all") + profiles = cache_get(rows_key) + if profiles is None: + qs = ( + Profile.objects.select_related("user") + .annotate(total=Count("user__contributions")) + .filter(total__gt=0) + .order_by("-total", "pk") # pk tiebreaker = stable ranking across pages + ) + if country: + qs = qs.filter(country__iexact=country) + profiles = list(qs) # no [:50] slice any more + cache_set(rows_key, profiles, CACHE_TTL) countries_key = cache_key("leaderboard", "countries") countries = cache_get(countries_key) @@ -233,23 +240,37 @@ def leaderboard(request): ) cache_set(countries_key, countries, CACHE_TTL) - profiles = list(profiles[:50]) - top = profiles[0] if profiles else None + paginator = Paginator(profiles, LEADERBOARD_PAGE_SIZE) + # get_page() handles ?page=abc and out-of-range values safely. + page_obj = paginator.get_page(request.GET.get("page", "1")) + page_rows = list(page_obj.object_list) - response = render( + on_first_page = page_obj.number == 1 + if on_first_page: + podium, rest = page_rows[:3], page_rows[3:] + else: + podium, rest = [], page_rows + + # Rank of the row just before `rest` begins: 3 on page 1, 50 on page 2, ... + rest_offset = max(page_obj.start_index() - 1, 0) + (len(podium)) + + querystring = urlencode({"country": country}) if country else "" + + return render( request, "core/leaderboard.html", { - "profiles": profiles, - "podium": profiles[:3], - "rest": profiles[3:], - "top": top, + "profiles": page_rows, + "podium": podium, + "rest": rest, + "rest_offset": rest_offset, + "top": profiles[0] if (profiles and on_first_page) else None, + "page_obj": page_obj, + "querystring": querystring, "countries": countries, "selected_country": country, }, ) - cache_set(lb_key, response.content, CACHE_TTL) - return response @require_http_methods(["GET", "HEAD"])