analytics: Use user IDs to get user activity summaries.

Using user IDs instead of emails is more reliable since users can
have arbitrarily complex emails that are hard to encode in a URL.
This has led to NoReverseMatch exceptions in the past.

analytics/tests/test_activity_views.py

@@ -46,9 +46,10 @@ class ActivityTest(ZulipTestCase):
 
         self.assert_length(queries, 8)
 
+        iago = self.example_user("iago")
         flush_per_request_caches()
         with queries_captured() as queries:
-            result = self.client_get("/user_activity/iago@zulip.com/")
+            result = self.client_get(f"/user_activity/{iago.id}/")
             self.assertEqual(result.status_code, 200)
 
-        self.assert_length(queries, 4)
+        self.assert_length(queries, 5)

analytics/urls.py

@@ -27,7 +27,7 @@ i18n_urlpatterns: List[Union[URLPattern, URLResolver]] = [
     path("activity", get_installation_activity),
     path("activity/support", support, name="support"),
     path("realm_activity/<realm_str>/", get_realm_activity),
-    path("user_activity/<email>/", get_user_activity),
+    path("user_activity/<user_profile_id>/", get_user_activity),
     path("stats/realm/<realm_str>/", stats_for_realm),
     path("stats/installation", stats_for_installation),
     path("stats/remote/<int:remote_server_id>/installation", stats_for_remote_installation),

analytics/views/activity_common.py

@@ -52,10 +52,10 @@ def format_date_for_activity_reports(date: Optional[datetime]) -> str:
         return ""
 
 
-def user_activity_link(email: str) -> mark_safe:
+def user_activity_link(email: str, user_profile_id: int) -> mark_safe:
     from analytics.views.user_activity import get_user_activity
 
-    url = reverse(get_user_activity, kwargs=dict(email=email))
+    url = reverse(get_user_activity, kwargs=dict(user_profile_id=user_profile_id))
     email_link = f'<a href="{escape(url)}">{escape(email)}</a>'
     return mark_safe(email_link)
 
@@ -84,13 +84,11 @@ def remote_installation_stats_link(server_id: int, hostname: str) -> mark_safe:
     return mark_safe(stats_link)
 
 
-def get_user_activity_summary(records: List[QuerySet]) -> Dict[str, Dict[str, Any]]:
+def get_user_activity_summary(records: List[QuerySet]) -> Dict[str, Any]:
-    #: `Any` used above should be `Union(int, datetime)`.
+    #: The type annotation used above is clearly overly permissive.
-    #: However current version of `Union` does not work inside other function.
+    #: We should perhaps use TypedDict to clearly lay out the schema
-    #: We could use something like:
+    #: for the user activity summary.
-    # `Union[Dict[str, Dict[str, int]], Dict[str, Dict[str, datetime]]]`
+    summary: Dict[str, Any] = {}
-    #: but that would require this long `Union` to carry on throughout inner functions.
-    summary: Dict[str, Dict[str, Any]] = {}
 
     def update(action: str, record: QuerySet) -> None:
         if action not in summary:
@@ -107,6 +105,7 @@ def get_user_activity_summary(records: List[QuerySet]) -> Dict[str, Dict[str, An
 
     if records:
         summary["name"] = records[0].user_profile.full_name
+        summary["user_profile_id"] = records[0].user_profile.id
 
     for record in records:
         client = record.client.name

analytics/views/realm_activity.py

@@ -41,7 +41,7 @@ def get_user_activity_records_for_realm(realm: str, is_bot: bool) -> QuerySet:
 
 def realm_user_summary_table(
     all_records: List[QuerySet], admin_emails: Set[str]
-) -> Tuple[Dict[str, Dict[str, Any]], str]:
+) -> Tuple[Dict[str, Any], str]:
     user_records = {}
 
     def by_email(record: QuerySet) -> str:
@@ -68,7 +68,7 @@ def realm_user_summary_table(
 
     rows = []
     for email, user_summary in user_records.items():
-        email_link = user_activity_link(email)
+        email_link = user_activity_link(email, user_summary["user_profile_id"])
         sent_count = get_count(user_summary, "send")
         cells = [user_summary["name"], email_link, sent_count]
         row_class = ""
@@ -107,10 +107,11 @@ def realm_user_summary_table(
     return user_records, content
 
 
-def realm_client_table(user_summaries: Dict[str, Dict[str, Dict[str, Any]]]) -> str:
+def realm_client_table(user_summaries: Dict[str, Dict[str, Any]]) -> str:
     exclude_keys = [
         "internal",
         "name",
+        "user_profile_id",
         "use",
         "send",
         "pointer",
@@ -120,7 +121,7 @@ def realm_client_table(user_summaries: Dict[str, Dict[str, Dict[str, Any]]]) ->
 
     rows = []
     for email, user_summary in user_summaries.items():
-        email_link = user_activity_link(email)
+        email_link = user_activity_link(email, user_summary["user_profile_id"])
         name = user_summary["name"]
         for k, v in user_summary.items():
             if k in exclude_keys:

analytics/views/user_activity.py

@@ -11,13 +11,13 @@ from analytics.views.activity_common import (
     make_table,
 )
 from zerver.decorator import require_server_admin
-from zerver.models import UserActivity
+from zerver.models import UserActivity, UserProfile, get_user_profile_by_id
 
 if settings.BILLING_ENABLED:
     pass
 
 
-def get_user_activity_records_for_email(email: str) -> List[QuerySet]:
+def get_user_activity_records(user_profile: UserProfile) -> List[QuerySet]:
     fields = [
         "user_profile__full_name",
         "query",
@@ -27,7 +27,7 @@ def get_user_activity_records_for_email(email: str) -> List[QuerySet]:
     ]
 
     records = UserActivity.objects.filter(
-        user_profile__delivery_email=email,
+        user_profile=user_profile,
     )
     records = records.order_by("-last_visit")
     records = records.select_related("user_profile", "client").only(*fields)
@@ -58,7 +58,7 @@ def raw_user_activity_table(records: List[QuerySet]) -> str:
 def user_activity_summary_table(user_summary: Dict[str, Dict[str, Any]]) -> str:
     rows = []
     for k, v in user_summary.items():
-        if k == "name":
+        if k == "name" or k == "user_profile_id":
             continue
         client = k
         count = v["count"]
@@ -83,8 +83,9 @@ def user_activity_summary_table(user_summary: Dict[str, Dict[str, Any]]) -> str:
 
 
 @require_server_admin
-def get_user_activity(request: HttpRequest, email: str) -> HttpResponse:
+def get_user_activity(request: HttpRequest, user_profile_id: int) -> HttpResponse:
-    records = get_user_activity_records_for_email(email)
+    user_profile = get_user_profile_by_id(user_profile_id)
+    records = get_user_activity_records(user_profile)
 
     data: List[Tuple[str, str]] = []
     user_summary = get_user_activity_summary(records)
@@ -95,7 +96,7 @@ def get_user_activity(request: HttpRequest, email: str) -> HttpResponse:
     content = raw_user_activity_table(records)
     data += [("Info", content)]
 
-    title = email
+    title = user_profile.delivery_email
     return render(
         request,
         "analytics/activity.html",