Address review: take viewing user id, use unpack_thread_id

- thread_to_pb / total_num_responses now take viewing_user_id instead of
  the full CouchersContext; callers pass context.user_id.
- approve_thread_post uses unpack_thread_id rather than duplicating the
  divmod-by-10 convention; moved the depth convention to pack_thread_id's
  docstring.

Author: aapeliv

app/backend/src/couchers/servicers/discussions.py

@@ -41,7 +41,7 @@ def discussion_to_pb(session: Session, discussion: Discussion, context: Couchers
         owner_title=discussion.owner_cluster.name,
         title=discussion.title,
         content=discussion.content,
-        thread=thread_to_pb(session, context, discussion.thread_id),
+        thread=thread_to_pb(session, context.user_id, discussion.thread_id),
         can_moderate=can_moderate,
     )
 

app/backend/src/couchers/servicers/events.py

@@ -204,7 +204,7 @@ def event_to_pb(session: Session, occurrence: EventOccurrence, context: Couchers
         owner_user_id=event.owner_user_id,
         owner_community_id=owner_community_id,
         owner_group_id=owner_group_id,
-        thread=thread_to_pb(session, context, event.thread_id),
+        thread=thread_to_pb(session, context.user_id, event.thread_id),
         can_edit=can_edit,
         can_moderate=can_moderate,
     )

app/backend/src/couchers/servicers/pages.py

@@ -78,7 +78,7 @@ def page_to_pb(session: Session, page: Page, context: CouchersContext) -> pages_
         owner_user_id=page.owner_user_id,
         owner_community_id=owner_community_id,
         owner_group_id=owner_group_id,
-        thread=thread_to_pb(session, context, page.thread_id),
+        thread=thread_to_pb(session, context.user_id, page.thread_id),
         title=current_version.title,
         content=current_version.content,
         photo_url=current_version.photo.full_url if current_version.photo_key else None,

app/backend/src/couchers/servicers/threads.py

@@ -6,8 +6,9 @@
 from sqlalchemy.orm import Session
 from sqlalchemy.sql import func
 
-from couchers.context import CouchersContext, make_background_user_context
+from couchers.context import CouchersContext, make_background_user_context, make_logged_out_context
 from couchers.db import session_scope
+from couchers.i18n import LocalizationContext
 from couchers.jobs.enqueue import queue_job
 from couchers.models import (
     Comment,
@@ -31,21 +32,30 @@
 
 logger = logging.getLogger(__name__)
 
-# Since the API exposes a single ID space regardless of nesting level,
-# we construct the API id by appending the nesting level to the
-# database ID.
-
 
 def pack_thread_id(database_id: int, depth: int) -> int:
+    """Pack (database_id, depth) into a single API thread id.
+
+    The API exposes a single ID space regardless of nesting level, so we append
+    the nesting level (depth) as the trailing digit of the database id: depth 0
+    = Thread, depth 1 = Comment, depth 2 = Reply.
+    """
     return database_id * 10 + depth
 
 
 def unpack_thread_id(thread_id: int) -> tuple[int, int]:
-    """Returns (database_id, depth) tuple."""
+    """Inverse of pack_thread_id; returns (database_id, depth)."""
     return divmod(thread_id, 10)
 
 
-def total_num_responses(session: Session, context: CouchersContext, database_id: int) -> int:
+def _viewing_context(viewing_user_id: int | None) -> CouchersContext:
+    if viewing_user_id is not None:
+        return make_background_user_context(user_id=viewing_user_id)
+    return make_logged_out_context(LocalizationContext.en_utc())
+
+
+def total_num_responses(session: Session, viewing_user_id: int | None, database_id: int) -> int:
+    context = _viewing_context(viewing_user_id)
     comments = where_moderated_content_visible(
         select(func.count()).select_from(Comment).where(Comment.thread_id == database_id),
         context,
@@ -64,10 +74,10 @@ def total_num_responses(session: Session, context: CouchersContext, database_id:
     return session.execute(comments).scalar_one() + session.execute(replies).scalar_one()
 
 
-def thread_to_pb(session: Session, context: CouchersContext, database_id: int) -> threads_pb2.Thread:
+def thread_to_pb(session: Session, viewing_user_id: int | None, database_id: int) -> threads_pb2.Thread:
     return threads_pb2.Thread(
         thread_id=pack_thread_id(database_id, 0),
-        num_responses=total_num_responses(session, context, database_id),
+        num_responses=total_num_responses(session, viewing_user_id, database_id),
     )
 
 

app/backend/src/tests/fixtures/misc.py

@@ -11,6 +11,7 @@
 from couchers.notifications.push import PushNotificationContent
 from couchers.proto import moderation_pb2
 from couchers.proto.internal import jobs_pb2
+from couchers.servicers.threads import unpack_thread_id
 from tests.fixtures.sessions import real_moderation_session
 
 
@@ -212,12 +213,8 @@ def approve_reply(self, reply_id: int, reason: str = "Test approval") -> None:
             )
 
     def approve_thread_post(self, packed_thread_id: int, reason: str = "Test approval") -> None:
-        """Approve whichever of Comment/Reply the packed thread_id refers to.
-
-        packed_thread_id uses the convention from couchers.servicers.threads.pack_thread_id:
-        the trailing digit is depth (1=Comment, 2=Reply); the rest is the database id.
-        """
-        database_id, depth = divmod(packed_thread_id, 10)
+        """Approve whichever of Comment/Reply the packed thread_id refers to."""
+        database_id, depth = unpack_thread_id(packed_thread_id)
         if depth == 1:
             self.approve_comment(database_id, reason=reason)
         elif depth == 2:

app/backend/src/tests/test_threads.py

@@ -338,18 +338,16 @@ def test_admin_can_hide_comment(db):
 
 
 def test_total_num_responses_excludes_shadowed(db):
-    from couchers.context import make_background_user_context  # noqa: PLC0415
     from couchers.servicers.threads import total_num_responses  # noqa: PLC0415
 
     author, author_token = generate_user()
     viewer, _ = generate_user()
     parent_thread_id, _ = _make_thread_and_comment(author_token, content="one")
-    viewer_context = make_background_user_context(user_id=viewer.id)
 
     parent_db_id, _ = divmod(parent_thread_id, 10)
 
     with session_scope() as session:
-        assert total_num_responses(session, viewer_context, parent_db_id) == 0
+        assert total_num_responses(session, viewer.id, parent_db_id) == 0
 
     with session_scope() as session:
         state = session.execute(
@@ -361,20 +359,18 @@ def test_total_num_responses_excludes_shadowed(db):
         state.visibility = ModerationVisibility.visible
 
     with session_scope() as session:
-        assert total_num_responses(session, viewer_context, parent_db_id) == 1
+        assert total_num_responses(session, viewer.id, parent_db_id) == 1
 
 
 def test_total_num_responses_includes_own_shadowed(db):
-    """The count uses the viewer's context so authors see their own shadowed content in the total,
+    """The count uses the viewer's id so authors see their own shadowed content in the total,
     matching what GetThread shows them in the list."""
-    from couchers.context import make_background_user_context  # noqa: PLC0415
     from couchers.servicers.threads import total_num_responses  # noqa: PLC0415
 
     author, author_token = generate_user()
     parent_thread_id, _ = _make_thread_and_comment(author_token, content="one")
-    author_context = make_background_user_context(user_id=author.id)
 
     parent_db_id, _ = divmod(parent_thread_id, 10)
 
     with session_scope() as session:
-        assert total_num_responses(session, author_context, parent_db_id) == 1
+        assert total_num_responses(session, author.id, parent_db_id) == 1