Skip to content

Commit

Permalink
Notifications: Fix broken push notifications for mobile apps (#8016)
Browse files Browse the repository at this point in the history
  • Loading branch information
TimOrtel committed Mar 2, 2024
1 parent d9949e3 commit 9bf300a
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@
* @param mentionedUsers users mentioned in the message
*/
public record CreatedConversationMessage(Post messageWithHiddenDetails, Conversation completeConversation, Set<User> mentionedUsers) {

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package de.tum.in.www1.artemis.domain.metis.conversation;

import java.time.ZonedDateTime;
import java.util.Set;

import javax.annotation.Nullable;
import javax.persistence.*;
import javax.validation.constraints.NotBlank;
Expand All @@ -9,10 +12,13 @@
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.in.www1.artemis.domain.Course;
import de.tum.in.www1.artemis.domain.Exercise;
import de.tum.in.www1.artemis.domain.Lecture;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.exam.Exam;
import de.tum.in.www1.artemis.domain.metis.ConversationParticipant;
import de.tum.in.www1.artemis.domain.metis.Post;

@Entity
@DiscriminatorValue("C")
Expand Down Expand Up @@ -90,6 +96,25 @@ public class Channel extends Conversation {
@JsonIgnoreProperties("channel")
private Exam exam;

public Channel(Long id, User creator, Set<ConversationParticipant> conversationParticipants, Set<Post> posts, Course course, ZonedDateTime creationDate,
ZonedDateTime lastMessageDate, String name, @Nullable String description, @Nullable String topic, Boolean isPublic, Boolean isAnnouncementChannel, Boolean isArchived,
boolean isCourseWide, Lecture lecture, Exercise exercise, Exam exam) {
super(id, creator, conversationParticipants, posts, course, creationDate, lastMessageDate);
this.name = name;
this.description = description;
this.topic = topic;
this.isPublic = isPublic;
this.isAnnouncementChannel = isAnnouncementChannel;
this.isArchived = isArchived;
this.isCourseWide = isCourseWide;
this.lecture = lecture;
this.exercise = exercise;
this.exam = exam;
}

public Channel() {
}

@Nullable
public String getName() {
return name;
Expand Down Expand Up @@ -190,4 +215,10 @@ public void hideDetails() {
setExercise(null);
super.hideDetails();
}

@Override
public Conversation copy() {
return new Channel(getId(), getCreator(), getConversationParticipants(), getPosts(), getCourse(), getCreationDate(), getLastMessageDate(), getName(), getDescription(),
getTopic(), getIsPublic(), getIsAnnouncementChannel(), getIsArchived(), getIsCourseWide(), getLecture(), getExercise(), getExam());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@ public abstract class Conversation extends DomainObject {
@Column(name = "last_message_date")
private ZonedDateTime lastMessageDate;

public Conversation(Long id, User creator, Set<ConversationParticipant> conversationParticipants, Set<Post> posts, Course course, ZonedDateTime creationDate,
ZonedDateTime lastMessageDate) {
this.setId(id);
this.creator = creator;
this.conversationParticipants = conversationParticipants;
this.posts = posts;
this.course = course;
this.creationDate = creationDate;
this.lastMessageDate = lastMessageDate;
}

public Conversation() {
}

public Set<ConversationParticipant> getConversationParticipants() {
return conversationParticipants;
}
Expand Down Expand Up @@ -107,6 +121,8 @@ public void setPosts(Set<Post> posts) {
this.posts = posts;
}

public abstract Conversation copy();

/**
* @param sender the sender of the message
* @return returns a human-readable name for this conversation, which can be used in notifications or emails.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static de.tum.in.www1.artemis.domain.metis.conversation.ConversationSettings.MAX_GROUP_CHAT_PARTICIPANTS;

import java.time.ZonedDateTime;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -15,8 +16,10 @@
import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.in.www1.artemis.config.Constants;
import de.tum.in.www1.artemis.domain.Course;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.metis.ConversationParticipant;
import de.tum.in.www1.artemis.domain.metis.Post;

@Entity
@DiscriminatorValue("G")
Expand All @@ -30,6 +33,15 @@ public class GroupChat extends Conversation {
@Size(min = 1, max = 20)
private String name;

public GroupChat(Long id, User creator, Set<ConversationParticipant> conversationParticipants, Set<Post> posts, Course course, ZonedDateTime creationDate,
ZonedDateTime lastMessageDate, String name) {
super(id, creator, conversationParticipants, posts, course, creationDate, lastMessageDate);
this.name = name;
}

public GroupChat() {
}

@Override
public void setConversationParticipants(Set<ConversationParticipant> conversationParticipant) {
if (conversationParticipant.size() > MAX_GROUP_CHAT_PARTICIPANTS) {
Expand Down Expand Up @@ -63,4 +75,9 @@ public String getHumanReadableNameForReceiver(User sender) {
return getName();
}
}

@Override
public Conversation copy() {
return new GroupChat(getId(), getCreator(), getConversationParticipants(), getPosts(), getCourse(), getCreationDate(), getLastMessageDate(), getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,32 @@

import static de.tum.in.www1.artemis.domain.metis.conversation.ConversationSettings.MAX_ONE_TO_ONE_CHAT_PARTICIPANTS;

import java.time.ZonedDateTime;
import java.util.Set;

import javax.persistence.DiscriminatorValue;
import javax.persistence.Entity;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.in.www1.artemis.domain.Course;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.metis.ConversationParticipant;
import de.tum.in.www1.artemis.domain.metis.Post;

@Entity
@DiscriminatorValue("O")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class OneToOneChat extends Conversation {

public OneToOneChat(Long id, User creator, Set<ConversationParticipant> conversationParticipants, Set<Post> posts, Course course, ZonedDateTime creationDate,
ZonedDateTime lastMessageDate) {
super(id, creator, conversationParticipants, posts, course, creationDate, lastMessageDate);
}

public OneToOneChat() {
}

@Override
public void setConversationParticipants(Set<ConversationParticipant> conversationParticipant) {
if (conversationParticipant.size() > MAX_ONE_TO_ONE_CHAT_PARTICIPANTS) {
Expand All @@ -29,4 +40,9 @@ public void setConversationParticipants(Set<ConversationParticipant> conversatio
public String getHumanReadableNameForReceiver(User sender) {
return sender.getName();
}

@Override
public Conversation copy() {
return new OneToOneChat(getId(), getCreator(), getConversationParticipants(), getPosts(), getCourse(), getCreationDate(), getLastMessageDate());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,11 @@ public static SingleUserNotification createNotification(AnswerPost answerPost, N
placeholders.add(answerPost.getContent());
placeholders.add(answerPost.getCreationDate().toString());
placeholders.add(answerPost.getAuthor().getName());
placeholders.add(conversation.getHumanReadableNameForReceiver(answerPost.getAuthor()));

String messageReplyTextType = MESSAGE_REPLY_IN_CONVERSATION_TEXT;

if (conversation instanceof Channel channel) {
placeholders.add(channel.getName());
if (conversation instanceof Channel) {
messageReplyTextType = MESSAGE_REPLY_IN_CHANNEL_TEXT;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package de.tum.in.www1.artemis.repository.metis.conversation;

import static de.tum.in.www1.artemis.config.Constants.PROFILE_CORE;
import static org.springframework.data.jpa.repository.EntityGraph.EntityGraphType.LOAD;

import java.util.List;
import java.util.Optional;

import javax.transaction.Transactional;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
Expand All @@ -33,6 +36,9 @@ public interface ConversationRepository extends JpaRepository<Conversation, Long
// This is used only for testing purposes
List<Conversation> findAllByCourseId(long courseId);

@EntityGraph(type = LOAD, attributePaths = { "conversationParticipants" })
Optional<Conversation> findWithParticipantsById(long conversationId);

default Conversation findByIdElseThrow(long conversationId) {
return this.findById(conversationId).orElseThrow(() -> new EntityNotFoundException("Conversation", conversationId));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,9 @@ public CreatedConversationMessage createMessage(Long courseId, Post newMessage)
newMessage.setDisplayPriority(DisplayPriority.NONE);

var conversation = conversationService.isMemberOrCreateForCourseWideElseThrow(newMessage.getConversation().getId(), author, Optional.empty())
.orElse(conversationRepository.findByIdElseThrow(newMessage.getConversation().getId()));
.orElse(conversationRepository.findWithParticipantsById(newMessage.getConversation().getId()).orElseThrow());
log.debug(" createMessage:conversationService.isMemberOrCreateForCourseWideElseThrow DONE");

// IMPORTANT we don't need it in the conversation any more, so we reduce the amount of data sent to clients
conversation.setConversationParticipants(Set.of());
var course = preCheckUserAndCourseForMessaging(author, courseId);

// extra checks for channels
Expand All @@ -132,8 +130,6 @@ public CreatedConversationMessage createMessage(Long courseId, Post newMessage)
log.debug(" conversationMessageRepository.save DONE");
// set the conversation again, because it might have been lost during save
createdMessage.setConversation(conversation);
// reduce the payload of the response / websocket message: this is important to avoid overloading the involved subsystems
createdMessage.getConversation().hideDetails();
log.debug(" conversationMessageRepository.save DONE");

createdMessage.setAuthor(author);
Expand All @@ -159,6 +155,7 @@ public void notifyAboutMessageCreation(CreatedConversationMessage createdConvers
ConversationNotification notification = conversationNotificationService.createNotification(createdMessage, conversation, course,
createdConversationMessage.mentionedUsers());
PostDTO postDTO = new PostDTO(createdMessage, MetisCrudAction.CREATE, notification);
createdMessage.getConversation().hideDetails();
if (createdConversationMessage.completeConversation() instanceof Channel channel && channel.getIsCourseWide()) {
// We don't need the list of participants for course-wide channels. We can delay the db query and send the WS messages first
if (conversationService.isChannelVisibleToStudents(channel)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public ConversationNotification createNotification(Post createdMessage, Conversa
String notificationText;
String[] placeholders;
NotificationType notificationType = NotificationType.CONVERSATION_NEW_MESSAGE;
String conversationName = createdMessage.getConversation().getHumanReadableNameForReceiver(createdMessage.getAuthor());
String conversationName = conversation.getHumanReadableNameForReceiver(createdMessage.getAuthor());

// add channel/groupChat/oneToOneChat string to placeholders for notification to distinguish in mobile client
if (conversation instanceof Channel channel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.security.Principal;
import java.util.Collections;
import java.util.List;

import javax.validation.Valid;
Expand Down Expand Up @@ -82,9 +83,13 @@ public ResponseEntity<Post> createMessage(@PathVariable Long courseId, @Valid @R
}
CreatedConversationMessage createdMessageData = conversationMessagingService.createMessage(courseId, post);
conversationMessagingService.notifyAboutMessageCreation(createdMessageData);

Post sendToUserPost = createdMessageData.messageWithHiddenDetails();
sendToUserPost.setConversation(sendToUserPost.getConversation().copy());
sendToUserPost.getConversation().setConversationParticipants(Collections.emptySet());

log.info("createMessage took {}", TimeLogUtil.formatDurationFrom(start));
return ResponseEntity.created(new URI("/api/courses/" + courseId + "/messages/" + createdMessageData.messageWithHiddenDetails().getId()))
.body(createdMessageData.messageWithHiddenDetails());
return ResponseEntity.created(new URI("/api/courses/" + courseId + "/messages/" + sendToUserPost.getId())).body(sendToUserPost);
}

/**
Expand Down

0 comments on commit 9bf300a

Please sign in to comment.