Skip to content

Commit

Permalink
Fix sending new contributor notifications (#3477)
Browse files Browse the repository at this point in the history
We broke sending new contributor notifications in #2677 by moving the is_new_contributor() check after the point when translation is saved, while keeping its logic (return True whether the user has not made any contributions yet). That way the function always returns False.

This changeset:
* Fixes the logic to detect that the submitted translation is the user's first contribution to the locale.
* Add an additional check that the new contributor notification is not sent to project managers.
* Adds a test case to checks whether the new contributor notification is sent and that it's only sent once.
  • Loading branch information
mathjazz authored Dec 9, 2024
1 parent 3090235 commit f3603fd
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 6 deletions.
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"

# 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

0 comments on commit f3603fd

Please sign in to comment.