Contributors: Exclude self-reviews from the dashboard

pontoon/base/forms.py

@@ -360,7 +360,6 @@ class GetEntitiesForm(forms.Form):
     author = forms.CharField(required=False)
     review_time = forms.CharField(required=False)
     reviewer = forms.CharField(required=False)
-    exclude_self_reviewed = forms.BooleanField(required=False)
     search = forms.CharField(required=False)
     entity_ids = forms.CharField(required=False)
     pk_only = forms.BooleanField(required=False)

pontoon/base/get_entities.py

@@ -32,7 +32,6 @@ def get_entities_for_project_locale(
     author: str | None = None,
     review_time: str | None = None,
     reviewer: str | None = None,
-    exclude_self_reviewed: bool = False,
 ) -> QuerySet[Entity]:
     """Get project entities with locale translations."""
 
@@ -46,7 +45,6 @@ def get_entities_for_project_locale(
             review_time,
             author,
             reviewer,
-            exclude_self_reviewed,
         )
     )
     if pre_filter:
@@ -172,7 +170,6 @@ def _time_and_user_filters(
     review_time: str | None,
     author: str | None,
     reviewer: str | None,
-    exclude_self_reviewed: bool,
 ) -> Iterator[Q]:
     if time and match("^[0-9]{12}-[0-9]{12}$", time):
         range = _parse_time_interval(time)
@@ -207,7 +204,7 @@ def _time_and_user_filters(
                 | Q(translation__rejected_user__email__in=emails)
             )
 
