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

Communication: Fix push notifications for user mentions #10418

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
310 changes: 307 additions & 3 deletions docs/user/notifications.rst
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,313 @@ Overview

The following tables gives an overview of all supported notification types:

|supported-notification-types-overview-1|
|supported-notification-types-overview-2|
|supported-notification-types-overview-3|
.. list-table:: Notification Types
:widths: 20 10 10 10
:header-rows: 1

* - NotificationType
- Push
- Web
- Email

* - **Course-Wide Discussion Notifications**
-
-
-

* - NEW_COURSE_POST
- X
- X
-

* - NEW_REPLY_FOR_COURSE_POST
- X
- X
-

* - NEW_ANNOUNCEMENT_POST
- X
- X
- X

* - **Exercise Notifications**
-
-
-

* - EXERCISE_RELEASED
- X
- X
- X

* - EXERCISE_PRACTICE
- X
- X
- X

* - EXERCISE_SUBMISSION_ASSESSED
- X
- X
- X

* - FILE_SUBMISSION_SUCCESSFUL
- X
- X
- X

* - NEW_EXERCISE_POST
- X
- X
-

* - NEW_REPLY_FOR_EXERCISE_POST
- X
- X
-

* - **Lecture Notifications**
-
-
-

* - ATTACHMENT_CHANGE
- X
- X
- X

* - NEW_LECTURE_POST
- X
- X
-

* - NEW_REPLY_FOR_LECTURE_POST
- X
- X
-

* - **New message/replies Notifications**
-
-
-

* - CONVERSATION_NEW_MESSAGE
- X
- X
-

* - CONVERSATION_NEW_REPLY_MESSAGE
- X
- X
-

* - CONVERSATION_USER_MENTIONED
- X
- X
- X

* - CONVERSATION_CREATE_GROUP_CHAT
- X
- X
-

* - CONVERSATION_ADD_USER_CHANNEL
- X
- X
-

* - CONVERSATION_ADD_USER_GROUP_CHAT
-
- X
-

* - CONVERSATION_REMOVE_USER_GROUP_CHAT
-
- X
-

* - CONVERSATION_REMOVE_USER_CHANNEL
-
- X
-

* - CONVERSATION_DELETE_CHANNEL
-
- X
-

* - **Tutorial Group Notifications**
-
-
-

* - TUTORIAL_GROUP_REGISTRATION_STUDENT
- X
- X
- X

* - TUTORIAL_GROUP_DEREGISTRATION_STUDENT
- X
- X
- X

* - TUTORIAL_GROUP_DELETED
- X
- X
- X

* - TUTORIAL_GROUP_UPDATED
- X
- X
- X

* - **Tutor Notifications**
-
-
-

* - TUTORIAL_GROUP_REGISTRATION_TUTOR
- X
- X
- X

* - TUTORIAL_GROUP_MULTIPLE_REGISTRATION_TUTOR
- X
- X
- X

* - TUTORIAL_GROUP_DEREGISTRATION_TUTOR
- X
- X
- X

* - TUTORIAL_GROUP_ASSIGNED
- X
- X
- X

* - TUTORIAL_GROUP_UNASSIGNED
- X
- X
- X

* - **Editor Notifications**
-
-
-

* - PROGRAMMING_TEST_CASES_CHANGED
-
- X
-

* - **Instructor Notifications**
-
-
-

* - COURSE_ARCHIVE_STARTED
- X
-
-

* - COURSE_ARCHIVE_FINISHED_WITHOUT_ERRORS
-
- X
-

* - COURSE_ARCHIVE_FINISHED_WITH_ERRORS
-
- X
-

* - COURSE_ARCHIVE_FAILED
-
- X
-

* - EXAM_ARCHIVE_STARTED
-
- X
-

* - EXAM_ARCHIVE_FINISHED_WITHOUT_ERRORS
-
- X
-

* - EXAM_ARCHIVE_FINISHED_WITH_ERRORS
-
- X
-

* - EXAM_ARCHIVE_FAILED
-
- X
-

* - **Unassigned Notifications**
-
-
-

* - EXERCISE_UPDATED
-
- X
-

* - QUIZ_EXERCISE_STARTED
- X
- X
-

* - DUPLICATE_TEST_CASE
- X
- X
- X

* - ILLEGAL_SUBMISSION
-
- X
-

* - NEW_PLAGIARISM_CASE_STUDENT
- X
- X
- X

* - NEW_CPC_PLAGIARISM_CASE_STUDENT
- X
- X
- X

* - PLAGIARISM_CASE_VERDICT_STUDENT
- X
- X
- X

* - PLAGIARISM_CASE_REPLY
- X
- X
- X

