Skip to content

Conversation

@tobyev
Copy link
Contributor

@tobyev tobyev commented Oct 6, 2025

I completed my Lesson 23 homework by wiring the REST controller to the Library API and ensuring IDs are UUID s. I updated MediaItemsController.java to use addMediaItem(item, Librarian) and removeMediaItem (UUID, Librarian), and updated AddMediaItemRequest.java to construct items with UUID.fromString(id) . All tests pass (52/52).

I encountered some small challenges, but worked through them and confirmed my changes were staged, committed, pushed, and ready for review.

Problem area(s):
• Initially called non-existent methods on Library (add (MediaItem), remove(String))
• Forgot that deletion requires a UUID and a Librarian argument
• Needed to convert request path id: String to UUID. fromString(id) in the DTO
• Minor path confusion when running gradlew from the repo root

Lessons learned:
• Verify real method names in target classes before wiring controllers
• Convert external/string IDs to domain types ( UUID ) at the API boundary
• Provide required collaborators (e.g., Librarian ) for mutating operations
• Use git status, git diff, and running the test suite to validate changes end-to-end

Copy link
Contributor

@anthonydmays anthonydmays left a comment

Choose a reason for hiding this comment

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

🎓 Automated Grading Report

Student: Tobais Evans
Date: 10/7/2025

Feedback

The submission demonstrates a good understanding of the requirements for modifying the MediaItemsController.java file to handle API requests. The student has successfully implemented the GET, POST, and DELETE endpoints with appropriate handling of media items using the library's search and manipulation methods. However, there are some deviations from the sample solution that impact the functional and technical aspects. For instance, the librarian field was removed and replaced with a new Librarian instance in each method, which may not align with the intended design of using a consistent librarian from the library. Additionally, the response structure for the GET and POST requests has been altered to use a Map instead of the response classes shown in the sample solution. While this approach works, it deviates from the expected design pattern and could lead to inconsistencies in the API response format. The code is generally well-formatted and syntactically correct, but the changes in the response structure and librarian handling suggest a partial understanding of the design intentions. Overall, the submission meets many of the requirements but could benefit from closer adherence to the provided sample solution.


This is an automated preliminary review. Please review and adjust before finalizing.

@anthonydmays anthonydmays reopened this Oct 8, 2025
Copy link
Contributor

@anthonydmays anthonydmays left a comment

Choose a reason for hiding this comment

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

Checks are failing, please fix.

@PostMapping("/items")
public ResponseEntity<Map<String, MediaItemResponse>> addItem(@Valid @RequestBody AddMediaItemRequest request) {
MediaItem item = request.getItem().toDomain();
library.addMediaItem(item, new Librarian("System", "[email protected]"));
Copy link
Contributor

Choose a reason for hiding this comment

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

There's already a librarian instance in the class you can (and should) use.

}

@PostMapping("/items")
public ResponseEntity<Map<String, MediaItemResponse>> addItem(@Valid @RequestBody AddMediaItemRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the existing CreateMediaItemRequest and CreateMediaItemResponse types.

}
MediaItem item = items.iterator().next();
var body = Collections.singletonMap("item", MediaItemResponse.from(item));
return ResponseEntity.ok(body);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should just return MediaItemResponse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants