-
Notifications
You must be signed in to change notification settings - Fork 13
Feat/281 Controlador y endpoint completo para ver recursos de un reto #852
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
base: develop
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR small and clear. Some changes requested over the endpoint mapping. I was reviewing code that was already in a previous PR and even though it should not happen, I am not sure if it could have been avoided since it's from the same endpoint.
@@ -41,5 +45,22 @@ public Mono<ResponseEntity<ResourceDto>> createNewResource(@RequestBody @Valid R | |||
return resourceService.createResource(resourceDto) | |||
.map(createdResource -> ResponseEntity.ok().body(createdResource)); | |||
} | |||
|
|||
@GetMapping("/resources") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is misleading because "/resources" alone it would normally indicate that the endpoint retrieves All the resources in the database.
I'd rather use GET /resources/challenge/{challengeId} or something in the line to explicitly clarify it is using a challenge Id. Also because later we may want to create another endpoint to retrieve a resource by its Id alone, and like as it is now it would become too confusing.
} | ||
) | ||
public Flux<ResourceDto> getResourcesByChallengeId( | ||
@RequestParam @NotNull UUID challengeId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line with the @GetMapping comment above, I'd rather use @PathVariable instead of @RequestParam to follow the convention (you can check ChallengeController to see how it is done there)
description = "Retrieve all resources associated with a specific challenge.", | ||
responses = { | ||
@ApiResponse(responseCode = "200", description = "Resources found", content = @Content(schema = @Schema(implementation = ResourceDto.class))), | ||
@ApiResponse(responseCode = "400", description = "Invalid challenge ID"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing code 404 maybe? Would be really useful
public Flux<ResourceDto> getResourcesByChallengeId( | ||
@RequestParam @NotNull UUID challengeId | ||
) { | ||
log.info("Fetching resources for challenge ID: {}", challengeId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one
...llenge-challenge/src/main/java/com/itachallenge/challenge/repository/ResourceRepository.java
Show resolved
Hide resolved
…eption Handler y corregido la respuesta 404 en los controladores.
Implement GET Endpoint for Challenge Resources
Overview:
This PR implements the endpoint GET /itachallenge/api/v1/resource?challengeId={id} that allows students to view all learning resources associated with a specific challenge.
This completes feature #279: "Como alumno, puedo ver los recursos de un reto".
Technical Implementation
Endpoint Details:
Path: GET /itachallenge/api/v1/resource
Query Parameter:
challengeId (required, UUID): ID of the challenge whose resources are being requested.
🔸 Responses
200 OK: Returns an array of ResourceDto objects.
400 Bad Request: Returned when challengeId is missing or invalid.
500 Internal Server Error: For unexpected failures.
🔗 Resource–Challenge Relationship
Resources are associated with challenges in two ways:
Direct Association: Each resource contains a challengeIds array field with UUIDs of related challenges.
Repository Query: The method findByChallengeIdsContaining() efficiently fetches resources from MongoDB where the given challengeId appears in their challengeIds array.
🔄 Flow Summary
📍 Controller
Validates request parameters.
Logs the operation.
Delegates processing to the service layer.
Returns a reactive Flux.
🧠 Service Layer
Performs null validation.
Queries MongoDB via reactive repository.
Maps documents to DTOs.
Handles errors and logs them.
Maintains a fully reactive pipeline.
Tras el turbo commit con las revisiones aplicadas:
Fix in the Global Exception Handler: The handling of ResourceNotFoundException was modified. The previous exception returned a 200 status code with the error message, which was incorrect. Now, it returns a 404 (HttpStatus.NOT_FOUND) status code with the corresponding error message.
Controllers: The resource controller now correctly handles the ResourceNotFoundException to return a 404 when no resources are found for a challengeId.
Tests: The tests were adjusted to correctly validate responses with 404 status codes when resources are not found, as well as ensuring proper error propagation.
Updated the controller to work with @PathVariable
Updated the route "/challenge/{challengeId}"