Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -6,6 +6,7 @@
import com.fredmaina.chatapp.Auth.services.JWTService;
import com.fredmaina.chatapp.core.DTOs.ChatMessageDto;
import com.fredmaina.chatapp.core.DTOs.ChatSessionDto;
import com.fredmaina.chatapp.core.DTOs.PaginationDto;
import com.fredmaina.chatapp.core.Repositories.ChatMessageRepository;
import com.fredmaina.chatapp.core.Services.ChatService;
import com.fredmaina.chatapp.core.models.ChatMessage;
Expand Down Expand Up @@ -63,12 +64,21 @@ public ResponseEntity<?> getUserChats(@RequestHeader(value = "Authorization", re
@GetMapping("/chat/session_history")
public ResponseEntity<?> getAnonChatHistory(
@RequestParam String sessionId,
@RequestParam String recipient // This is the username of the dashboard owner
@RequestParam String recipient, // This is the username of the dashboard owner
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "50") int size
Comment on lines +68 to +69
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation. The @RequestParam annotation on line 68 has extra leading spaces compared to the @RequestParam on line 69, making the parameter list formatting inconsistent.

Suggested change
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "50") int size
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "50") int size

Copilot uses AI. Check for mistakes.
) {
;
List<ChatMessageDto> messages = chatService.getChatHistoryForAnonymous(sessionId, recipient);

return ResponseEntity.ok(Map.of("success", true, "messages", messages));
PaginationDto chatPage =
chatService.getChatHistoryForAnonymous(sessionId, recipient, page, size);

return ResponseEntity.ok(Map.of(
"success", true,
"page", chatPage.getPage(),
"size", chatPage.getSize(),
"totalPages", chatPage.getTotalPages(),
"hasNextPage", chatPage.isHasNextPage(),
"messages", chatPage.getMessages()
));
}

@DeleteMapping("/chat/{anonSessionId}")
Expand Down
19 changes: 19 additions & 0 deletions src/main/java/com/fredmaina/chatapp/core/DTOs/PaginationDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.fredmaina.chatapp.core.DTOs;

import lombok.AllArgsConstructor;
import lombok.Data;

import java.util.List;

@Data
@AllArgsConstructor
public class PaginationDto {
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name PaginationDto is too generic for a DTO that is specifically designed for chat message pagination. Based on the PR description mentioning ChatHistoryPageDto and following the naming pattern of other DTOs in this package (like ChatMessageDto, ChatSessionDto), this should be named ChatHistoryPageDto instead. This would make the purpose clearer and avoid confusion if other paginated endpoints are added in the future.

Suggested change
public class PaginationDto {
public class ChatHistoryPageDto {

Copilot uses AI. Check for mistakes.

private List<ChatMessageDto> messages;
private int page;
private int size;
private int totalPages;
private boolean hasNextPage;
}


Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.fredmaina.chatapp.Auth.Models.User;
import com.fredmaina.chatapp.core.models.ChatMessage;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
Expand All @@ -24,8 +26,9 @@ public interface ChatMessageRepository extends JpaRepository<ChatMessage, UUID>
"OR " +
"(m.toSessionId = :sessionId AND m.fromUser.id = :recipientId) " +
"ORDER BY m.timestamp")
Comment on lines 27 to 28
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded ORDER BY m.timestamp in the @Query annotation conflicts with the Sort parameter passed through the Pageable. When Sort.by("timestamp").descending() is provided in the Pageable (as done in ChatService.java line 86), Spring Data JPA will append it to the query, resulting in duplicate ORDER BY clauses which could cause unexpected behavior or errors. Remove the ORDER BY m.timestamp from the query string and let the Pageable handle the sorting.

Suggested change
"(m.toSessionId = :sessionId AND m.fromUser.id = :recipientId) " +
"ORDER BY m.timestamp")
"(m.toSessionId = :sessionId AND m.fromUser.id = :recipientId)")