-    if exclude_self_reviewed:
+    if reviewer or review_time:
         yield ~Q(
             Q(translation__approved_user=F("translation__user"))
             | Q(translation__rejected_user=F("translation__user"))

pontoon/base/tests/test_get_entities.py

@@ -0,0 +1,110 @@
+import pytest
+
+from django.utils import timezone
+
+from pontoon.base.get_entities import get_entities_for_project_locale
+from pontoon.base.models import TranslatedResource
+from pontoon.test.factories import (
+    EntityFactory,
+    ProjectLocaleFactory,
+    TranslationFactory,
+)
+
+
+def _time_interval(date):
+    stamp = date.strftime("%Y%m%d%H%M")
+    return f"{stamp}-{stamp}"
+
+
+@pytest.fixture
+def reviewed_entities(resource_a, locale_a, user_a, user_b):
+    """
+    Return two entities reviewed at the same time: one where user_a reviewed
+    their own translation, one where user_a reviewed user_b's translation.
+    """
+    ProjectLocaleFactory.create(project=resource_a.project, locale=locale_a)
+    TranslatedResource.objects.create(resource=resource_a, locale=locale_a)
+
+    now = timezone.now()
+    entities = {}
+
+    for key, author in (("self", user_a), ("peer", user_b)):
+        entity = EntityFactory.create(resource=resource_a, string=f"{key} string")
+        TranslationFactory.create(
+            entity=entity,
+            locale=locale_a,
+            user=author,
+            approved=True,
+            approved_user=user_a,
+            approved_date=now,
+            date=now,
+        )
+        entities[key] = entity
+
+    return entities, now
+
+
+@pytest.mark.django_db
+def test_reviewer_filter_excludes_self_reviews(
+    reviewed_entities, resource_a, locale_a, user_a
+):
+    """Approving your own translation is not a review performed."""
+    entities, now = reviewed_entities
+
+    matches = get_entities_for_project_locale(
+        user_a,
+        resource_a.project,
+        locale_a,
+        reviewer=user_a.email,
+        review_time=_time_interval(now),
+    )
+
+    assert list(matches) == [entities["peer"]]
+
+
+@pytest.mark.django_db
+def test_author_review_time_filter_excludes_self_reviews(
+    reviewed_entities, resource_a, locale_a, user_a, user_b
+):
+    """Approving your own translation is not a review received."""
+    entities, now = reviewed_entities
+
+    # user_a authored the self-reviewed translation, so they received no review
+    assert not list(
+        get_entities_for_project_locale(
+            user_a,
+            resource_a.project,
+            locale_a,
+            author=user_a.email,
+            review_time=_time_interval(now),
+        )
+    )
+
+    # user_b's translation was reviewed by user_a
+    assert list(
+        get_entities_for_project_locale(
+            user_a,
+            resource_a.project,
+            locale_a,
+            author=user_b.email,
+            review_time=_time_interval(now),
+        )
+    ) == [entities["peer"]]
+
+
+@pytest.mark.django_db
+def test_self_reviewed_strings_shown_without_review_filters(
+    reviewed_entities, resource_a, locale_a, user_a
+):
+    """The exclusion is scoped to the review filters, and doesn't leak elsewhere."""
+    entities, now = reviewed_entities
+
+    matches = get_entities_for_project_locale(
+        user_a,
+        resource_a.project,
+        locale_a,
+        author=user_a.email,
+        time=_time_interval(now),
+    )
+
+    assert list(matches) == [entities["self"]]

pontoon/base/views.py

@@ -378,7 +378,6 @@ def entities(request: HttpRequest):
         "author",
         "review_time",
         "reviewer",
-        "exclude_self_reviewed",
         "tag",
     )
     form_data = {

pontoon/contributors/templates/contributors/profile.html

@@ -413,7 +413,9 @@ <h3 class="title">{{ contribution_graph.title }}</h3>
                     >
                   </li>
                   <li class="clearfix">
-                    <span data-type="user_reviews" title="Reviews performed"
+                    <span
+                      data-type="user_reviews"
+                      title="Reviews performed on another translator's suggestions"
                       >Reviews performed</span
                     >
                   </li>

pontoon/contributors/tests/test_utils.py

@@ -65,7 +65,32 @@ def action_c(translation_a):
 
 
 @pytest.fixture
-def action_user_a(translation_a, user_a):
+def peer_translation(locale_a, project_locale_a, entity_a, user_b):
+    """Return a translation by another user so reviews of it are peer reviews."""
+    return TranslationFactory(
+        entity=entity_a,
+        locale=locale_a,
+        user=user_b,
+        string="Translation by user_b",
+        value=["Translation by user_b"],
+    )
+
+
+@pytest.fixture
+def action_user_a(peer_translation, user_a):
+    action = ActionLog.objects.create(
+        action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
+        performed_by=user_a,
+        translation=peer_translation,
+    )
+    action.created_at = timezone.now() - relativedelta(months=1)
+    action.save()
+    return action
+
+
+@pytest.fixture
+def self_approval_user_a(translation_a, user_a):
+    """Return user_a approving their own translation."""
     action = ActionLog.objects.create(
         action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
         performed_by=user_a,
@@ -89,12 +114,12 @@ def action_user_b(translation_a, user_b):
 
 
 @pytest.fixture
-def yesterdays_action_user_a(translation_a, user_a):
+def yesterdays_action_user_a(peer_translation, user_a):
     current_date = timezone.now()
     action = ActionLog.objects.create(
         action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
         performed_by=user_a,
-        translation=translation_a,
+        translation=peer_translation,
     )
     if current_date.day == 1:
         # First day of the month, so we instead set created_at to be earlier today
@@ -198,7 +223,9 @@ def test_get_approvals_charts_data_without_actions(user_a):
 
 
 @pytest.mark.django_db
-def test_get_approvals_charts_data_with_actions(user_a, action_user_a, action_user_b):
+def test_get_approvals_charts_data_with_actions(
+    user_a, self_approval_user_a, action_user_b
+):
     data = utils.get_approvals_charts_data(user_a)
 
     assert data["approval_rates"] == [0] * 11 + [100]
@@ -270,7 +297,9 @@ def test_get_contributions_map_without_actions(user_a, user_b):
 
 
 @pytest.mark.django_db
-def test_get_contributions_map_with_actions(user_a, action_user_a, user_b):
+def test_get_contributions_map_with_actions(
+    user_a, action_user_a, action_user_b, user_b
+):
     map = utils.get_contributions_map(user_a, user_b)
 
     for key, value in map.items():
@@ -280,6 +309,103 @@ def test_get_contributions_map_with_actions(user_a, action_user_a, user_b):
             assert value.exists()
 
 
+@pytest.mark.django_db
+def test_get_contributions_map_excludes_self_reviews(user_a, user_b, translation_a):
+    """Self-reviews count as neither performed nor received reviews."""
+    ActionLog.objects.create(
+        action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
+        performed_by=user_a,
+        translation=translation_a,
+    )
+
+    map = utils.get_contributions_map(user_a, user_b)
+
+    assert not map["user_reviews"].exists()
+    assert not map["peer_reviews"].exists()
+    assert not map["all_user_contributions"].exists()
+    assert not map["all_contributions"].exists()
+
+
+@pytest.mark.django_db
+def test_get_contributions_map_keeps_reviews_of_imported_translations(
+    user_a, user_b, locale_a, project_locale_a, entity_a
+):
+    """A translation without an author is nobody's own work, so reviewing it counts."""
+    imported = TranslationFactory(
+        entity=entity_a,
+        locale=locale_a,
+        user=None,
+        string="Imported translation",
+        value=["Imported translation"],
+    )
+    ActionLog.objects.create(
+        action_type=ActionLog.ActionType.TRANSLATION_REJECTED,
+        performed_by=user_a,
+        translation=imported,
+    )
+
+    map = utils.get_contributions_map(user_a, user_b)
+
+    assert map["user_reviews"].exists()
+    assert not map["peer_reviews"].exists()
+
+
+@pytest.mark.django_db
+def test_get_contributions_map_excludes_obsolete_entities(
+    user_a, user_b, locale_a, project_locale_a, resource_a
+):
+    """The translate view can't show obsolete strings, so they aren't counted."""
+    obsolete_entity = EntityFactory.create(
+        resource=resource_a, string="Obsolete string", obsolete=True
+    )
+    translation = TranslationFactory(
+        entity=obsolete_entity,
+        locale=locale_a,
+        user=user_b,
+        string="Translation of an obsolete string",
+        value=["Translation of an obsolete string"],
+    )
+    ActionLog.objects.create(
+        action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
+        performed_by=user_a,
+        translation=translation,
+    )
+
+    map = utils.get_contributions_map(user_a, user_b)
+
+    assert not map["user_reviews"].exists()
+    assert not map["all_contributions"].exists()
+
+
+@pytest.mark.django_db
+def test_get_contributions_map_excludes_disabled_projects(user_a, user_b, locale_a):
+    """The translate view can't show strings of disabled projects either."""
+    project = ProjectFactory.create(
+        slug="disabled_project", name="Disabled Project", disabled=True
+    )
+    resource = ResourceFactory.create(
+        project=project, path="resource_disabled.po", format="gettext"
+    )
+    entity = EntityFactory.create(resource=resource, string="Disabled string")
+    translation = TranslationFactory(
+        entity=entity,
+        locale=locale_a,
+        user=user_b,
+        string="Translation in a disabled project",
+        value=["Translation in a disabled project"],
+    )
+    ActionLog.objects.create(
+        action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
+        performed_by=user_a,
+        translation=translation,
+    )
+
+    map = utils.get_contributions_map(user_a, user_b)
+
+    assert not map["user_reviews"].exists()
+    assert not map["all_contributions"].exists()
+
+
 @pytest.mark.django_db
 def test_get_contribution_graph_data_without_actions(user_a, user_b):
     assert utils.get_contribution_graph_data(user_a, user_b) == (
@@ -301,12 +427,12 @@ def test_get_contribution_graph_data_with_actions(user_a, action_user_a, user_b)
 
 
 @pytest.mark.django_db
-def test_get_contribution_graph_data_for_year(user_a, user_b, translation_a):
+def test_get_contribution_graph_data_for_year(user_a, user_b, peer_translation):
     # Action in 2025
     action_2025 = ActionLog.objects.create(
         action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
         performed_by=user_a,
-        translation=translation_a,
+        translation=peer_translation,
     )
     action_2025.created_at = timezone.make_aware(datetime(2025, 6, 15))
     action_2025.save()
@@ -315,7 +441,7 @@ def test_get_contribution_graph_data_for_year(user_a, user_b, translation_a):
     action_2026 = ActionLog.objects.create(
         action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
         performed_by=user_a,
-        translation=translation_a,
+        translation=peer_translation,
     )
     action_2026.created_at = timezone.make_aware(datetime(2026, 1, 1))
     action_2026.save()
@@ -385,13 +511,13 @@ def test_get_contribution_timeline_data_with_actions(
 
 
 @pytest.mark.django_db
-def test_get_contribution_timeline_data_for_year(user_a, user_b, translation_a):
+def test_get_contribution_timeline_data_for_year(user_a, user_b, peer_translation):
     # Reviews in two different months of 2025
     for review_date in [datetime(2025, 6, 15), datetime(2025, 12, 10)]:
         action = ActionLog.objects.create(
             action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
             performed_by=user_a,
-            translation=translation_a,
+            translation=peer_translation,
         )
         action.created_at = timezone.make_aware(review_date)
         action.save()
@@ -400,7 +526,7 @@ def test_get_contribution_timeline_data_for_year(user_a, user_b, translation_a):
     action_2026 = ActionLog.objects.create(
         action_type=ActionLog.ActionType.TRANSLATION_APPROVED,
         performed_by=user_a,
-        translation=translation_a,
+        translation=peer_translation,
     )
     action_2026.created_at = timezone.make_aware(datetime(2026, 6, 15))
     action_2026.save()