Skip to content
Merged
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
@@ -1,6 +1,10 @@
package com.testify.Testify_Backend.controller;

import com.testify.Testify_Backend.responses.GenericAddOrUpdateResponse;
import com.testify.Testify_Backend.responses.exam_management.CandidateResponse;
import com.testify.Testify_Backend.responses.exam_management.ExamResponse;
import com.testify.Testify_Backend.responses.exam_management.OrganizationResponse;
import com.testify.Testify_Backend.service.ExamManagementService;
import com.testify.Testify_Backend.responses.exam_management.ExamResponse;
import com.testify.Testify_Backend.responses.exam_management.OrganizationResponse;
import com.testify.Testify_Backend.responses.examsetter_management.ModerateExamResponse;
Expand All @@ -20,6 +24,7 @@
public class ExamSetterController {
private static final Logger log = LoggerFactory.getLogger(ExamSetterController.class);
private final ExamSetterService examSetterService;
private final ExamManagementService examManagementService;

@GetMapping("/{setterId}/getOrganizations")
public ResponseEntity<Set<OrganizationResponse>> getOrganizations(@PathVariable("setterId") long setterId) {
Expand All @@ -40,6 +45,17 @@ public ResponseEntity<GenericAddOrUpdateResponse> addSetterToOrganization(@PathV
return ResponseEntity.ok(response);
}

@GetMapping("/proctor/{proctorId}/{organizationId}")
public ResponseEntity<List<ExamResponse>> getExamsForProctor(@PathVariable Long proctorId, @PathVariable Long organizationId) {
List<ExamResponse> exams = examSetterService.getExamsForProctor(proctorId, organizationId);
return ResponseEntity.ok(exams);
}
Comment on lines +48 to +52
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and error handling.

The endpoint needs validation for:

  • Non-null and positive IDs
  • Existence of proctor and organization
  • Proctor's association with the organization

Add validation and error handling:

     @GetMapping("/proctor/{proctorId}/{organizationId}")
     public ResponseEntity<List<ExamResponse>> getExamsForProctor(@PathVariable Long proctorId, @PathVariable Long organizationId) {
+        if (proctorId == null || proctorId <= 0 || organizationId == null || organizationId <= 0) {
+            return ResponseEntity.badRequest().build();
+        }
         List<ExamResponse> exams = examSetterService.getExamsForProctor(proctorId, organizationId);
         return ResponseEntity.ok(exams);
     }

Committable suggestion skipped: line range outside the PR's diff.


@GetMapping("/{examId}/candidates")
public ResponseEntity<Set<CandidateResponse>> getCandidatesForExam(@PathVariable Long examId) {
Set<CandidateResponse> candidates = examSetterService.getCandidatesForExam(examId);
return ResponseEntity.ok(candidates);
}
Comment on lines +54 to +58
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and error handling.

The endpoint needs validation for:

  • Non-null and positive exam ID
  • Existence of exam

Add validation and error handling:

     @GetMapping("/{examId}/candidates")
     public ResponseEntity<Set<CandidateResponse>> getCandidatesForExam(@PathVariable Long examId) {
+        if (examId == null || examId <= 0) {
+            return ResponseEntity.badRequest().build();
+        }
         Set<CandidateResponse> candidates = examSetterService.getCandidatesForExam(examId);
         return ResponseEntity.ok(candidates);
     }

Committable suggestion skipped: line range outside the PR's diff.

@GetMapping("/{examSetterId}/moderating-exams")
public ResponseEntity<List<ModerateExamResponse>> getModeratingExams(@PathVariable long examSetterId) {
List<ModerateExamResponse> responses = examSetterService.getModeratingExams(examSetterId);
Expand All @@ -52,4 +68,10 @@ public ResponseEntity<List<ModerateExamResponse>> getModeratingExams(@PathVariab
return ResponseEntity.ok(responses);
}

@PostMapping("/{candidateId}/{examId}/proctorComments")
public ResponseEntity<String> addComment(@PathVariable Long candidateId, @PathVariable Long examId, @RequestBody String content) {
examSetterService.addCommentToCandidate(candidateId, examId, content);
return ResponseEntity.ok("Comment added successfully");
}
Comment on lines +71 to +75
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance POST endpoint with proper request mapping and validation.

The endpoint needs:

  1. Proper @RequestBody mapping with a DTO
  2. Content-Type specification
  3. Input validation
  4. Better error handling

Apply these improvements:

     @PostMapping("/{candidateId}/{examId}/proctorComments")
+    @Consumes(MediaType.APPLICATION_JSON_VALUE)
-    public ResponseEntity<String> addComment(@PathVariable Long candidateId, @PathVariable Long examId, @RequestBody String content) {
+    public ResponseEntity<String> addComment(
+        @PathVariable Long candidateId,
+        @PathVariable Long examId,
+        @Valid @RequestBody ProctorCommentRequest request
+    ) {
+        if (candidateId == null || candidateId <= 0 || examId == null || examId <= 0) {
+            return ResponseEntity.badRequest().body("Invalid candidate or exam ID");
+        }
+        if (request.getContent() == null || request.getContent().trim().isEmpty()) {
+            return ResponseEntity.badRequest().body("Comment content cannot be empty");
+        }
-        examSetterService.addCommentToCandidate(candidateId, examId, content);
+        examSetterService.addCommentToCandidate(candidateId, examId, request.getContent());
         return ResponseEntity.ok("Comment added successfully");
     }

Create a new DTO:

@Data
public class ProctorCommentRequest {
    @NotBlank(message = "Comment content is required")
    private String content;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import lombok.NoArgsConstructor;
import lombok.experimental.SuperBuilder;

import java.util.List;
import java.util.Set;

@Entity
Expand All @@ -26,4 +27,7 @@ public class Candidate extends User{
@ManyToMany(mappedBy = "candidates")
private Set<Exam> exams;

@OneToMany(mappedBy = "candidate", cascade = CascadeType.ALL, orphanRemoval = true)
private List<ProctorComment> comments;

}
3 changes: 3 additions & 0 deletions src/main/java/com/testify/Testify_Backend/model/Exam.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,7 @@ public class Exam {
@Column(nullable = false)
private boolean hosted = false;

@OneToMany(mappedBy = "exam", cascade = CascadeType.ALL, orphanRemoval = true)
private List<ProctorComment> proctorComments;

Comment on lines +119 to +121
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider initializing the List and managing JSON serialization.

While the relationship mapping is correct, consider these improvements:

  1. Initialize the List to prevent potential NPEs
  2. Add @JsonIgnore to prevent infinite recursion during serialization
  3. Add a helper method to manage the bidirectional relationship

Apply this diff:

    @OneToMany(mappedBy = "exam", cascade = CascadeType.ALL, orphanRemoval = true)
+   @JsonIgnore
-   private List<ProctorComment> proctorComments;
+   private List<ProctorComment> proctorComments = new ArrayList<>();
+
+   public void addProctorComment(ProctorComment comment) {
+       proctorComments.add(comment);
+       comment.setExam(this);
+   }
+
+   public void removeProctorComment(ProctorComment comment) {
+       proctorComments.remove(comment);
+       comment.setExam(null);
+   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@OneToMany(mappedBy = "exam", cascade = CascadeType.ALL, orphanRemoval = true)
private List<ProctorComment> proctorComments;
@OneToMany(mappedBy = "exam", cascade = CascadeType.ALL, orphanRemoval = true)
@JsonIgnore
private List<ProctorComment> proctorComments = new ArrayList<>();
public void addProctorComment(ProctorComment comment) {
proctorComments.add(comment);
comment.setExam(this);
}
public void removeProctorComment(ProctorComment comment) {
proctorComments.remove(comment);
comment.setExam(null);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.testify.Testify_Backend.model;

import jakarta.persistence.*;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;

import java.time.LocalDateTime;

@Entity
@AllArgsConstructor
@NoArgsConstructor
@Getter
@Setter
public class ProctorComment {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@ManyToOne
@JoinColumn(name = "candidate_id", nullable = false)
private Candidate candidate;

@ManyToOne
@JoinColumn(name = "exam_id", nullable = false)
private Exam exam;

private String content;

private LocalDateTime createdAt;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.testify.Testify_Backend.model.Candidate;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;


import java.time.LocalDateTime;
Expand All @@ -22,5 +23,8 @@ public interface CandidateRepository extends JpaRepository<Candidate, Long> {
List<Candidate> findCandidatesAssignedToExamWithConflictingExams(Long examId, LocalDateTime startDatetime, LocalDateTime endDatetime);

boolean existsByEmail(String currentUserEmail);

@Query("SELECT c FROM Candidate c JOIN c.exams e WHERE e.id = :examId")
Set<Candidate> findByExamId(@Param("examId") Long examId);
}

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public interface ExamRepository extends JpaRepository<Exam, Long> {
@Query("SELECT e FROM Exam e WHERE e.isPrivate = false")
List<Exam> findAllPublicExams();

@Query("SELECT e FROM Exam e JOIN e.proctors p WHERE p.id = :proctorId AND e.organization.id = :organizationId")
List<Exam> findByProctorIdAndOrganizationId(@Param("proctorId") Long proctorId, @Param("organizationId") Long organizationId);


List<Exam> findByModeratorId(long moderatorId);
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.testify.Testify_Backend.repository;

import com.testify.Testify_Backend.model.ProctorComment;
import org.springframework.data.jpa.repository.JpaRepository;

public interface ProctorCommentRepository extends JpaRepository<ProctorComment, Long> {

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.testify.Testify_Backend.service;

import com.testify.Testify_Backend.responses.GenericAddOrUpdateResponse;
import com.testify.Testify_Backend.responses.exam_management.CandidateResponse;
import com.testify.Testify_Backend.responses.exam_management.ExamResponse;
import com.testify.Testify_Backend.responses.exam_management.OrganizationResponse;
import com.testify.Testify_Backend.responses.examsetter_management.ModerateExamResponse;

Expand All @@ -14,5 +16,10 @@ public interface ExamSetterService {

GenericAddOrUpdateResponse addSetterToOrganization(String token);

List<ExamResponse> getExamsForProctor(Long proctorId, Long organizationId);

Set<CandidateResponse> getCandidatesForExam(Long examId);
List<ModerateExamResponse> getModeratingExams(long examSetterId);

void addCommentToCandidate(Long candidateId, Long examId, String content);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package com.testify.Testify_Backend.service;

import com.testify.Testify_Backend.model.*;
import com.testify.Testify_Backend.repository.*;
import com.testify.Testify_Backend.responses.GenericAddOrUpdateResponse;
import com.testify.Testify_Backend.responses.exam_management.CandidateResponse;
import com.testify.Testify_Backend.responses.exam_management.ExamResponse;
import com.testify.Testify_Backend.responses.exam_management.OrganizationResponse;
import jakarta.transaction.Transactional;
import com.testify.Testify_Backend.model.ExamSetter;
import com.testify.Testify_Backend.model.ExamSetterInvitation;
import com.testify.Testify_Backend.model.Organization;
Expand All @@ -17,6 +24,8 @@
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;

import java.time.LocalDateTime;
import java.util.*;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
Expand All @@ -30,6 +39,8 @@ public class ExamSetterServiceImpl implements ExamSetterService {
private final ExamSetterInvitationRepository examSetterInvitationRepository;
private final OrganizationRepository organizationRepository;
private final ExamRepository examRepository;
private final CandidateRepository candidateRepository;
private final ProctorCommentRepository proctorCommentRepository;

@Autowired
private ModelMapper modelMapper;
Expand Down Expand Up @@ -77,6 +88,28 @@ public GenericAddOrUpdateResponse addSetterToOrganization(String token) {
return response;
}

@Override
@Transactional
public List<ExamResponse> getExamsForProctor(Long proctorId, Long organizationId) {
List<Exam> exams = examRepository.findByProctorIdAndOrganizationId(proctorId, organizationId); // Ensure repository method returns a List
List<ExamResponse> examResponses = new ArrayList<>();
for (Exam exam : exams) {
examResponses.add(modelMapper.map(exam, ExamResponse.class));
}
return examResponses;
}


@Override
public Set<CandidateResponse> getCandidatesForExam(Long examId) {
Set<Candidate> candidates = candidateRepository.findByExamId(examId);
Set<CandidateResponse> candidateResponses = new HashSet<>();
for (Candidate candidate : candidates) {
candidateResponses.add(modelMapper.map(candidate, CandidateResponse.class));
}

return candidateResponses;
}
public List<ModerateExamResponse> getModeratingExams(long examSetterId) {
return examRepository.findByModeratorId(examSetterId).stream()
.map(exam -> new ModerateExamResponse(
Expand All @@ -87,4 +120,19 @@ public List<ModerateExamResponse> getModeratingExams(long examSetterId) {
))
.collect(Collectors.toList());
}

@Override
public void addCommentToCandidate(Long candidateId, Long examId, String content) {
Candidate candidate = candidateRepository.findById(candidateId)
.orElseThrow(() -> new RuntimeException("Candidate not found"));
Exam exam = examRepository.findById(examId).orElseThrow(() -> new RuntimeException("Exam not found"));

ProctorComment comment = new ProctorComment();
comment.setCandidate(candidate);
comment.setExam(exam);
comment.setContent(content);
comment.setCreatedAt(LocalDateTime.now());

proctorCommentRepository.save(comment);
}
Comment on lines +125 to +137
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance exception handling and transaction management

  • Annotate the method with @Transactional to ensure atomicity of database operations.
  • Replace generic RuntimeException with more specific exceptions like EntityNotFoundException for clearer error handling.

Apply this diff:

+@Transactional
 public void addCommentToCandidate(Long candidateId, Long examId, String content) {
     Candidate candidate = candidateRepository.findById(candidateId)
-            .orElseThrow(() -> new RuntimeException("Candidate not found"));
+            .orElseThrow(() -> new EntityNotFoundException("Candidate not found with id: " + candidateId));
     Exam exam = examRepository.findById(examId)
-            .orElseThrow(() -> new RuntimeException("Exam not found"));
+            .orElseThrow(() -> new EntityNotFoundException("Exam not found with id: " + examId));

     ProctorComment comment = new ProctorComment();
     comment.setCandidate(candidate);
     comment.setExam(exam);
     comment.setContent(content);
     comment.setCreatedAt(LocalDateTime.now());

     proctorCommentRepository.save(comment);
 }

Ensure to import jakarta.persistence.EntityNotFoundException or define a custom exception class if appropriate.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void addCommentToCandidate(Long candidateId, Long examId, String content) {
Candidate candidate = candidateRepository.findById(candidateId)
.orElseThrow(() -> new RuntimeException("Candidate not found"));
Exam exam = examRepository.findById(examId).orElseThrow(() -> new RuntimeException("Exam not found"));
ProctorComment comment = new ProctorComment();
comment.setCandidate(candidate);
comment.setExam(exam);
comment.setContent(content);
comment.setCreatedAt(LocalDateTime.now());
proctorCommentRepository.save(comment);
}
@Transactional
public void addCommentToCandidate(Long candidateId, Long examId, String content) {
Candidate candidate = candidateRepository.findById(candidateId)
.orElseThrow(() -> new EntityNotFoundException("Candidate not found with id: " + candidateId));
Exam exam = examRepository.findById(examId)
.orElseThrow(() -> new EntityNotFoundException("Exam not found with id: " + examId));
ProctorComment comment = new ProctorComment();
comment.setCandidate(candidate);
comment.setExam(exam);
comment.setContent(content);
comment.setCreatedAt(LocalDateTime.now());
proctorCommentRepository.save(comment);
}

}
Loading