From efd8ef50d6b0a5301971c41eafe8b5ff17977970 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Fri, 6 Dec 2024 18:47:07 +0100 Subject: [PATCH 01/12] Preliminary implementation --- .../service/conversation/ChannelService.java | 7 +++++++ .../service/conversation/ConversationService.java | 7 +++++++ .../web/conversation/ChannelResource.java | 12 ++++++++++++ .../course-conversations.component.html | 1 + .../course-conversations.component.ts | 10 ++++++++++ .../metis/conversations/conversation.service.ts | 4 ++++ .../app/shared/metis/metis-conversation.service.ts | 12 ++++++++++++ .../webapp/app/shared/sidebar/sidebar.component.html | 4 ++++ .../webapp/app/shared/sidebar/sidebar.component.ts | 8 +++++++- src/main/webapp/i18n/de/student-dashboard.json | 3 ++- src/main/webapp/i18n/en/student-dashboard.json | 3 ++- 11 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java index 791847b75670..90260698ecc4 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java @@ -450,4 +450,11 @@ public Channel createFeedbackChannel(Course course, Long exerciseId, ChannelDTO return createdChannel; } + + public void markAllChannelsOfCourseAsRead(Course course, User requestingUser) { + var channels = channelRepository.findChannelsByCourseId(course.getId()); + for (Channel channel : channels) { + conversationService.markConversationAsRead(channel, requestingUser); + } + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index c2a1761b6b46..23842e6c7e23 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -442,6 +442,13 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted conversationParticipantRepository.save(conversationParticipant); } + public void markConversationAsRead(Channel channel, User requestingUser) { + var conversationParticipant = getOrCreateConversationParticipant(channel.getId(), requestingUser); + conversationParticipant.setLastRead(ZonedDateTime.now()); + conversationParticipant.setUnreadMessagesCount(0L); + conversationParticipantRepository.save(conversationParticipant); + } + /** * The user can select one of these roles to filter the conversation members by role */ diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java index cbb59c4b7e46..642dd02a5822 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java @@ -496,6 +496,18 @@ public ResponseEntity createFeedbackChannel(@PathVariable Long cours return ResponseEntity.created(new URI("/api/channels/" + createdChannel.getId())).body(conversationDTOService.convertChannelToDTO(requestingUser, createdChannel)); } + @PutMapping("{courseId}/channels/mark-as-read") + @EnforceAtLeastStudent + public ResponseEntity markAllChannelsOfCourseAsRead(@PathVariable Long courseId) { + log.debug("REST request to mark all channels of course {} as read", courseId); + var requestingUser = userRepository.getUserWithGroupsAndAuthorities(); + var course = courseRepository.findByIdElseThrow(courseId); + checkCommunicationEnabledElseThrow(course); + authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, requestingUser); + channelService.markAllChannelsOfCourseAsRead(course, requestingUser); + return ResponseEntity.ok().build(); + } + private void checkEntityIdMatchesPathIds(Channel channel, Optional courseId, Optional conversationId) { courseId.ifPresent(courseIdValue -> { if (!channel.getCourse().getId().equals(courseIdValue)) { diff --git a/src/main/webapp/app/overview/course-conversations/course-conversations.component.html b/src/main/webapp/app/overview/course-conversations/course-conversations.component.html index 6b2bac7d8ada..612880e9c6cd 100644 --- a/src/main/webapp/app/overview/course-conversations/course-conversations.component.html +++ b/src/main/webapp/app/overview/course-conversations/course-conversations.component.html @@ -30,6 +30,7 @@ [courseId]="course.id" [sidebarData]="sidebarData" (onCreateChannelPressed)="openCreateChannelDialog()" + (onMarkAllChannelsAsRead)="markAllChannelAsRead()" (onBrowsePressed)="openChannelOverviewDialog()" (onDirectChatPressed)="openCreateOneToOneChatDialog()" (onGroupChatPressed)="openCreateGroupChatDialog()" diff --git a/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts b/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts index f65878c794e1..d38efad0247c 100644 --- a/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts +++ b/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts @@ -519,6 +519,16 @@ export class CourseConversationsComponent implements OnInit, OnDestroy { }); } + markAllChannelAsRead() { + this.metisConversationService.markAllChannelsAsRead(this.course); + this.metisConversationService.forceRefresh().subscribe({ + complete: () => { + this.prepareSidebarData(); + this.closeSidebarOnMobile(); + }, + }); + } + openChannelOverviewDialog() { const subType = null; const modalRef: NgbModalRef = this.modalService.open(ChannelsOverviewDialogComponent, defaultFirstLayerDialogOptions); diff --git a/src/main/webapp/app/shared/metis/conversations/conversation.service.ts b/src/main/webapp/app/shared/metis/conversations/conversation.service.ts index 5bf109a6a476..3709c2f8016e 100644 --- a/src/main/webapp/app/shared/metis/conversations/conversation.service.ts +++ b/src/main/webapp/app/shared/metis/conversations/conversation.service.ts @@ -184,4 +184,8 @@ export class ConversationService { params = params.set('page', String(page)); return params.set('size', String(size)); }; + + markAllChannelsAsRead(courseId: number) { + return this.http.put(`${this.resourceUrl}${courseId}/channels/mark-as-read`, { observe: 'response' }); + } } diff --git a/src/main/webapp/app/shared/metis/metis-conversation.service.ts b/src/main/webapp/app/shared/metis/metis-conversation.service.ts index b21b4074294d..46610054d990 100644 --- a/src/main/webapp/app/shared/metis/metis-conversation.service.ts +++ b/src/main/webapp/app/shared/metis/metis-conversation.service.ts @@ -470,4 +470,16 @@ export class MetisConversationService implements OnDestroy { static getLinkForConversation(courseId: number): RouteComponents { return ['/courses', courseId, 'communication']; } + + markAllChannelsAsRead(course: Course | undefined) { + if (!course?.id) { + return; + } + + this.conversationService.markAllChannelsAsRead(course.id).subscribe({ + error: (errorResponse: HttpErrorResponse) => { + onError(this.alertService, errorResponse); + }, + }); + } } diff --git a/src/main/webapp/app/shared/sidebar/sidebar.component.html b/src/main/webapp/app/shared/sidebar/sidebar.component.html index e0a664240503..22ebdfb74ac7 100644 --- a/src/main/webapp/app/shared/sidebar/sidebar.component.html +++ b/src/main/webapp/app/shared/sidebar/sidebar.component.html @@ -43,6 +43,10 @@ + } diff --git a/src/main/webapp/app/shared/sidebar/sidebar.component.ts b/src/main/webapp/app/shared/sidebar/sidebar.component.ts index f3ea292820bb..180188bc8dd4 100644 --- a/src/main/webapp/app/shared/sidebar/sidebar.component.ts +++ b/src/main/webapp/app/shared/sidebar/sidebar.component.ts @@ -1,5 +1,5 @@ import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, effect, input, output } from '@angular/core'; -import { faFilter, faFilterCircleXmark, faHashtag, faPeopleGroup, faPlusCircle, faSearch, faUser } from '@fortawesome/free-solid-svg-icons'; +import { faCheck, faFilter, faFilterCircleXmark, faHashtag, faPeopleGroup, faPlusCircle, faSearch, faUser } from '@fortawesome/free-solid-svg-icons'; import { ActivatedRoute, Params } from '@angular/router'; import { Subscription, distinctUntilChanged } from 'rxjs'; import { ProfileService } from '../layouts/profiles/profile.service'; @@ -28,6 +28,7 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit { onGroupChatPressed = output(); onBrowsePressed = output(); onCreateChannelPressed = output(); + onMarkAllChannelsAsRead = output(); @Input() searchFieldEnabled: boolean = true; @Input() sidebarData: SidebarData; @Input() courseId?: number; @@ -61,6 +62,7 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit { readonly faPlusCircle = faPlusCircle; readonly faSearch = faSearch; readonly faHashtag = faHashtag; + readonly faCheck = faCheck; sidebarDataBeforeFiltering: SidebarData; @@ -195,4 +197,8 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit { achievablePoints: scoreAndPointsFilterOptions?.achievablePoints, }; } + + markAllMessagesAsChecked() { + this.onMarkAllChannelsAsRead.emit(); + } } diff --git a/src/main/webapp/i18n/de/student-dashboard.json b/src/main/webapp/i18n/de/student-dashboard.json index 642af5f892fc..a6c87a05577e 100644 --- a/src/main/webapp/i18n/de/student-dashboard.json +++ b/src/main/webapp/i18n/de/student-dashboard.json @@ -82,7 +82,8 @@ "createDirectChat": "Direkt-Chat erstellen", "groupChats": "Gruppenchats", "directMessages": "Direktnachrichten", - "filterConversationPlaceholder": "Konversationen filtern" + "filterConversationPlaceholder": "Konversationen filtern", + "setMessagesAsRead": "Alle Nachrichten als gelesen markieren" }, "menu": { "exercises": "Aufgaben", diff --git a/src/main/webapp/i18n/en/student-dashboard.json b/src/main/webapp/i18n/en/student-dashboard.json index eb79ff327373..7a39368d4615 100644 --- a/src/main/webapp/i18n/en/student-dashboard.json +++ b/src/main/webapp/i18n/en/student-dashboard.json @@ -82,7 +82,8 @@ "createDirectChat": "Create direct chat", "groupChats": "Group Chats", "directMessages": "Direct Messages", - "filterConversationPlaceholder": "Filter conversations" + "filterConversationPlaceholder": "Filter conversations", + "setMessagesAsRead": "Mark all messages as read" }, "menu": { "exercises": "Exercises", From 85e26126173aaefa2e044418e9e7de77813c4246 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Tue, 10 Dec 2024 19:03:51 +0100 Subject: [PATCH 02/12] Added testcases --- .../conversation/ConversationService.java | 3 ++ .../communication/ChannelIntegrationTest.java | 32 +++++++++++++++++++ .../course-conversations.component.spec.ts | 8 +++++ .../metis-conversation.service.spec.ts | 6 ++++ 4 files changed, 49 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index 23842e6c7e23..092c37ca42e6 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -444,6 +444,9 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted public void markConversationAsRead(Channel channel, User requestingUser) { var conversationParticipant = getOrCreateConversationParticipant(channel.getId(), requestingUser); + if (conversationParticipant.getUnreadMessagesCount() == 0) { + return; + } conversationParticipant.setLastRead(ZonedDateTime.now()); conversationParticipant.setUnreadMessagesCount(0L); conversationParticipantRepository.save(conversationParticipant); diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java index 3bb88f191080..d0aefa333622 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java @@ -7,6 +7,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -20,11 +21,13 @@ import org.springframework.security.test.context.support.WithMockUser; import org.springframework.util.LinkedMultiValueMap; +import de.tum.cit.aet.artemis.communication.domain.ConversationParticipant; import de.tum.cit.aet.artemis.communication.domain.conversation.Channel; import de.tum.cit.aet.artemis.communication.dto.ChannelDTO; import de.tum.cit.aet.artemis.communication.dto.ChannelIdAndNameDTO; import de.tum.cit.aet.artemis.communication.dto.FeedbackChannelRequestDTO; import de.tum.cit.aet.artemis.communication.dto.MetisCrudAction; +import de.tum.cit.aet.artemis.communication.service.conversation.ChannelService; import de.tum.cit.aet.artemis.communication.util.ConversationUtilService; import de.tum.cit.aet.artemis.core.domain.Course; import de.tum.cit.aet.artemis.core.domain.CourseInformationSharingConfiguration; @@ -70,6 +73,9 @@ class ChannelIntegrationTest extends AbstractConversationTest { @Autowired private ProgrammingExerciseUtilService programmingExerciseUtilService; + @Autowired + private ChannelService channelService; + @BeforeEach @Override void setupTestScenario() throws Exception { @@ -962,6 +968,32 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() { assertThat(response.getDescription()).isEqualTo("Discussion channel for feedback"); } + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void markAllChannelsAsRead() throws Exception { + // ensure there exist atleast two channel with unread messages in the course + ChannelDTO newChannel1 = createChannel(true, "channel1"); + ChannelDTO newChannel2 = createChannel(true, "channel2"); + List channels = channelRepository.findChannelsByCourseId(exampleCourseId); + channels.forEach(channel -> { + addUsersToConversation(channel.getId(), "instructor1"); + conversationParticipantRepository.findConversationParticipantsByConversationId(channel.getId()).forEach(conversationParticipant -> { + conversationParticipant.setUnreadMessagesCount(1L); + conversationParticipantRepository.save(conversationParticipant); + }); + }); + + User requestingUser = userTestRepository.getUser(); + request.put("/api/courses/" + exampleCourseId + "/channels/mark-as-read", null, HttpStatus.OK); + List updatedChannels = channelRepository.findChannelsByCourseId(exampleCourseId); + updatedChannels.forEach(channel -> { + Optional conversationParticipant = conversationParticipantRepository.findConversationParticipantByConversationIdAndUserId(channel.getId(), + requestingUser.getId()); + assertThat(conversationParticipant.get().getUnreadMessagesCount()).isEqualTo(0L); + }); + + } + private void testArchivalChangeWorks(ChannelDTO channel, boolean isPublicChannel, boolean shouldArchive) throws Exception { // prepare channel in db if (shouldArchive) { diff --git a/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts b/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts index 825d153b7af7..8c6e624cc51e 100644 --- a/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts +++ b/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts @@ -580,6 +580,14 @@ examples.forEach((activeConversation) => { }); }); + it('should mark all channels as read', () => { + const markAllChannelsAsRead = jest.spyOn(metisConversationService, 'markAllChannelsAsRead').mockReturnValue(); + const forceRefresh = jest.spyOn(metisConversationService, 'forceRefresh'); + component.markAllChannelAsRead(); + expect(markAllChannelsAsRead).toHaveBeenCalledOnce(); + expect(forceRefresh).toHaveBeenCalledTimes(2); + }); + describe('conversation selection', () => { it('should handle numeric conversationId', () => { component.onConversationSelected(123); diff --git a/src/test/javascript/spec/component/overview/course-conversations/services/metis-conversation.service.spec.ts b/src/test/javascript/spec/component/overview/course-conversations/services/metis-conversation.service.spec.ts index 489aa6a853d2..5f91ea8347e3 100644 --- a/src/test/javascript/spec/component/overview/course-conversations/services/metis-conversation.service.spec.ts +++ b/src/test/javascript/spec/component/overview/course-conversations/services/metis-conversation.service.spec.ts @@ -406,4 +406,10 @@ describe('MetisConversationService', () => { metisConversationService.markAsRead(2); expect(metisConversationService['conversationsOfUser'][1].unreadMessagesCount).toBe(0); }); + + it('should call refresh after marking all channels as read', () => { + const markAllChannelAsReadSpy = jest.spyOn(conversationService, 'markAllChannelsAsRead').mockReturnValue(of()); + metisConversationService.markAllChannelsAsRead(course); + expect(markAllChannelAsReadSpy).toHaveBeenCalledOnce(); + }); }); From 70531a353c4e15591d7aa8d459bf38ca862a4eef Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Thu, 12 Dec 2024 11:16:04 +0100 Subject: [PATCH 03/12] Renamed button --- src/main/webapp/app/shared/sidebar/sidebar.component.html | 2 +- src/main/webapp/i18n/de/student-dashboard.json | 2 +- src/main/webapp/i18n/en/student-dashboard.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/shared/sidebar/sidebar.component.html b/src/main/webapp/app/shared/sidebar/sidebar.component.html index 22ebdfb74ac7..ce47071dd115 100644 --- a/src/main/webapp/app/shared/sidebar/sidebar.component.html +++ b/src/main/webapp/app/shared/sidebar/sidebar.component.html @@ -45,7 +45,7 @@ diff --git a/src/main/webapp/i18n/de/student-dashboard.json b/src/main/webapp/i18n/de/student-dashboard.json index a6c87a05577e..1efa807cf985 100644 --- a/src/main/webapp/i18n/de/student-dashboard.json +++ b/src/main/webapp/i18n/de/student-dashboard.json @@ -83,7 +83,7 @@ "groupChats": "Gruppenchats", "directMessages": "Direktnachrichten", "filterConversationPlaceholder": "Konversationen filtern", - "setMessagesAsRead": "Alle Nachrichten als gelesen markieren" + "setChannelAsRead": "Alle Kanäle als gelesen markieren" }, "menu": { "exercises": "Aufgaben", diff --git a/src/main/webapp/i18n/en/student-dashboard.json b/src/main/webapp/i18n/en/student-dashboard.json index 7a39368d4615..a1be35bda0a4 100644 --- a/src/main/webapp/i18n/en/student-dashboard.json +++ b/src/main/webapp/i18n/en/student-dashboard.json @@ -83,7 +83,7 @@ "groupChats": "Group Chats", "directMessages": "Direct Messages", "filterConversationPlaceholder": "Filter conversations", - "setMessagesAsRead": "Mark all messages as read" + "setChannelAsRead": "Mark all channels as read" }, "menu": { "exercises": "Exercises", From 624db7c5589054080b30e6eeafcc832c4b8cf109 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Thu, 12 Dec 2024 12:54:48 +0100 Subject: [PATCH 04/12] Remove 0 check --- .../service/conversation/ConversationService.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index c37243ba9d4a..a905d7f11456 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -446,9 +446,6 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted public void markConversationAsRead(Channel channel, User requestingUser) { var conversationParticipant = getOrCreateConversationParticipant(channel.getId(), requestingUser); - if (conversationParticipant.getUnreadMessagesCount() == 0) { - return; - } conversationParticipant.setLastRead(ZonedDateTime.now()); conversationParticipant.setUnreadMessagesCount(0L); conversationParticipantRepository.save(conversationParticipant); From 1f4915b7b35cc698c18d2d3e208e13e13c27b7e7 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Sat, 14 Dec 2024 02:42:32 +0100 Subject: [PATCH 05/12] Try to fix server performance issue --- .../course-conversations.component.ts | 11 +++++++---- .../app/shared/metis/metis-conversation.service.ts | 11 ++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts b/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts index 478565002f37..94cfb4929066 100644 --- a/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts +++ b/src/main/webapp/app/overview/course-conversations/course-conversations.component.ts @@ -522,11 +522,14 @@ export class CourseConversationsComponent implements OnInit, OnDestroy { } markAllChannelAsRead() { - this.metisConversationService.markAllChannelsAsRead(this.course); - this.metisConversationService.forceRefresh().subscribe({ + this.metisConversationService.markAllChannelsAsRead(this.course).subscribe({ complete: () => { - this.prepareSidebarData(); - this.closeSidebarOnMobile(); + this.metisConversationService.forceRefresh().subscribe({ + complete: () => { + this.prepareSidebarData(); + this.closeSidebarOnMobile(); + }, + }); }, }); } diff --git a/src/main/webapp/app/shared/metis/metis-conversation.service.ts b/src/main/webapp/app/shared/metis/metis-conversation.service.ts index 46610054d990..6045c2b19717 100644 --- a/src/main/webapp/app/shared/metis/metis-conversation.service.ts +++ b/src/main/webapp/app/shared/metis/metis-conversation.service.ts @@ -473,13 +473,14 @@ export class MetisConversationService implements OnDestroy { markAllChannelsAsRead(course: Course | undefined) { if (!course?.id) { - return; + return EMPTY; } - this.conversationService.markAllChannelsAsRead(course.id).subscribe({ - error: (errorResponse: HttpErrorResponse) => { + return this.conversationService.markAllChannelsAsRead(course.id).pipe( + catchError((errorResponse: HttpErrorResponse) => { onError(this.alertService, errorResponse); - }, - }); + return EMPTY; + }), + ); } } From 2a2bb79b9295b9cedbd545a1e39826e8cfd5101c Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Sat, 14 Dec 2024 02:44:40 +0100 Subject: [PATCH 06/12] Try to fix server performance issue --- .../webapp/app/shared/metis/metis-conversation.service.ts | 4 ++-- .../course-conversations.component.spec.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/shared/metis/metis-conversation.service.ts b/src/main/webapp/app/shared/metis/metis-conversation.service.ts index 6045c2b19717..aa5029d6fc23 100644 --- a/src/main/webapp/app/shared/metis/metis-conversation.service.ts +++ b/src/main/webapp/app/shared/metis/metis-conversation.service.ts @@ -473,13 +473,13 @@ export class MetisConversationService implements OnDestroy { markAllChannelsAsRead(course: Course | undefined) { if (!course?.id) { - return EMPTY; + return of(); } return this.conversationService.markAllChannelsAsRead(course.id).pipe( catchError((errorResponse: HttpErrorResponse) => { onError(this.alertService, errorResponse); - return EMPTY; + return of(); }), ); } diff --git a/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts b/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts index 8c6e624cc51e..7add13f9c64f 100644 --- a/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts +++ b/src/test/javascript/spec/component/overview/course-conversations/course-conversations.component.spec.ts @@ -581,7 +581,7 @@ examples.forEach((activeConversation) => { }); it('should mark all channels as read', () => { - const markAllChannelsAsRead = jest.spyOn(metisConversationService, 'markAllChannelsAsRead').mockReturnValue(); + const markAllChannelsAsRead = jest.spyOn(metisConversationService, 'markAllChannelsAsRead').mockReturnValue(of()); const forceRefresh = jest.spyOn(metisConversationService, 'forceRefresh'); component.markAllChannelAsRead(); expect(markAllChannelsAsRead).toHaveBeenCalledOnce(); From 507fd42f83cd9f94f80fac27962d8679ebf5a9f1 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Sat, 14 Dec 2024 03:05:27 +0100 Subject: [PATCH 07/12] Fix server style --- .../communication/web/conversation/ChannelResource.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java index 642dd02a5822..c9d2f0423380 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java @@ -496,6 +496,12 @@ public ResponseEntity createFeedbackChannel(@PathVariable Long cours return ResponseEntity.created(new URI("/api/channels/" + createdChannel.getId())).body(conversationDTOService.convertChannelToDTO(requestingUser, createdChannel)); } + /** + * PUT /api/courses/:courseId/channels/mark-as-read: Marks all channels of a course as read for the current user. + * + * @param courseId the id of the course. + * @return ResponseEntity with status 200 (Ok). + */ @PutMapping("{courseId}/channels/mark-as-read") @EnforceAtLeastStudent public ResponseEntity markAllChannelsOfCourseAsRead(@PathVariable Long courseId) { From 078d72bd314f659398ee45fa25c26ad8b0c1f432 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Mon, 16 Dec 2024 07:17:19 +0100 Subject: [PATCH 08/12] Hopefully fix access error --- .../conversation/ConversationRepository.java | 3 +++ .../service/conversation/ChannelService.java | 5 +---- .../conversation/ConversationService.java | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java index c0c7303336c1..5c525c5fd1d8 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java @@ -89,4 +89,7 @@ SELECT COUNT(p.id) > 0 ) """) boolean userHasUnreadMessageInCourse(@Param("courseId") Long courseId, @Param("userId") Long userId); + + @Query + List findAllByCourseId(@Param("courseId") Long courseId); } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java index 90260698ecc4..832c91c6523b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java @@ -452,9 +452,6 @@ public Channel createFeedbackChannel(Course course, Long exerciseId, ChannelDTO } public void markAllChannelsOfCourseAsRead(Course course, User requestingUser) { - var channels = channelRepository.findChannelsByCourseId(course.getId()); - for (Channel channel : channels) { - conversationService.markConversationAsRead(channel, requestingUser); - } + conversationService.markConversationAsRead(course.getId(), requestingUser); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index a905d7f11456..62c2560e4935 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -444,11 +444,20 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted conversationParticipantRepository.save(conversationParticipant); } - public void markConversationAsRead(Channel channel, User requestingUser) { - var conversationParticipant = getOrCreateConversationParticipant(channel.getId(), requestingUser); - conversationParticipant.setLastRead(ZonedDateTime.now()); - conversationParticipant.setUnreadMessagesCount(0L); - conversationParticipantRepository.save(conversationParticipant); + public void markConversationAsRead(Long courseId, User requestingUser) { + List conversations = conversationRepository.findAllByCourseId(courseId); + for (Conversation conversation : conversations) { + boolean userCanBePartOfConversation = conversationParticipantRepository + .findConversationParticipantByConversationIdAndUserId(conversation.getId(), requestingUser.getId()).isPresent() + || (conversation instanceof Channel channel && channel.getIsCourseWide()); + if (userCanBePartOfConversation) { + ConversationParticipant conversationParticipant = getOrCreateConversationParticipant(conversation.getId(), requestingUser); + conversationParticipant.setLastRead(ZonedDateTime.now()); + conversationParticipant.setUnreadMessagesCount(0L); + conversationParticipantRepository.save(conversationParticipant); + } + + } } /** From 693afc6c126a33675acaa37c41ce4675e1d9e24a Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Mon, 16 Dec 2024 07:47:02 +0100 Subject: [PATCH 09/12] Optimize query to only save once --- .../ConversationParticipantRepository.java | 1 + .../service/conversation/ChannelService.java | 8 +++++++- .../conversation/ConversationService.java | 18 ++++++++++++++---- .../communication/ChannelIntegrationTest.java | 3 --- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java b/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java index aaf733332105..4e92454cd131 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java @@ -113,4 +113,5 @@ public interface ConversationParticipantRepository extends ArtemisJpaRepository< AND conversationParticipant.unreadMessagesCount IS NOT NULL """) void decrementUnreadMessagesCountOfParticipants(@Param("conversationId") Long conversationId, @Param("senderId") Long senderId); + } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java index 832c91c6523b..ec61f3a8fe42 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java @@ -451,7 +451,13 @@ public Channel createFeedbackChannel(Course course, Long exerciseId, ChannelDTO return createdChannel; } + /** + * Marks all channels of a course as read for the requesting user. + * + * @param course the course for which all channels should be marked as read. + * @param requestingUser the user requesting the marking of all channels as read. + */ public void markAllChannelsOfCourseAsRead(Course course, User requestingUser) { - conversationService.markConversationAsRead(course.getId(), requestingUser); + conversationService.markAllConversationOfAUserAsRead(course.getId(), requestingUser); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index 62c2560e4935..0ed2bd4eff59 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -12,6 +12,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import jakarta.transaction.Transactional; import jakarta.validation.constraints.NotNull; import org.springframework.context.annotation.Profile; @@ -444,19 +445,28 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted conversationParticipantRepository.save(conversationParticipant); } - public void markConversationAsRead(Long courseId, User requestingUser) { + /** + * Mark all conversation of a user as read + * + * @param courseId the id of the course + * @param requestingUser the user that wants to mark the conversation as read + */ + @Transactional + public void markAllConversationOfAUserAsRead(Long courseId, User requestingUser) { List conversations = conversationRepository.findAllByCourseId(courseId); + ZonedDateTime now = ZonedDateTime.now(); + List participants = new ArrayList<>(); for (Conversation conversation : conversations) { boolean userCanBePartOfConversation = conversationParticipantRepository .findConversationParticipantByConversationIdAndUserId(conversation.getId(), requestingUser.getId()).isPresent() || (conversation instanceof Channel channel && channel.getIsCourseWide()); if (userCanBePartOfConversation) { ConversationParticipant conversationParticipant = getOrCreateConversationParticipant(conversation.getId(), requestingUser); - conversationParticipant.setLastRead(ZonedDateTime.now()); + conversationParticipant.setLastRead(now); conversationParticipant.setUnreadMessagesCount(0L); - conversationParticipantRepository.save(conversationParticipant); + participants.add(conversationParticipant); } - + conversationParticipantRepository.saveAll(participants); } } diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java index 4ce6c6753310..18fb5eb34cff 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java @@ -28,11 +28,8 @@ import de.tum.cit.aet.artemis.communication.dto.ChannelIdAndNameDTO; import de.tum.cit.aet.artemis.communication.dto.FeedbackChannelRequestDTO; import de.tum.cit.aet.artemis.communication.dto.MetisCrudAction; - import de.tum.cit.aet.artemis.communication.service.conversation.ChannelService; - import de.tum.cit.aet.artemis.communication.service.conversation.ConversationService; - import de.tum.cit.aet.artemis.communication.util.ConversationUtilService; import de.tum.cit.aet.artemis.core.domain.Course; import de.tum.cit.aet.artemis.core.domain.CourseInformationSharingConfiguration; From 013b8f7eedd18eaf49a64aab11732829d19f19f8 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Mon, 16 Dec 2024 07:56:18 +0100 Subject: [PATCH 10/12] Fix server style --- .../repository/ConversationParticipantRepository.java | 1 - .../repository/conversation/ConversationRepository.java | 1 - .../communication/service/conversation/ConversationService.java | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java b/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java index 4e92454cd131..aaf733332105 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationParticipantRepository.java @@ -113,5 +113,4 @@ public interface ConversationParticipantRepository extends ArtemisJpaRepository< AND conversationParticipant.unreadMessagesCount IS NOT NULL """) void decrementUnreadMessagesCountOfParticipants(@Param("conversationId") Long conversationId, @Param("senderId") Long senderId); - } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java index 5c525c5fd1d8..489adbb69a1f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java @@ -90,6 +90,5 @@ SELECT COUNT(p.id) > 0 """) boolean userHasUnreadMessageInCourse(@Param("courseId") Long courseId, @Param("userId") Long userId); - @Query List findAllByCourseId(@Param("courseId") Long courseId); } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index 0ed2bd4eff59..8c754586b13d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -12,13 +12,13 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import jakarta.transaction.Transactional; import jakarta.validation.constraints.NotNull; import org.springframework.context.annotation.Profile; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.RequestBody; import de.tum.cit.aet.artemis.communication.domain.ConversationParticipant; From 65bcb70b1910e3d253cc2cbc79763aca8494c9de Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Mon, 16 Dec 2024 08:19:40 +0100 Subject: [PATCH 11/12] Fix server style --- .../repository/conversation/ConversationRepository.java | 8 +++++++- .../service/conversation/ConversationService.java | 2 -- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java index 489adbb69a1f..df3782a95a22 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/repository/conversation/ConversationRepository.java @@ -90,5 +90,11 @@ SELECT COUNT(p.id) > 0 """) boolean userHasUnreadMessageInCourse(@Param("courseId") Long courseId, @Param("userId") Long userId); - List findAllByCourseId(@Param("courseId") Long courseId); + /** + * Retrieves a list of conversations for the given course + * + * @param courseId the course id + * @return a list of conversations for the given course + */ + List findAllByCourseId(Long courseId); } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java index 8c754586b13d..56afa4a5c497 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ConversationService.java @@ -18,7 +18,6 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.RequestBody; import de.tum.cit.aet.artemis.communication.domain.ConversationParticipant; @@ -451,7 +450,6 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted * @param courseId the id of the course * @param requestingUser the user that wants to mark the conversation as read */ - @Transactional public void markAllConversationOfAUserAsRead(Long courseId, User requestingUser) { List conversations = conversationRepository.findAllByCourseId(courseId); ZonedDateTime now = ZonedDateTime.now(); From ef89ab2d0a226cdeed18b888026a0e8cebcfd901 Mon Sep 17 00:00:00 2001 From: Tim Cremer Date: Mon, 16 Dec 2024 08:52:27 +0100 Subject: [PATCH 12/12] Double checkmark for read message --- src/main/webapp/app/shared/sidebar/sidebar.component.html | 2 +- src/main/webapp/app/shared/sidebar/sidebar.component.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/shared/sidebar/sidebar.component.html b/src/main/webapp/app/shared/sidebar/sidebar.component.html index deca3004d1a5..64b4517cd43a 100644 --- a/src/main/webapp/app/shared/sidebar/sidebar.component.html +++ b/src/main/webapp/app/shared/sidebar/sidebar.component.html @@ -46,7 +46,7 @@ diff --git a/src/main/webapp/app/shared/sidebar/sidebar.component.ts b/src/main/webapp/app/shared/sidebar/sidebar.component.ts index 180188bc8dd4..839f1583f930 100644 --- a/src/main/webapp/app/shared/sidebar/sidebar.component.ts +++ b/src/main/webapp/app/shared/sidebar/sidebar.component.ts @@ -1,5 +1,5 @@ import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, effect, input, output } from '@angular/core'; -import { faCheck, faFilter, faFilterCircleXmark, faHashtag, faPeopleGroup, faPlusCircle, faSearch, faUser } from '@fortawesome/free-solid-svg-icons'; +import { faCheckDouble, faFilter, faFilterCircleXmark, faHashtag, faPeopleGroup, faPlusCircle, faSearch, faUser } from '@fortawesome/free-solid-svg-icons'; import { ActivatedRoute, Params } from '@angular/router'; import { Subscription, distinctUntilChanged } from 'rxjs'; import { ProfileService } from '../layouts/profiles/profile.service'; @@ -62,7 +62,7 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit { readonly faPlusCircle = faPlusCircle; readonly faSearch = faSearch; readonly faHashtag = faHashtag; - readonly faCheck = faCheck; + readonly faCheckDouble = faCheckDouble; sidebarDataBeforeFiltering: SidebarData;