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: Allow users to mark all channels as read #9994

Merged
merged 22 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
efd8ef5
Preliminary implementation
cremertim Dec 6, 2024
85e2612
Added testcases
cremertim Dec 10, 2024
9b13e85
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 11, 2024
4dc66ea
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 12, 2024
558106e
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 12, 2024
70531a3
Renamed button
cremertim Dec 12, 2024
a2f58cd
Merge remote-tracking branch 'origin/feature/communication/set-all-me…
cremertim Dec 12, 2024
624db7c
Remove 0 check
cremertim Dec 12, 2024
8f77c2c
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 14, 2024
1f4915b
Try to fix server performance issue
cremertim Dec 14, 2024
2a2bb79
Try to fix server performance issue
cremertim Dec 14, 2024
4b401ce
Merge remote-tracking branch 'origin/feature/communication/set-all-me…
cremertim Dec 14, 2024
507fd42
Fix server style
cremertim Dec 14, 2024
fa2c428
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 15, 2024
e945fe8
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 15, 2024
078d72b
Hopefully fix access error
cremertim Dec 16, 2024
693afc6
Optimize query to only save once
cremertim Dec 16, 2024
013b8f7
Fix server style
cremertim Dec 16, 2024
65bcb70
Fix server style
cremertim Dec 16, 2024
22693fe
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 16, 2024
ef89ab2
Double checkmark for read message
cremertim Dec 16, 2024
6e2b5a3
Merge branch 'develop' into feature/communication/set-all-messages-to…
cremertim Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,12 @@ SELECT COUNT(p.id) > 0
)
""")
boolean userHasUnreadMessageInCourse(@Param("courseId") Long courseId, @Param("userId") Long userId);

/**
* Retrieves a list of conversations for the given course
*
* @param courseId the course id
* @return a list of conversations for the given course
*/
List<Conversation> findAllByCourseId(Long courseId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -450,4 +450,14 @@ 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.markAllConversationOfAUserAsRead(course.getId(), requestingUser);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,30 @@ public void setIsMuted(Long conversationId, User requestingUser, boolean isMuted
conversationParticipantRepository.save(conversationParticipant);
}

/**
* 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
*/
public void markAllConversationOfAUserAsRead(Long courseId, User requestingUser) {
List<Conversation> conversations = conversationRepository.findAllByCourseId(courseId);
ZonedDateTime now = ZonedDateTime.now();
List<ConversationParticipant> 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(now);
conversationParticipant.setUnreadMessagesCount(0L);
participants.add(conversationParticipant);
}
conversationParticipantRepository.saveAll(participants);
}
}

/**
* The user can select one of these roles to filter the conversation members by role
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,24 @@ public ResponseEntity<ChannelDTO> 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<ChannelDTO> 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<Long> courseId, Optional<Long> conversationId) {
courseId.ifPresent(courseIdValue -> {
if (!channel.getCourse().getId().equals(courseIdValue)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
[courseId]="course.id"
[sidebarData]="sidebarData"
(onCreateChannelPressed)="openCreateChannelDialog()"
(onMarkAllChannelsAsRead)="markAllChannelAsRead()"
(onBrowsePressed)="openChannelOverviewDialog()"
(onDirectChatPressed)="openCreateOneToOneChatDialog()"
(onGroupChatPressed)="openCreateGroupChatDialog()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,19 @@ export class CourseConversationsComponent implements OnInit, OnDestroy {
});
}

markAllChannelAsRead() {
this.metisConversationService.markAllChannelsAsRead(this.course).subscribe({
complete: () => {
this.metisConversationService.forceRefresh().subscribe({
complete: () => {
this.prepareSidebarData();
this.closeSidebarOnMobile();
},
});
},
});
}

openChannelOverviewDialog() {
const subType = null;
const modalRef: NgbModalRef = this.modalService.open(ChannelsOverviewDialogComponent, defaultFirstLayerDialogOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>(`${this.resourceUrl}${courseId}/channels/mark-as-read`, { observe: 'response' });
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved
}
13 changes: 13 additions & 0 deletions src/main/webapp/app/shared/metis/metis-conversation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,17 @@ export class MetisConversationService implements OnDestroy {
static getLinkForConversation(courseId: number): RouteComponents {
return ['/courses', courseId, 'communication'];
}

markAllChannelsAsRead(course: Course | undefined) {
if (!course?.id) {
return of();
}

return this.conversationService.markAllChannelsAsRead(course.id).pipe(
catchError((errorResponse: HttpErrorResponse) => {
onError(this.alertService, errorResponse);
return of();
}),
);
}
}
4 changes: 4 additions & 0 deletions src/main/webapp/app/shared/sidebar/sidebar.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
<fa-icon [icon]="faSearch" class="item-icon"></fa-icon>
<span jhiTranslate="artemisApp.courseOverview.sidebar.browseChannels"></span>
</button>
<button (click)="markAllMessagesAsChecked()" class="p-2" ngbDropdownItem>
<fa-icon [icon]="faCheckDouble" class="item-icon"></fa-icon>
<span jhiTranslate="artemisApp.courseOverview.sidebar.setChannelAsRead"></span>
</button>
</div>
</div>
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/webapp/app/shared/sidebar/sidebar.component.ts
Original file line number Diff line number Diff line change
@@ -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 { 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';
Expand Down Expand Up @@ -28,6 +28,7 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit {
onGroupChatPressed = output<void>();
onBrowsePressed = output<void>();
onCreateChannelPressed = output<void>();
onMarkAllChannelsAsRead = output<void>();
@Input() searchFieldEnabled: boolean = true;
@Input() sidebarData: SidebarData;
@Input() courseId?: number;
Expand Down Expand Up @@ -61,6 +62,7 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit {
readonly faPlusCircle = faPlusCircle;
readonly faSearch = faSearch;
readonly faHashtag = faHashtag;
readonly faCheckDouble = faCheckDouble;

sidebarDataBeforeFiltering: SidebarData;

Expand Down Expand Up @@ -195,4 +197,8 @@ export class SidebarComponent implements OnDestroy, OnChanges, OnInit {
achievablePoints: scoreAndPointsFilterOptions?.achievablePoints,
};
}

markAllMessagesAsChecked() {
this.onMarkAllChannelsAsRead.emit();
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved
}
3 changes: 2 additions & 1 deletion src/main/webapp/i18n/de/student-dashboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@
"createDirectChat": "Direkt-Chat erstellen",
"groupChats": "Gruppenchats",
"directMessages": "Direktnachrichten",
"filterConversationPlaceholder": "Konversationen filtern"
"filterConversationPlaceholder": "Konversationen filtern",
"setChannelAsRead": "Alle Kanäle als gelesen markieren"
},
"menu": {
"exercises": "Aufgaben",
Expand Down
3 changes: 2 additions & 1 deletion src/main/webapp/i18n/en/student-dashboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@
"createDirectChat": "Create direct chat",
"groupChats": "Group Chats",
"directMessages": "Direct Messages",
"filterConversationPlaceholder": "Filter conversations"
"filterConversationPlaceholder": "Filter conversations",
"setChannelAsRead": "Mark all channels as read"
},
"menu": {
"exercises": "Exercises",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,11 +22,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.service.conversation.ConversationService;
import de.tum.cit.aet.artemis.communication.util.ConversationUtilService;
import de.tum.cit.aet.artemis.core.domain.Course;
Expand Down Expand Up @@ -75,6 +78,9 @@ class ChannelIntegrationTest extends AbstractConversationTest {
@Autowired
private ProgrammingExerciseUtilService programmingExerciseUtilService;

@Autowired
private ChannelService channelService;

@BeforeEach
@Override
void setupTestScenario() throws Exception {
Expand Down Expand Up @@ -973,6 +979,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<Channel> 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<Channel> updatedChannels = channelRepository.findChannelsByCourseId(exampleCourseId);
updatedChannels.forEach(channel -> {
Optional<ConversationParticipant> conversationParticipant = conversationParticipantRepository.findConversationParticipantByConversationIdAndUserId(channel.getId(),
requestingUser.getId());
assertThat(conversationParticipant.get().getUnreadMessagesCount()).isEqualTo(0L);
});

}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

private void testArchivalChangeWorks(ChannelDTO channel, boolean isPublicChannel, boolean shouldArchive) throws Exception {
// prepare channel in db
if (shouldArchive) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,14 @@ examples.forEach((activeConversation) => {
});
});

it('should mark all channels as read', () => {
const markAllChannelsAsRead = jest.spyOn(metisConversationService, 'markAllChannelsAsRead').mockReturnValue(of());
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
cremertim marked this conversation as resolved.
Show resolved Hide resolved
});
Loading