Skip to content

Commit

Permalink
Send notifications only for answers in your thread (#2407)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulinenik authored Sep 11, 2024
1 parent 66a479f commit 5918de5
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 14 deletions.
6 changes: 3 additions & 3 deletions src/apps/homework/models/answer.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ def get_absolute_url(self) -> str:
return url

def get_root_answer(self) -> "Answer":
ancesorts = self.ancestors()
if ancesorts.count():
return ancesorts[0]
ancestors = self.ancestors()
if ancestors.count():
return ancestors[0]

return self

Expand Down
4 changes: 2 additions & 2 deletions src/apps/homework/services/new_answer_notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ def get_notification_classes() -> list[Type[BaseAnswerNotification]]:
]

def get_users_to_notify(self) -> QuerySet[User]:
"""Get all users that have ever written an answer to the root of the disqussion"""
authors = self.answer.get_root_answer().descendants(include_self=True).values_list("author", flat=True)
"""Get authors of ancestor answers, excluding current answer author"""
authors = self.answer.ancestors().values_list("author", flat=True)

authors = list(authors) # have to execute this query cuz django-tree-queries fails to compile it

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,24 @@ def mocked_crosschecked_answer_notification_send_if_should(mocker):
)


@pytest.mark.usefixtures("parent_of_another_answer")
def test_sends_crosschecked_answer_notification(
notifier, parent_of_another_answer, mocked_crosschecked_answer_notification_send_if_should, mocked_default_answer_notification_send_if_should
notifier, another_answer, mocked_crosschecked_answer_notification_send_if_should, mocked_default_answer_notification_send_if_should
):
mocked_crosschecked_answer_notification_send_if_should.return_value = True

notifier(parent_of_another_answer)()
notifier(another_answer)()

mocked_crosschecked_answer_notification_send_if_should.assert_called_once()
assert not mocked_default_answer_notification_send_if_should.called


@pytest.mark.usefixtures("parent_of_another_answer")
def test_calls_default_notification_if_crosschecked_not_sent(
notifier, parent_of_another_answer, mocked_crosschecked_answer_notification_send_if_should, mocked_default_answer_notification_send_if_should
notifier, another_answer, mocked_crosschecked_answer_notification_send_if_should, mocked_default_answer_notification_send_if_should
):
mocked_crosschecked_answer_notification_send_if_should.return_value = False

notifier(parent_of_another_answer)()
notifier(another_answer)()

mocked_default_answer_notification_send_if_should.assert_called_once()
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ def test_parent_answer_author_is_in_the_list(notifier, answer, another_answer):
assert another_answer.author in get_users_to_notify(notifier)


def test_sibling_answer_author_is_in_the_list(notifier, answer, another_answer, mixer):
def test_sibling_answer_author_is_not_in_the_list(notifier, answer, another_answer, mixer):
parent = mixer.blend("homework.Answer")
answer.update(parent=parent)
another_answer.update(parent=parent)

notifier = notifier(answer)

assert another_answer.author in get_users_to_notify(notifier)
assert another_answer.author not in get_users_to_notify(notifier)


def test_parent_of_parent_answer_author_is_in_the_list(notifier, answer, another_answer, parent_of_another_answer):
Expand All @@ -45,7 +45,7 @@ def test_parent_of_parent_answer_author_is_in_the_list(notifier, answer, another
assert parent_of_another_answer.author in get_users_to_notify(notifier)


def test_author_is_excluded_event_if_he_is_in_answer_tree(notifier, answer, another_answer):
def test_author_is_excluded_even_if_he_is_in_answer_tree(notifier, answer, another_answer):
answer.update(parent=another_answer)
another_answer.update(author=answer.author)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_not_notifying_commenter(reply, answer, commenter, get_notified_users):
assert commenter.email not in get_notified_users()


def test_notifying_another_commenter(reply, answer, another_user, commenter, get_notified_users):
def test_not_notifying_another_commenter(reply, answer, another_user, commenter, get_notified_users):
reply(
answer,
{
Expand All @@ -79,7 +79,7 @@ def test_notifying_another_commenter(reply, answer, another_user, commenter, get
as_user=another_user,
)

assert set(get_notified_users()) == {answer.author.email, answer.author.email, commenter.email}
assert set(get_notified_users()) == {answer.author.email}


def test_disabling_feature_disables_sending(reply, answer, settings, get_notified_users):
Expand Down

0 comments on commit 5918de5

Please sign in to comment.