Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix new contributor detection logic #3477

Merged
merged 6 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions pontoon/base/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,13 @@ def can_translate(self, locale, project):
return self.has_perm("base.can_translate_locale", locale)


def is_new_contributor(self, locale):
"""Return True if the user hasn't made contributions to the locale yet."""
def has_one_contribution(self, locale):
"""Return True if the user has made just 1 contribution to the locale."""
return (
not self.translation_set.filter(locale=locale)
self.translation_set.filter(locale=locale)
.exclude(entity__resource__project__system_project=True)
.exists()
.count()
== 1
)


Expand Down Expand Up @@ -499,7 +500,7 @@ def latest_action(self):
User.add_to_class("has_approved_translations", has_approved_translations)
User.add_to_class("top_contributed_locale", top_contributed_locale)
User.add_to_class("can_translate", can_translate)
User.add_to_class("is_new_contributor", is_new_contributor)
User.add_to_class("has_one_contribution", has_one_contribution)
User.add_to_class("notification_list", notification_list)
User.add_to_class("menu_notifications", menu_notifications)
User.add_to_class("unread_notifications_display", unread_notifications_display)
Expand Down
72 changes: 72 additions & 0 deletions pontoon/translations/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest.mock import patch

import pytest

from django.urls import reverse
Expand Down Expand Up @@ -210,6 +212,76 @@ def test_run_checks_during_translation_update(
assert warning.message == "trailing argument 1 `s` missing"


@patch("notifications.signals.notify.send")
@pytest.mark.django_db
def test_notify_managers_on_first_contribution(
mock_notify,
member,
entity_a,
entity_b,
locale_a,
user_a,
user_b,
project_locale_a,
request_create_translation,
):
"""
Test that managers are notified when a user makes their first contribution
to a locale.
"""
translation = TranslationFactory(
entity=entity_a,
locale=locale_a,
user=user_b, # user_b has no contributions to locale_a yet
approved=True,
active=True,
)

# Set user_a as a manager for locale_a and subscribe it to new contributor notifications
translation.locale.managers_group.user_set.add(user_a)
user_a.profile.new_contributor_notifications = True
user_a.profile.save()

mock_notify.reset_mock()
response = request_create_translation(
member.client,
entity=translation.entity.pk,
original=translation.entity.string,
locale=translation.locale.code,
translation="First translation",
)

assert response.status_code == 200
assert response.json()["status"]

mock_notify.assert_called_once()

# Check that the new contributor notification was sent to the correct manager
notify_args = mock_notify.call_args[1]
assert notify_args["recipient"] == user_a
assert notify_args["category"] == "new_contributor"
eemeli marked this conversation as resolved.
Show resolved Hide resolved

# Check that the new contributor notification is only sent for the first contribution
second_translation = TranslationFactory(
entity=entity_b,
locale=locale_a,
user=user_b,
approved=True,
active=True,
)

mock_notify.reset_mock()
response = request_create_translation(
member.client,
entity=second_translation.entity.pk,
original=second_translation.entity.string,
locale=second_translation.locale.code,
translation="Second translation",
)

mock_notify.assert_not_called()


@pytest.fixture
def approved_translation(locale_a, project_locale_a, entity_a, user_a):
return TranslationFactory(
Expand Down
6 changes: 5 additions & 1 deletion pontoon/translations/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ def create_translation(request):
)

# When user makes their first contribution to the team, notify team managers
first_contribution = not project.system_project and user.is_new_contributor(locale)
first_contribution = (
not project.system_project
and user != project.contact
and user.has_one_contribution(locale)
)
if first_contribution:
description = render_to_string(
"messaging/notifications/new_contributor.html",
Expand Down
Loading