* - NEW_MANUAL_FEEDBACK_REQUEST
-
- X
-

* - DATA_EXPORT_CREATED
- X
- X
- X

* - DATA_EXPORT_FAILED
- X
- X
- X


For the exact contents sent for each notification, please check out the usages of the `NotificationPlaceholderCreator` interface in the code.


Settings
^^^^^^^^
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,6 @@ private void sendAndSaveNotifications(ConversationNotification notification, Cre
.map(user -> new User(user.getId(), user.getLogin(), user.getFirstName(), user.getLastName(), user.getLangKey(), user.getEmail())).collect(Collectors.toSet());

Set<User> notificationRecipients = filterNotificationRecipients(author, conversation, recipientSummaries, mentionedUsers);
// Add all mentioned users, including the author (if mentioned). Since working with sets, there are no duplicate user entries
notificationRecipients.addAll(mentionedUsers);

conversationNotificationService.notifyAboutNewMessage(createdMessage, notification, notificationRecipients);
log.debug(" conversationNotificationService.notifyAboutNewMessage DONE");
Expand Down Expand Up @@ -245,7 +243,7 @@ private static Set<User> mapToUsers(Set<ConversationNotificationRecipientSummary
/**
* Filters the given list of recipients for users that should receive a notification about a new message.
* <p>
* In all cases, the author will be filtered out.
* The author will be filtered out, unless he is also in the list of mentioned users.
* If the conversation is not an announcement channel, the method filters out participants, that have muted or hidden the conversation.
* If the conversation is not visible to students, the method also filters out students from the provided list of recipients.
*
Expand All @@ -263,8 +261,7 @@ private Set<User> filterNotificationRecipients(User author, Conversation convers
if (conversation instanceof Channel channel) {
// If a channel is not an announcement channel, filter out users, that muted or hid the conversation
if (!channel.getIsAnnouncementChannel()) {
filter = filter.and(summary -> summary.shouldNotifyRecipient() || mentionedUsers
.contains(new User(summary.userId(), summary.userLogin(), summary.firstName(), summary.lastName(), summary.userLangKey(), summary.userEmail())));
filter = filter.and(ConversationNotificationRecipientSummary::shouldNotifyRecipient);
}

// If a channel is not visible to students, filter out participants that are only students
Expand All @@ -276,6 +273,10 @@ private Set<User> filterNotificationRecipients(User author, Conversation convers
filter = filter.and(ConversationNotificationRecipientSummary::shouldNotifyRecipient);
}

var mentionedUserIds = mentionedUsers.stream().map(User::getId).collect(Collectors.toSet());
// We explicitly also want to add the author to the recipients in case he tagged himself
filter = filter.or(summary -> mentionedUserIds.contains(summary.userId()));

return notificationRecipients.stream().filter(filter)
.map(summary -> new User(summary.userId(), summary.userLogin(), summary.firstName(), summary.lastName(), summary.userLangKey(), summary.userEmail()))
.collect(Collectors.toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ public class NotificationSettingsService {
PLAGIARISM_CASE_VERDICT_STUDENT, PLAGIARISM_CASE_REPLY, TUTORIAL_GROUP_REGISTRATION_STUDENT, TUTORIAL_GROUP_REGISTRATION_TUTOR,
TUTORIAL_GROUP_MULTIPLE_REGISTRATION_TUTOR, TUTORIAL_GROUP_DEREGISTRATION_STUDENT, TUTORIAL_GROUP_DEREGISTRATION_TUTOR, TUTORIAL_GROUP_DELETED, TUTORIAL_GROUP_UPDATED,
TUTORIAL_GROUP_ASSIGNED, TUTORIAL_GROUP_UNASSIGNED, NEW_EXERCISE_POST, NEW_LECTURE_POST, NEW_REPLY_FOR_LECTURE_POST, NEW_COURSE_POST, NEW_REPLY_FOR_COURSE_POST,
NEW_REPLY_FOR_EXERCISE_POST, QUIZ_EXERCISE_STARTED, DATA_EXPORT_CREATED, DATA_EXPORT_FAILED, CONVERSATION_NEW_MESSAGE, CONVERSATION_NEW_REPLY_MESSAGE);
NEW_REPLY_FOR_EXERCISE_POST, QUIZ_EXERCISE_STARTED, DATA_EXPORT_CREATED, DATA_EXPORT_FAILED, CONVERSATION_NEW_MESSAGE, CONVERSATION_NEW_REPLY_MESSAGE,
CONVERSATION_USER_MENTIONED);

// More information on supported notification types can be found here: https://docs.artemis.cit.tum.de/user/notifications/
// Please adapt the above docs if you change the supported notification types
Expand Down