Sort @-mention results by locale and project activity (mozilla#3997)

Sort get users results by locale and project

Adds locale and project parameters to the endpoint and use them
to annotate each user with their translation activity.
Results are then ordered by that activity.

The frontend is updated to pass the locale and project when calling
the endpoint

Fixes mozilla#2548

Author: MundiaNderi

pontoon/base/tests/views/test_get_users.py

@@ -0,0 +1,102 @@
+import json
+
+import pytest
+
+from django.contrib.auth.models import User
+
+from pontoon.base.views import get_users
+from pontoon.test.factories import (
+    EntityFactory,
+    LocaleFactory,
+    ProjectFactory,
+    ResourceFactory,
+    TranslationFactory,
+    UserFactory,
+)
+
+
+@pytest.mark.django_db
+def test_get_users_excludes_system_users(rf, admin):
+    request = rf.get("/get-users/", HTTP_X_REQUESTED_WITH="XMLHttpRequest")
+    request.user = admin
+    response = get_users(request)
+    data = json.loads(response.content)
+    usernames = [u["username"] for u in data]
+
+    system_users = User.objects.filter(profile__system_user=True)
+    for u in system_users:
+        assert u.username not in usernames
+
+
+@pytest.mark.django_db
+def test_get_users_sorted_by_locale_activity(rf, admin):
+    locale = LocaleFactory()
+    project = ProjectFactory(locales=[locale])
+    resource = ResourceFactory(project=project)
+    entity = EntityFactory(resource=resource)
+
+    other_locale = LocaleFactory()
+
+    # create a user that is from a different locale, not the current one
+    other_locale_user = UserFactory(first_name="Michael", last_name="OtherLocale")
+    TranslationFactory(user=other_locale_user, locale=other_locale, entity=entity)
+
+    # create a user active in the current locale
+    locale_user = UserFactory(first_name="Alice", last_name="ActiveLocale")
+    TranslationFactory(user=locale_user, locale=locale, entity=entity)
+
+    request = rf.get(
+        "/get-users/",
+        {"locale": locale.code, "project": project.pk},
+        HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+    )
+
+    request.user = admin
+    response = get_users(request)
+    data = json.loads(response.content)
+
+    names = [u["name"] for u in data]
+    assert names.index(locale_user.name_or_email) < names.index(
+        other_locale_user.name_or_email
+    )
+
+
+# project & locale activity should be prioritized over alphabetic order
+# for users that are both active in the project or locale
+
+
+@pytest.mark.django_db
+def test_get_users_sorted_by_project_and_locale_activity(rf, admin):
+    locale = LocaleFactory()
+    project = ProjectFactory(locales=[locale])
+    resource = ResourceFactory(project=project)
+    entity = EntityFactory(resource=resource)
+
+    # create a user that is active in the current locale and project
+    active_user = UserFactory(first_name="Zara", last_name="ActiveUser")
+    TranslationFactory(user=active_user, locale=locale, entity=entity)
+
+    # create a user that is active in the current locale but not the project
+    other_project_user = UserFactory(first_name="Bob", last_name="OtherProject")
+    TranslationFactory(user=other_project_user, locale=locale)
+
+    # create a user that is active in the current project but not the locale
+    other_locale_user = UserFactory(first_name="Charlie", last_name="OtherLocale")
+    TranslationFactory(user=other_locale_user, entity=entity)
+
+    request = rf.get(
+        "/get-users/",
+        {"locale": locale.code, "project": project.pk},
+        HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+    )
+
+    request.user = admin
+    response = get_users(request)
+    data = json.loads(response.content)
+
+    names = [u["name"] for u in data]
+    assert (
+        names.index(active_user.name_or_email)
+        < names.index(other_project_user.name_or_email)
+        < names.index(other_locale_user.name_or_email)
+    )

pontoon/base/views.py

@@ -14,7 +14,7 @@
 from django.contrib.auth.models import User
 from django.core.paginator import EmptyPage, Paginator
 from django.db import transaction
-from django.db.models import F, Prefetch, Q
+from django.db.models import Count, F, Prefetch, Q
 from django.http import (
     Http404,
     HttpResponse,
@@ -741,7 +741,12 @@ def unpin_comment(request):
 @utils.require_AJAX
 @login_required(redirect_field_name="", login_url="/403")
 def get_users(request):
-    """Get all users."""
+    """Get all users sorted by locale and project."""
+    locale_code = request.GET.get("locale")
+    project_id = request.GET.get("project")
+    if project_id:
+        project_id = int(project_id)
+
     users = (
         User.objects
         # Exclude system users
@@ -750,6 +755,16 @@ def get_users(request):
         .exclude(email__regex=r"^deleted-user-(\w+)@example.com$")
         # Prefetch profile for retrieving username
         .prefetch_related("profile")
+        .annotate(
+            in_locale=Count(
+                "translation", filter=Q(translation__locale__code=locale_code)
+            ),
+            in_project=Count(
+                "translation",
+                filter=Q(translation__entity__resource__project__id=project_id),
+            ),
+        )
+        .order_by("-in_locale", "-in_project", "first_name", "last_name")
     )
     payload = []
 

translate/src/api/user.ts

@@ -69,8 +69,17 @@ export const dismissAddonPromotion = (): Promise<void> =>
 /** Return data about the current user from the server. */
 export const fetchUserData = (): Promise<ApiUserData> => GET('/user-data/');
 
-/** Get all users from server. */
-export const fetchUsersList = (): Promise<MentionUser[]> => GET('/get-users/');
+/** Get all users from server, sorted by activity in the current locale and project. */
+export const fetchUsersList = (
+  locale: string,
+  projectId: number,
+): Promise<MentionUser[]> => {
+  const params = new URLSearchParams();
+  if (locale) params.append('locale', locale);
+  if (projectId) params.append('project', String(projectId));
+
+  return GET(`/get-users/?${params}`);
+};
 
 /** Mark all notifications of the current user as read. */
 export const markAllNotificationsAsRead = (): Promise<void> =>

translate/src/context/MentionUsers.test.jsx

@@ -0,0 +1,30 @@
+import React, { useContext, useEffect } from 'react';
+import { render } from '@testing-library/react';
+import { vi } from 'vitest';
+
+import { MentionUsers, MentionUsersProvider } from './MentionUsers';
+import { fetchUsersList } from '~/api/user';
+
+vi.mock('~/api/user', () => ({
+  fetchUsersList: vi.fn(() => Promise.resolve([])),
+}));
+
+describe('MentionUsersProvider', () => {
+  it('fetches users with locale and project from Location context', () => {
+    const Trigger = () => {
+      const { initMentions } = useContext(MentionUsers);
+      useEffect(() => {
+        initMentions('sl', 42);
+      }, []);
+      return null;
+    };
+
+    render(
+      <MentionUsersProvider>
+        <Trigger />
+      </MentionUsersProvider>,
+    );
+
+    expect(fetchUsersList).toHaveBeenCalledWith('sl', 42);
+  });
+});

translate/src/context/MentionUsers.tsx

@@ -7,7 +7,7 @@ type MentionUsers = {
    *
    * May be called multiple times, but data will only be fetched once.
    */
-  initMentions(): void;
+  initMentions(locale: string, projectId: number): void;
   mentionUsers: MentionUser[];
 };
 
@@ -21,18 +21,28 @@ export function MentionUsersProvider({
 }: {
   children: React.ReactElement;
 }) {
-  const [mentionUsers, setUsers] = useState<MentionUser[]>([]);
-  const [initMentions, setInit] = useState(() => () => {
-    setInit(() => () => {});
-    fetchUsersList().then((list) => {
-      if (Array.isArray(list)) {
-        setUsers(list);
+  const [state, setState] = useState<{
+    projectId: number;
+    mentionUsers: MentionUser[];
+  } | null>(null);
+
+  const initMentions = useMemo(
+    () => (locale: string, projectId: number) => {
+      if (state?.projectId === projectId) {
+        return;
       }
-    });
-  });
+      fetchUsersList(locale, projectId).then((list) => {
+        if (Array.isArray(list)) {
+          setState({ projectId, mentionUsers: list });
+        }
+      });
+    },
+    [],
+  );
+
   const value = useMemo(
-    () => ({ initMentions, mentionUsers }),
-    [initMentions, mentionUsers],
+    () => ({ initMentions, mentionUsers: state?.mentionUsers ?? [] }),
+    [state],
   );
   return (
     <MentionUsers.Provider value={value}>{children}</MentionUsers.Provider>

translate/src/modules/comments/components/AddComment.tsx

@@ -26,6 +26,8 @@ import {
   withReact,
 } from 'slate-react';
 
+import { Location } from '~/context/Location';
+import { EntityView } from '~/context/EntityView';
 import type { MentionUser } from '~/api/user';
 import { MentionUsers } from '~/context/MentionUsers';
 import type { UserState } from '~/modules/user';
@@ -116,7 +118,10 @@ export function AddComment({
     [editor],
   );
 
-  useEffect(initMentions, []);
+  const { entity } = useContext(EntityView);
+  const { locale } = useContext(Location);
+
+  useEffect(() => initMentions(locale, entity.project.pk), []);
 
   // Insert project manager as mention when 'Request context / Report issue' button used
   // and then clear the value from state