Copilot uses AI. Check for mistakes.
List<ChatMessage> findFullChatHistory(@Param("sessionId") String sessionId,
@Param("recipientId") UUID recipientId);
Page<ChatMessage> findFullChatHistory(@Param("sessionId") String sessionId,
@Param("recipientId") UUID recipientId
, Pageable pageable);
Comment on lines +29 to +31
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent parameter formatting. The Pageable pageable parameter should be on the same line as the previous parameter or consistently indented. The comma-space before Pageable should follow the same pattern as line 30.

Suggested change
Page<ChatMessage> findFullChatHistory(@Param("sessionId") String sessionId,
@Param("recipientId") UUID recipientId
, Pageable pageable);
Page<ChatMessage> findFullChatHistory(
@Param("sessionId") String sessionId,
@Param("recipientId") UUID recipientId,
Pageable pageable);

Copilot uses AI. Check for mistakes.



Expand Down
21 changes: 18 additions & 3 deletions src/main/java/com/fredmaina/chatapp/core/Services/ChatService.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
import com.fredmaina.chatapp.core.DTOs.ChatMessageDto;
import com.fredmaina.chatapp.core.DTOs.ChatMessageMapper;
import com.fredmaina.chatapp.core.DTOs.ChatSessionDto;
import com.fredmaina.chatapp.core.DTOs.PaginationDto;
import com.fredmaina.chatapp.core.Repositories.ChatMessageRepository;
import com.fredmaina.chatapp.core.models.ChatMessage;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand Down Expand Up @@ -73,17 +78,27 @@ public List<ChatSessionDto> getUserChatSessions(UUID userId) {
}

@Transactional
public List<ChatMessageDto> getChatHistoryForAnonymous(String sessionId, String recipientUsername) {
public PaginationDto getChatHistoryForAnonymous(String sessionId, String recipientUsername,
int page,int size) {
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation for page and size parameters. Negative values for page or non-positive values for size would cause issues when passed to PageRequest.of(). Consider adding validation to ensure page >= 0 and size > 0, or handle the IllegalArgumentException that PageRequest.of() throws for invalid values. You could also set reasonable upper bounds for size to prevent excessive memory usage.

Suggested change
int page,int size) {
int page,int size) {
// Validate page and size parameters
if (page < 0) {
throw new IllegalArgumentException("Page index must not be negative: " + page);
}
int MAX_PAGE_SIZE = 100;
if (size <= 0 || size > MAX_PAGE_SIZE) {
throw new IllegalArgumentException("Page size must be between 1 and " + MAX_PAGE_SIZE + ": " + size);
}

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma in the parameter list. Should be int page, int size instead of int page,int size for consistency with Java formatting conventions.

Suggested change
int page,int size) {
int page, int size) {

Copilot uses AI. Check for mistakes.
User recipient = userRepository.findByUsername(recipientUsername)
.orElseThrow(() -> new RuntimeException("Recipient user not found: " + recipientUsername));

List<ChatMessage> messages = chatMessageRepository.findFullChatHistory(sessionId,recipient.getId());
Pageable pageable = PageRequest.of(page, size, Sort.by("timestamp").descending());
Page<ChatMessage> messages = chatMessageRepository.findFullChatHistory(sessionId,recipient.getId(),pageable);
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after commas in method call. Should be findFullChatHistory(sessionId, recipient.getId(), pageable) instead of findFullChatHistory(sessionId,recipient.getId(),pageable) for consistency with Java formatting conventions.

Suggested change
Page<ChatMessage> messages = chatMessageRepository.findFullChatHistory(sessionId,recipient.getId(),pageable);
Page<ChatMessage> messages = chatMessageRepository.findFullChatHistory(sessionId, recipient.getId(), pageable);

Copilot uses AI. Check for mistakes.

chatMessageRepository.markMessagesAsRead(recipient.getId(), sessionId);

return messages.stream()
List<ChatMessageDto> dtoList = messages.stream()
.map(msg -> ChatMessageMapper.toDto(msg, recipient.getId()))
.collect(Collectors.toList());

return new PaginationDto(
dtoList,
messages.getNumber(),
messages.getSize(),
messages.getTotalPages(),
messages.hasNext()
);
}


Expand Down