[Feat/user-feedback] - 피드백 저장 API #112
Conversation
|
Warning Rate limit exceeded@ekgns33 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughA complete feedback feature was added, including REST API endpoints, domain model, service and repository layers, database migrations, and acceptance testing. The implementation includes DTOs, command objects, a JPA entity, repository, use case interface and implementation, controller, response codes, and supporting test data and cleanup utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FeedbackController
participant FeedbackUsecase
participant FeedbackRepository
participant Database
Client->>FeedbackController: POST /api/v1/feedback (FeedbackRequest, userId)
FeedbackController->>FeedbackUsecase: createFeedback(FeedbackCommand)
FeedbackUsecase->>FeedbackRepository: save(Feedback)
FeedbackRepository->>Database: Insert Feedback row
FeedbackRepository-->>FeedbackUsecase: Saved Feedback
FeedbackUsecase-->>FeedbackController: (void)
FeedbackController-->>Client: 201 Created (SuccessResponse)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
src/test/java/org/runimo/runimo/CleanUpUtil.java (1)
27-28: Reorder deletes to satisfy foreign-key constraintsThe hard-coded USER_TABLES list already serves as a whitelist (so SQL-injection isn’t a concern), but deleting
usersbefore its child tables (e.g.user_token,oauth_account) will violate FK rules. Either move"users"to the end of the array or switch to a CASCADE truncate.• File: src/test/java/org/runimo/runimo/CleanUpUtil.java
• Lines 10–20: update USER_TABLES order
• Lines 27–28: (optional) replaceDELETE … ALTER TABLEwith
jdbcTemplate.execute("TRUNCATE TABLE " + table + " RESTART IDENTITY CASCADE");Suggested diff:
@@ private static final String[] USER_TABLES = { - "user_item", - "oauth_account", - "users", - "running_record", - "user_love_point", - "incubating_egg", - "runimo", - "user_token", - "user_feedback" + "user_item", + "oauth_account", + "user_token", + "running_record", + "user_love_point", + "incubating_egg", + "runimo", + "user_feedback", + "users" // parent table last to avoid FK violations }
🧹 Nitpick comments (7)
src/test/resources/sql/user_default_data.sql (2)
7-8: Hard-coded test data may cause brittleness in tests.Consider using more flexible approaches like test data builders or factories to generate test data, especially for values like
public_idandimg_urlthat might need to be unique across different test scenarios.
29-32: Remove unnecessary trailing empty lines.Clean up the file by removing the trailing empty lines to improve code quality.
- - -src/main/java/org/runimo/runimo/user/service/usecases/FeedbackUsecase.java (1)
7-7: Consider returning a result object instead of void.While the current void return type works for fire-and-forget operations, consider returning a result object (e.g.,
FeedbackResultorCreatedFeedback) to provide better error handling and allow callers to access the created feedback's ID or other metadata.- void createFeedback(FeedbackCommand command); + FeedbackResult createFeedback(FeedbackCommand command);This would align with more robust error handling patterns and provide better integration testing capabilities.
src/test/java/org/runimo/runimo/user/api/FeedbackAcceptanceTest.java (1)
65-69: Clean up trailing whitespace and empty lines.Remove unnecessary empty lines at the end of the file for better code cleanliness.
.statusCode(HttpStatus.CREATED.value()); - } -src/main/java/org/runimo/runimo/user/controller/FeedbackController.java (2)
28-28: Consider refactoring the static toCommand method design.Using a static method on the request DTO for conversion might not be the best design. Consider moving this logic to a dedicated mapper class or making it an instance method for better separation of concerns.
Alternative approach:
-feedbackUsecase.createFeedback(FeedbackRequest.toCommand(userId, request)); +feedbackUsecase.createFeedback(new FeedbackCommand(userId, request.rate(), request.feedback()));Or create a dedicated mapper class:
@Component public class FeedbackMapper { public FeedbackCommand toCommand(Long userId, FeedbackRequest request) { return new FeedbackCommand(userId, request.rate(), request.feedback()); } }
24-31: Consider adding input validation for userId.While the request body is validated with
@Valid, there's no explicit validation for theuserIdparameter. Consider adding validation to ensure the user ID is not null or invalid.public ResponseEntity<SuccessResponse<Void>> createFeedback( - @UserId Long userId, + @UserId @NotNull Long userId, @Valid @RequestBody FeedbackRequest request ) {src/main/java/org/runimo/runimo/user/controller/request/FeedbackRequest.java (1)
20-22: Consider improving the toCommand method design.The static method approach for conversion might not be the best design pattern. Consider using a dedicated mapper class or making this an instance method for better maintainability and testability.
Alternative approach as an instance method:
-public static FeedbackCommand toCommand(Long userId, FeedbackRequest request) { - return new FeedbackCommand(userId, request.rate(), request.feedback()); -} +public FeedbackCommand toCommand(Long userId) { + return new FeedbackCommand(userId, this.rate, this.feedback); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/org/runimo/runimo/user/controller/FeedbackController.java(1 hunks)src/main/java/org/runimo/runimo/user/controller/request/FeedbackRequest.java(1 hunks)src/main/java/org/runimo/runimo/user/domain/Feedback.java(1 hunks)src/main/java/org/runimo/runimo/user/enums/UserHttpResponseCode.java(1 hunks)src/main/java/org/runimo/runimo/user/repository/FeedbackRepository.java(1 hunks)src/main/java/org/runimo/runimo/user/service/dto/command/FeedbackCommand.java(1 hunks)src/main/java/org/runimo/runimo/user/service/usecases/FeedbackUsecase.java(1 hunks)src/main/java/org/runimo/runimo/user/service/usecases/FeedbackUsecaseImpl.java(1 hunks)src/main/resources/db/migration/V20250710__add_feedback_table.sql(1 hunks)src/test/java/org/runimo/runimo/CleanUpUtil.java(1 hunks)src/test/java/org/runimo/runimo/user/api/FeedbackAcceptanceTest.java(1 hunks)src/test/resources/db/migration/h2/V20250710__add_feedback_table.sql(1 hunks)src/test/resources/sql/user_default_data.sql(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/org/runimo/runimo/user/api/FeedbackAcceptanceTest.java (1)
src/test/java/org/runimo/runimo/TestConsts.java (1)
TestConsts(3-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and analyze
🔇 Additional comments (5)
src/test/resources/sql/user_default_data.sql (1)
2-2: Consider the security implications of disabling foreign key checks.While disabling foreign key checks is common in test environments, ensure this script is never used in production environments where data integrity is critical.
src/test/java/org/runimo/runimo/CleanUpUtil.java (1)
18-20: LGTM! Proper addition of user_feedback table to cleanup routine.The addition of the
user_feedbacktable to the cleanup routine ensures test isolation and aligns with the new feedback feature.src/main/java/org/runimo/runimo/user/enums/UserHttpResponseCode.java (1)
19-19: LGTM! Appropriate HTTP status code and message for feedback creation.The addition of
FEEDBACK_CREATEDwithHttpStatus.CREATED(201) is semantically correct for resource creation, and the Korean messages are consistent with the existing codebase patterns.src/main/java/org/runimo/runimo/user/repository/FeedbackRepository.java (1)
6-8: LGTM! Clean and standard Spring Data JPA repository interface.The interface follows Spring Data JPA conventions correctly, extending
JpaRepository<Feedback, Long>to provide standard CRUD operations for the Feedback entity.src/test/resources/db/migration/h2/V20250710__add_feedback_table.sql (1)
1-9: Add foreign key, rate check, and indexes to user_feedback (verify users table name)The
user_feedbacktable needs data-integrity and performance improvements:
- Confirm the actual users table name and its schema—no
CREATE TABLE userorusersentry was found insrc/main/resources/db/migration.- Add a rate constraint:
ALTER TABLE user_feedback MODIFY COLUMN rate INT CHECK (rate BETWEEN 1 AND 5);- Add indexes for common lookups:
CREATE INDEX idx_user_feedback_user_id ON user_feedback(user_id); CREATE INDEX idx_user_feedback_created_at ON user_feedback(created_at);- Once you’ve confirmed the target users table, add the FK:
ALTER TABLE user_feedback ADD CONSTRAINT fk_user_feedback_user_id FOREIGN KEY (user_id) REFERENCES <users_table_name>(id);
| public record FeedbackCommand( | ||
| Long userId, | ||
| Integer rate, | ||
| String content | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation annotations to ensure data integrity.
The record lacks validation constraints which could allow invalid data to propagate through the system. Consider adding validation annotations:
+import jakarta.validation.constraints.Max;
+import jakarta.validation.constraints.Min;
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Size;
public record FeedbackCommand(
- Long userId,
- Integer rate,
- String content
+ @NotNull Long userId,
+ @NotNull @Min(1) @Max(5) Integer rate,
+ @NotBlank @Size(max = 128) String content
) {📝 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.
| public record FeedbackCommand( | |
| Long userId, | |
| Integer rate, | |
| String content | |
| ) { | |
| import jakarta.validation.constraints.Max; | |
| import jakarta.validation.constraints.Min; | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| import jakarta.validation.constraints.Size; | |
| public record FeedbackCommand( | |
| @NotNull Long userId, | |
| @NotNull @Min(1) @Max(5) Integer rate, | |
| @NotBlank @Size(max = 128) String content | |
| ) { | |
| // other methods or logic, if any | |
| } |
🤖 Prompt for AI Agents
In src/main/java/org/runimo/runimo/user/service/dto/command/FeedbackCommand.java
around lines 3 to 7, the FeedbackCommand record lacks validation annotations.
Add appropriate validation annotations such as @NotNull for userId and rate,
@Min and @Max for rate to restrict its range, and @NotBlank for content to
ensure it is not empty. This will enforce data integrity and prevent invalid
data from being processed.
| @Override | ||
| @Transactional | ||
| public void createFeedback(FeedbackCommand command) { | ||
| Feedback feedback = Feedback.builder() | ||
| .userId(command.userId()) | ||
| .rate(command.rate()) | ||
| .content(command.content()) | ||
| .build(); | ||
| feedbackRepository.save(feedback); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and error handling.
The method lacks validation and error handling which could lead to persistence of invalid data or unclear error messages:
@Override
@Transactional
public void createFeedback(FeedbackCommand command) {
+ // Validate command (or use @Valid parameter)
+ if (command.userId() == null || command.rate() == null || command.content() == null) {
+ throw new IllegalArgumentException("All feedback fields are required");
+ }
+
+ if (command.rate() < 1 || command.rate() > 5) {
+ throw new IllegalArgumentException("Rate must be between 1 and 5");
+ }
+
+ if (command.content().trim().isEmpty() || command.content().length() > 128) {
+ throw new IllegalArgumentException("Content must not be empty and max 128 characters");
+ }
+
Feedback feedback = Feedback.builder()
.userId(command.userId())
.rate(command.rate())
- .content(command.content())
+ .content(command.content().trim())
.build();
+ try {
feedbackRepository.save(feedback);
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to save feedback", e);
+ }
}📝 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.
| @Override | |
| @Transactional | |
| public void createFeedback(FeedbackCommand command) { | |
| Feedback feedback = Feedback.builder() | |
| .userId(command.userId()) | |
| .rate(command.rate()) | |
| .content(command.content()) | |
| .build(); | |
| feedbackRepository.save(feedback); | |
| } | |
| @Override | |
| @Transactional | |
| public void createFeedback(FeedbackCommand command) { | |
| // Validate command (or use @Valid parameter) | |
| if (command.userId() == null || command.rate() == null || command.content() == null) { | |
| throw new IllegalArgumentException("All feedback fields are required"); | |
| } | |
| if (command.rate() < 1 || command.rate() > 5) { | |
| throw new IllegalArgumentException("Rate must be between 1 and 5"); | |
| } | |
| if (command.content().trim().isEmpty() || command.content().length() > 128) { | |
| throw new IllegalArgumentException("Content must not be empty and max 128 characters"); | |
| } | |
| Feedback feedback = Feedback.builder() | |
| .userId(command.userId()) | |
| .rate(command.rate()) | |
| .content(command.content().trim()) | |
| .build(); | |
| try { | |
| feedbackRepository.save(feedback); | |
| } catch (Exception e) { | |
| throw new RuntimeException("Failed to save feedback", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/runimo/runimo/user/service/usecases/FeedbackUsecaseImpl.java
around lines 16 to 25, the createFeedback method lacks input validation and
error handling. Add checks to validate the command's userId, rate, and content
before building the Feedback object, throwing appropriate exceptions if
validation fails. Wrap the save operation in a try-catch block to handle
persistence exceptions and provide clear error messages or rethrow as needed.
src/main/resources/db/migration/V20250710__add_feedback_table.sql
Outdated
Show resolved
Hide resolved
| @Length(max = 100) | ||
| String feedback |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null validation for feedback field.
The feedback field can currently be null, which might not be intended. Consider adding @NotNull or @NotBlank validation if feedback is required.
@Schema(description = "피드백 내용", example = "피드백 내용")
+@NotBlank
@Length(max = 100)
String feedback📝 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.
| @Length(max = 100) | |
| String feedback | |
| @Schema(description = "피드백 내용", example = "피드백 내용") | |
| @NotBlank | |
| @Length(max = 100) | |
| String feedback |
🤖 Prompt for AI Agents
In src/main/java/org/runimo/runimo/user/controller/request/FeedbackRequest.java
around lines 16 to 17, the feedback field lacks null validation, allowing it to
be null which may be unintended. Add the @NotNull or @NotBlank annotation above
the feedback field declaration to enforce that feedback must not be null or
blank, ensuring proper validation of this input.
| @Min(1) @Max(6) | ||
| Integer rate, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation context and constraints.
The rate field allows values 1-6 but lacks documentation about what these values represent. Consider adding more descriptive documentation or constants for the rating scale.
+public static final int MIN_RATE = 1;
+public static final int MAX_RATE = 6;
+
-@Schema(description = "평가지표", example = "1")
+@Schema(description = "평가지표 (1: 매우 불만족, 6: 매우 만족)", example = "3")
-@Min(1) @Max(6)
+@Min(MIN_RATE) @Max(MAX_RATE)
Integer rate,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/org/runimo/runimo/user/controller/request/FeedbackRequest.java
around lines 13 to 14, the rate field has @Min(1) and @Max(6) annotations but
lacks descriptive documentation or constants explaining what the values 1 to 6
represent. Add JavaDoc comments to the rate field describing the meaning of each
rating value or define constants/enums representing the rating scale to improve
code clarity and maintainability.
|
작업내역
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores