Story 2548: Webpage Integration -> Public Profile Routing

ak/homepage.py

@@ -109,7 +109,11 @@ def build_community_posts(limit=5):
         Entry.objects.ranked()
         .filter(deleted_at__isnull=True, published=True)
         .select_related("author", "author__displayed_profile_role_library")
-        .prefetch_related(active_badges_prefetch("author__badges"))[:limit]
+        # Badges per card, and the author's routing keys for the profile link.
+        .prefetch_related(
+            active_badges_prefetch("author__badges"),
+            "author__profile_routing_keys",
+        )[:limit]
     )
     return [entry.to_v3_post_card_dict() for entry in popular_entries]
 

ak/tests/test_homepage_posts.py

@@ -0,0 +1,32 @@
+import pytest
+from django.db import connection
+from django.test.utils import CaptureQueriesContext
+from model_bakery import baker
+
+from ak.homepage import build_community_posts
+
+pytestmark = pytest.mark.django_db
+
+
+def routing_key_queries(queries):
+    return [
+        query["sql"]
+        for query in queries.captured_queries
+        if "users_userprofileroutingkey" in query["sql"]
+    ]
+
+
+def test_community_posts_fetch_routing_keys_in_one_query(make_entry):
+    """Each card links its author's profile, which reads that author's routing
+    keys. Those are prefetched, so more posts must not mean more queries."""
+    for i in range(3):
+        make_entry(
+            author=baker.make("users.User", display_name=f"User {i}", image=None)
+        )
+
+    with CaptureQueriesContext(connection) as queries:
+        posts = build_community_posts(limit=3)
+
+    assert len(posts) == 3
+    assert all(post["author"]["profile_url"] for post in posts)
+    assert len(routing_key_queries(queries)) == 1

config/urls.py

@@ -175,11 +175,15 @@
             DeleteImmediatelyView.as_view(),
             name="profile-delete-immediately",
         ),
-        # Must stay after the "users/me/..." routes above: `int` cannot match
-        # "me" today, but that stops being true if this ever moves to a
-        # username or slug converter.
-        path("users/<int:pk>/", PublicUserProfileView.as_view(), name="profile-user"),
         path("users/avatar/", UserAvatar.as_view(), name="user-avatar"),
+        # Must stay last of the single-segment "users/..." routes: `slug`
+        # matches "me" and "avatar" too, so every literal segment has to be
+        # registered ahead of it.
+        path(
+            "users/<slug:routing_key>/",
+            PublicUserProfileView.as_view(),
+            name="profile-user",
+        ),
         path("api/v1/users/me/", CurrentUserAPIView.as_view(), name="current-user"),
         path(
             "api/v1/import-versions/",

core/tests/test_user_card_component.py

@@ -0,0 +1,41 @@
+from django.template.loader import render_to_string
+
+TEMPLATE = "v3/includes/_user_card.html"
+BASE_CONTEXT = {"username": "Jane Doe", "avatar_url": "/img/avatar.png"}
+
+
+def render_card(**overrides):
+    return render_to_string(TEMPLATE, {**BASE_CONTEXT, **overrides})
+
+
+def test_user_card_links_avatar_and_username_when_given_a_profile_url():
+    html = render_card(profile_url="/users/jane-doe-k3f9/")
+    assert '<a href="/users/jane-doe-k3f9/" class="user-card__avatar-link">' in html
+    assert (
+        '<a href="/users/jane-doe-k3f9/" class="user-card__username">Jane Doe</a>'
+        in html
+    )
+
+
+def test_user_card_renders_plain_without_a_profile_url():
+    """The profile page shows this card for the user whose page it is, where a
+    link back to the same page would be noise."""
+    html = render_card()
+    assert '<span class="user-card__username">Jane Doe</span>' in html
+    assert "user-card__avatar-link" not in html
+    assert "<a href" not in html
+
+
+def test_user_card_keeps_the_country_flag_out_of_the_link():
+    """The flag labels a country; it is not a second click target."""
+    html = render_card(profile_url="/users/jane-doe-k3f9/", flag_emoji="🇺🇸")
+    flag = '<span class="user-card__flag" aria-hidden="true">🇺🇸</span>'
+    assert flag in html
+    assert flag in html.split("</a>", 1)[1]
+
+
+def test_logged_out_user_card_ignores_a_profile_url():
+    """The logged-out variant has no user to link to."""
+    html = render_card(logged_out=True, profile_url="/users/jane-doe-k3f9/")
+    assert "user-card__avatar-link" not in html
+    assert "user-card__username" not in html

core/views.py

@@ -150,7 +150,11 @@ def build_recent_community_posts():
         Entry.objects.published()
         .filter(deleted_at__isnull=True)
         .select_related("author", "author__displayed_profile_role_library")
-        .prefetch_related(active_badges_prefetch("author__badges"))
+        .prefetch_related(
+            active_badges_prefetch("author__badges"),
+            # Each card links its author's profile, which reads their routing keys.
+            "author__profile_routing_keys",
+        )
         .order_by("-publish_at")[:4]
     )
     return [entry.to_v3_post_card_dict() for entry in entries]

libraries/mixins.py

@@ -276,10 +276,11 @@ def get_related(self, library_version, relation="maintainers", exclude_ids=None)
 
         Also patches the CommitAuthor onto the user, if a matching email exists.
         """
+        # Their rows link to the profile, which reads the user's routing keys.
         if relation == "maintainers":
-            qs = library_version.maintainers.all()
+            qs = library_version.maintainers.prefetch_related("profile_routing_keys")
         elif relation == "authors":
-            qs = library_version.authors.all()
+            qs = library_version.authors.prefetch_related("profile_routing_keys")
         else:
             raise ValueError("relation must be maintainers or authors.")
         if exclude_ids:
@@ -323,7 +324,14 @@ def get_top_contributors(self, library_version=None, exclude=None):
             )
         if exclude:
             qs = qs.exclude(id__in=exclude)
-        qs = qs.annotate(count=Count("commit")).order_by("-count")
+        qs = (
+            qs.annotate(count=Count("commit"))
+            # A claimed contributor links to their Boost profile, which reads
+            # the user and their routing keys.
+            .select_related("user")
+            .prefetch_related("user__profile_routing_keys")
+            .order_by("-count")
+        )
         return qs
 
     def get_previous_contributors(self, library_version, exclude=None):
@@ -336,6 +344,8 @@ def get_previous_contributors(self, library_version, exclude=None):
         qs = (
             CommitAuthor.humans.filter(commit__library_version__in=library_versions)
             .annotate(count=Count("commit"))
+            .select_related("user")
+            .prefetch_related("user__profile_routing_keys")
             .order_by("-count")
         )
         if exclude:
@@ -379,6 +389,10 @@ def build_all_contributors(self, library_version, authors, maintainers):
             CommitAuthor.humans.filter(commit__library_version__in=library_versions)
             .exclude(id__in=author_ca_ids + maintainer_ca_ids)
             .annotate(count=Count("commit"))
+            # A claimed contributor links to their Boost profile, which reads
+            # the user and their routing keys.
+            .select_related("user")
+            .prefetch_related("user__profile_routing_keys")
             .order_by("-count")
         )
         return (

libraries/models.py

@@ -132,10 +132,16 @@ def to_v3_profile_dict(self, role=None):
 
         Mirrors `User.to_v3_profile_dict` so the same template can render
         either a registered user or a git-only contributor.
+
+        A contributor who has claimed a Boost account links to that profile;
+        one who has not falls back to their GitHub page, which is all this site
+        knows about them. A deactivated account falls back the same way, since
+        its profile 404s.
         """
+        user_profile_url = self.user.profile_url if self.user else None
         return {
             "name": self.display_name,
-            "profile_url": self.github_profile_url,
+            "profile_url": user_profile_url or self.github_profile_url,
             "role": role,
             "avatar_url": self.avatar_url or "",
             "badge": None,
@@ -836,6 +842,7 @@ def author_details(self):
         return {
             "name": author.display_name if author else "Unknown",
             "role": "Author",
+            "profile_url": author.profile_url if author else None,
             "avatar_url": author.get_avatar_url() if author else "",
             "badge_url": large_static("img/v3/badges/badge-first-place.png"),
         }

libraries/tests/test_models.py

@@ -215,3 +215,66 @@ def test_merge_author_reassigns_emaildata():
     assert sum(authors[0].emaildata_set.all().values_list("count", flat=True)) == 200
     # total should stay the same
     assert EmailData.objects.all().aggregate(total=Sum("count"))["total"] == 1000
+
+
+def test_author_details_links_the_author_profile(library_version):
+    author = baker.make("users.User", display_name="Jane Doe", image=None)
+    library_version.authors.add(author)
+    assert library_version.author_details["profile_url"] == author.get_absolute_url()
+
+
+def test_author_details_does_not_link_a_deactivated_author(library_version):
+    """Their profile 404s, so the library card shows the name unlinked."""
+    author = baker.make(
+        "users.User", display_name="Jane Doe", image=None, is_active=False
+    )
+    library_version.authors.add(author)
+    assert library_version.author_details["profile_url"] is None
+
+
+def test_author_details_without_an_author(library_version):
+    assert library_version.author_details["profile_url"] is None
+
+
+def test_commit_author_links_a_claimed_boost_profile():
+    """A contributor who has claimed an account gets their profile, not GitHub."""
+    user = baker.make("users.User", display_name="Jane Doe", image=None)
+    author = baker.make(
+        CommitAuthor,
+        name="Jane Doe",
+        user=user,
+        github_profile_url="https://github.com/janedoe",
+    )
+    assert author.to_v3_profile_dict()["profile_url"] == user.get_absolute_url()
+
+
+def test_commit_author_falls_back_to_github_without_a_boost_profile():
+    """Most contributors are git-only; GitHub is all the site knows of them."""
+    author = baker.make(
+        CommitAuthor,
+        name="Jane Doe",
+        user=None,
+        github_profile_url="https://github.com/janedoe",
+    )
+    assert author.to_v3_profile_dict()["profile_url"] == "https://github.com/janedoe"
+
+
+def test_commit_author_falls_back_to_github_for_a_deactivated_account():
+    """The Boost profile 404s, but the GitHub page still resolves."""
+    user = baker.make(
+        "users.User", display_name="Jane Doe", image=None, is_active=False
+    )
+    author = baker.make(
+        CommitAuthor,
+        name="Jane Doe",
+        user=user,
+        github_profile_url="https://github.com/janedoe",
+    )
+    assert author.to_v3_profile_dict()["profile_url"] == "https://github.com/janedoe"
+
+
+def test_commit_author_with_neither_profile_is_unlinked():
+    author = baker.make(
+        CommitAuthor, name="Jane Doe", user=None, github_profile_url=None
+    )
+    assert author.to_v3_profile_dict()["profile_url"] is None