feat: #2357 create germination tray, api to assign germinator Id#2378
feat: #2357 create germination tray, api to assign germinator Id#2378
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements an API endpoint to assign a germinator ID to an existing germination tray, addressing issue #2357. The implementation adds a PATCH endpoint that updates the GERMINATOR_ID field in the CNS_T_GERMINATOR_TRAY table. However, the implementation deviates from established codebase conventions for PATCH endpoints, which will require refactoring before the PR can be merged.
Changes:
- Added PATCH endpoint
/api/germinator-trays/{germinatorTrayId}/germinator-idto assign germinator IDs - Created request and response DTOs for the germinator ID assignment operation
- Implemented service layer logic to update germinator tray entities
- Added comprehensive unit tests covering success and error scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| GerminatorTrayAssignGerminatorIdDto.java | Request DTO containing germinator tray ID and germinator ID (needs refactoring to remove tray ID) |
| GerminatorTrayAssignGerminatorIdResponseDto.java | Response DTO returning the assigned germinator tray ID and germinator ID |
| GerminatorTrayEndpoint.java | PATCH endpoint with path/body validation logic (needs refactoring to follow conventions) |
| TestResultService.java | Service method to persist germinator ID assignment (needs signature update) |
| GerminatorTrayEndpointTest.java | Unit tests for the new endpoint (needs refactoring and naming fixes) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param request contains the germinator tray ID and the germinator ID to assign | ||
| * @return a response DTO confirming the assignment | ||
| * @throws ResponseStatusException if the request is null or if the tray is not found | ||
| */ | ||
| public GerminatorTrayAssignGerminatorIdResponseDto assignGerminatorIdToTray( | ||
| GerminatorTrayAssignGerminatorIdDto request | ||
| ) { | ||
| if (request == null) { | ||
| throw new ResponseStatusException( | ||
| HttpStatus.BAD_REQUEST, | ||
| "Germinator tray assignment request cannot be null" | ||
| ); | ||
| } | ||
|
|
||
| SparLog.info( | ||
| "Assigning germinator ID {} to germinator tray ID {}", | ||
| request.germinatorId(), | ||
| request.germinatorTrayId() | ||
| ); | ||
|
|
||
| GerminatorTrayEntity tray = germinatorTrayRepository.findById(request.germinatorTrayId()) | ||
| .orElseThrow(() -> new ResponseStatusException( | ||
| HttpStatus.NOT_FOUND, | ||
| "Germinator tray not found with ID: " + request.germinatorTrayId() | ||
| )); | ||
|
|
||
| tray.setGerminatorId(request.germinatorId()); | ||
| germinatorTrayRepository.save(tray); | ||
|
|
||
| SparLog.info( | ||
| "Successfully assigned germinator ID {} to tray ID {}", | ||
| request.germinatorId(), | ||
| request.germinatorTrayId() | ||
| ); | ||
|
|
||
| return new GerminatorTrayAssignGerminatorIdResponseDto( | ||
| request.germinatorTrayId(), | ||
| request.germinatorId() |
There was a problem hiding this comment.
Once the API is refactored to follow the codebase convention (removing germinatorTrayId from the request DTO), this service method signature will need to be updated to accept the germinatorTrayId and germinatorId as separate parameters instead of as a single DTO. The method should be changed to accept Integer germinatorTrayId and String germinatorId directly.
| * @param request contains the germinator tray ID and the germinator ID to assign | |
| * @return a response DTO confirming the assignment | |
| * @throws ResponseStatusException if the request is null or if the tray is not found | |
| */ | |
| public GerminatorTrayAssignGerminatorIdResponseDto assignGerminatorIdToTray( | |
| GerminatorTrayAssignGerminatorIdDto request | |
| ) { | |
| if (request == null) { | |
| throw new ResponseStatusException( | |
| HttpStatus.BAD_REQUEST, | |
| "Germinator tray assignment request cannot be null" | |
| ); | |
| } | |
| SparLog.info( | |
| "Assigning germinator ID {} to germinator tray ID {}", | |
| request.germinatorId(), | |
| request.germinatorTrayId() | |
| ); | |
| GerminatorTrayEntity tray = germinatorTrayRepository.findById(request.germinatorTrayId()) | |
| .orElseThrow(() -> new ResponseStatusException( | |
| HttpStatus.NOT_FOUND, | |
| "Germinator tray not found with ID: " + request.germinatorTrayId() | |
| )); | |
| tray.setGerminatorId(request.germinatorId()); | |
| germinatorTrayRepository.save(tray); | |
| SparLog.info( | |
| "Successfully assigned germinator ID {} to tray ID {}", | |
| request.germinatorId(), | |
| request.germinatorTrayId() | |
| ); | |
| return new GerminatorTrayAssignGerminatorIdResponseDto( | |
| request.germinatorTrayId(), | |
| request.germinatorId() | |
| * @param germinatorTrayId the ID of the germinator tray to assign to | |
| * @param germinatorId the germinator ID to assign to the tray | |
| * @return a response DTO confirming the assignment | |
| * @throws ResponseStatusException if any parameter is null or if the tray is not found | |
| */ | |
| public GerminatorTrayAssignGerminatorIdResponseDto assignGerminatorIdToTray( | |
| Integer germinatorTrayId, | |
| String germinatorId | |
| ) { | |
| if (germinatorTrayId == null) { | |
| throw new ResponseStatusException( | |
| HttpStatus.BAD_REQUEST, | |
| "Germinator tray ID cannot be null" | |
| ); | |
| } | |
| if (germinatorId == null) { | |
| throw new ResponseStatusException( | |
| HttpStatus.BAD_REQUEST, | |
| "Germinator ID cannot be null" | |
| ); | |
| } | |
| SparLog.info( | |
| "Assigning germinator ID {} to germinator tray ID {}", | |
| germinatorId, | |
| germinatorTrayId | |
| ); | |
| GerminatorTrayEntity tray = germinatorTrayRepository.findById(germinatorTrayId) | |
| .orElseThrow(() -> new ResponseStatusException( | |
| HttpStatus.NOT_FOUND, | |
| "Germinator tray not found with ID: " + germinatorTrayId | |
| )); | |
| tray.setGerminatorId(germinatorId); | |
| germinatorTrayRepository.save(tray); | |
| SparLog.info( | |
| "Successfully assigned germinator ID {} to tray ID {}", | |
| germinatorId, | |
| germinatorTrayId | |
| ); | |
| return new GerminatorTrayAssignGerminatorIdResponseDto( | |
| germinatorTrayId, | |
| germinatorId |
| if (request == null) { | ||
| throw new ResponseStatusException( | ||
| HttpStatus.BAD_REQUEST, | ||
| "Germinator tray assignment request cannot be null" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Once the API is refactored to follow the codebase convention, this null check should be updated. Instead of checking if the request DTO is null, the method should validate the individual parameters (germinatorTrayId and germinatorId) or rely on Spring's validation annotations at the controller level. Similar to other service methods in the codebase, consider using @NonNull annotations on the parameters.
| @Test | ||
| void assignGerminatorIdToTray_returns200AndBody_andCallsService() throws Exception { | ||
| // Arrange | ||
| Integer germinatorTrayId = 101; | ||
| String germinatorId = "A"; | ||
| GerminatorTrayAssignGerminatorIdDto request = | ||
| new GerminatorTrayAssignGerminatorIdDto(germinatorTrayId, germinatorId); | ||
|
|
||
| GerminatorTrayAssignGerminatorIdResponseDto response = | ||
| new GerminatorTrayAssignGerminatorIdResponseDto( | ||
| germinatorTrayId, | ||
| germinatorId | ||
| ); | ||
|
|
||
| when(testResultService.assignGerminatorIdToTray(any())).thenReturn(response); |
There was a problem hiding this comment.
Once the API is refactored to follow the codebase convention (removing germinatorTrayId from the request DTO), this test will need to be updated. The request object should only contain the germinatorId, not the germinatorTrayId. The service method will also need to be updated to accept separate parameters.
| @Test | ||
| void assignGerminatorIdToTray_returns404_whenTrayNotFound() throws Exception { | ||
| // Arrange | ||
| Integer germinatorTrayId = 999; | ||
| String germinatorId = "A"; | ||
| GerminatorTrayAssignGerminatorIdDto request = | ||
| new GerminatorTrayAssignGerminatorIdDto(germinatorTrayId, germinatorId); | ||
|
|
||
| when(testResultService.assignGerminatorIdToTray(any())) | ||
| .thenThrow(new ResponseStatusException( | ||
| HttpStatus.NOT_FOUND, | ||
| "Germinator tray not found with ID: " + germinatorTrayId | ||
| )); | ||
|
|
||
| // Act / Assert | ||
| mockMvc.perform(patch(BASE_URL + "/" + germinatorTrayId + "/germinator-id") | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isNotFound()); | ||
|
|
||
| // Verify service was called | ||
| verify(testResultService, times(1)).assignGerminatorIdToTray(request); | ||
| } |
There was a problem hiding this comment.
Once the API is refactored to follow the codebase convention (removing germinatorTrayId from the request DTO), this test will need to be updated. The request object should only contain the germinatorId, not the germinatorTrayId.
| @Test | ||
| void whenGerminatorIdBlank() throws Exception { | ||
| // Arrange - empty germinator ID | ||
| Integer germinatorTrayId = 101; | ||
| GerminatorTrayAssignGerminatorIdDto request = | ||
| new GerminatorTrayAssignGerminatorIdDto(germinatorTrayId, ""); | ||
|
|
||
| // Act / Assert | ||
| mockMvc.perform(patch(BASE_URL + "/" + germinatorTrayId + "/germinator-id") | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isBadRequest()); | ||
|
|
||
| // Verify service was NOT called because validation failed | ||
| verify(testResultService, times(0)).assignGerminatorIdToTray(any()); | ||
| } |
There was a problem hiding this comment.
Once the API is refactored to follow the codebase convention (removing germinatorTrayId from the request DTO), this test will need to be updated. The request object should only contain the germinatorId, not the germinatorTrayId.
| HttpStatus.BAD_REQUEST, | ||
| "Germinator tray ID in path does not match the ID in request body" | ||
| ); | ||
| } return testResultService.assignGerminatorIdToTray(request); |
There was a problem hiding this comment.
The return statement should be on a new line after the closing brace. This appears to be a formatting error where the return statement is incorrectly placed on the same line as the closing brace of the if block.
| } return testResultService.assignGerminatorIdToTray(request); | |
| } | |
| return testResultService.assignGerminatorIdToTray(request); |
| } | ||
|
|
||
| @Test | ||
| void whenGerminatorIdBlank() throws Exception { |
There was a problem hiding this comment.
Test method name does not follow the established naming convention in this file. Other test methods follow the pattern "methodName_expectedBehavior_condition" (e.g., "assignGerminatorIdToTray_returns400_whenPathAndBodyIdMismatch"). This test should be renamed to follow the same pattern, such as "assignGerminatorIdToTray_returns400_whenGerminatorIdBlank".
| void whenGerminatorIdBlank() throws Exception { | |
| void assignGerminatorIdToTray_returns400_whenGerminatorIdBlank() throws Exception { |
| void assignGerminatorIdToTray_returns400_whenPathAndBodyIdMismatch() throws Exception { | ||
| // Arrange - path has ID 101 but body has ID 102 | ||
| Integer pathTrayId = 101; | ||
| Integer bodyTrayId = 102; | ||
| String germinatorId = "A"; | ||
| GerminatorTrayAssignGerminatorIdDto request = | ||
| new GerminatorTrayAssignGerminatorIdDto(bodyTrayId, germinatorId); | ||
|
|
||
| // Act / Assert | ||
| mockMvc.perform(patch(BASE_URL + "/" + pathTrayId + "/germinator-id") | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isBadRequest()); | ||
|
|
||
| // Verify service was NOT called because validation failed | ||
| verify(testResultService, times(0)).assignGerminatorIdToTray(any()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
This test validates logic that should not exist in the endpoint. Following the codebase's PATCH endpoint conventions, the germinatorTrayId should not be in the request body, making this path/body mismatch validation unnecessary. Once the API is refactored to follow the convention, this test should be removed.
| void assignGerminatorIdToTray_returns400_whenPathAndBodyIdMismatch() throws Exception { | |
| // Arrange - path has ID 101 but body has ID 102 | |
| Integer pathTrayId = 101; | |
| Integer bodyTrayId = 102; | |
| String germinatorId = "A"; | |
| GerminatorTrayAssignGerminatorIdDto request = | |
| new GerminatorTrayAssignGerminatorIdDto(bodyTrayId, germinatorId); | |
| // Act / Assert | |
| mockMvc.perform(patch(BASE_URL + "/" + pathTrayId + "/germinator-id") | |
| .with(csrf()) | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .content(objectMapper.writeValueAsString(request))) | |
| .andExpect(status().isBadRequest()); | |
| // Verify service was NOT called because validation failed | |
| verify(testResultService, times(0)).assignGerminatorIdToTray(any()); | |
| } | |
| @Test |
| /** | ||
| * Assign a germinator ID to an existing germinator tray. | ||
| * | ||
| * @param request contains the germinator tray ID and the germinator ID to assign | ||
| * @return a response DTO confirming the assignment | ||
| * @throws ResponseStatusException if the request is null or if the tray is not found |
There was a problem hiding this comment.
This JavaDoc will need to be updated once the API is refactored to follow the codebase convention. The @param description should reflect that germinatorTrayId and germinatorId are separate parameters rather than being contained in a single request DTO.
| public record GerminatorTrayAssignGerminatorIdDto( | ||
| @Schema(description = "Primary key of the germinator tray", example = "101") | ||
| @NotNull | ||
| Integer germinatorTrayId, |
There was a problem hiding this comment.
The request DTO should not include the germinatorTrayId field. In this codebase, PATCH endpoints follow the convention of passing the resource ID only as a path variable, not in the request body. This is consistent with other PATCH endpoints like MoistureContentConesEndpoint and PurityTestsEndpoint, where the resource ID is provided via @PathVariable and the request DTO (FormDto) contains only the fields to be updated. Remove the germinatorTrayId field from this DTO.
Description
Closes #2357
Changelog
New
Changed
Removed
How was this tested?
What gif/image best describes this PR or how it makes you feel?
Thanks for the PR!
Deployments, as required, will be available below:
Please create PRs in draft mode. Mark as ready to enable:
After merge, new images are deployed in: