Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @jbh010204, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented the functionality for users to view their own 1:1 inquiry history. This involved adding a new service method and repository query to fetch user-specific inquiries. Additionally, I've refactored the existing inquiry controllers by extracting all administrator-specific inquiry management endpoints into a new dedicated AdminInquiryController, ensuring a clearer separation of concerns between user and admin functionalities.
Highlights
- User-Specific Inquiry History: I've implemented a new API endpoint and the necessary backend logic to enable authenticated users to retrieve a paginated list of their own 1:1 inquiries, fulfilling the core requirement of viewing personal inquiry history.
- Separation of Admin and User Concerns: All administrative operations related to 1:1 inquiries, such as listing all inquiries and managing answers (creating, updating, deleting), have been extracted from the general
InquiryControllerand moved into a newly createdAdminInquiryController. This improves modularity and clarifies responsibility. - Enhanced Inquiry Detail Security: The
getInquiryDetailmethod in theInquiryServicenow includes a permission check, ensuring that only the owner of an inquiry or an authorized administrator can view its detailed information. - QueryDSL Utility Refactoring: The
getTotalCountmethod within the QueryDSL repository implementation has been refactored to be more generic and reusable, allowing it to calculate total counts for various QueryDSL queries more efficiently.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a feature for users to view their own inquiries and refactors admin-related inquiry endpoints into a separate controller. The changes are a good step forward, but I've identified several critical issues. There are significant bugs in the data access layer where pagination queries calculate total counts incorrectly, which will break pagination. Additionally, there's a critical authorization flaw in how inquiry details are fetched. I've also included some suggestions to improve the clarity of the API documentation.
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryService.java
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/infra/persistence/jpa/InquiryJpaRepositoryImpl.java
Outdated
Show resolved
Hide resolved
| import life.mosu.mosuserver.domain.inquiry.entity.InquiryStatus; | ||
| import life.mosu.mosuserver.global.util.ApiResponseWrapper; | ||
| import life.mosu.mosuserver.presentation.admin.docs.AdminInquiryControllerDocs; | ||
| import life.mosu.mosuserver.presentation.inquiry.InquiryController; |
| ResponseEntity<ApiResponseWrapper<Void>> create( | ||
| @AuthenticationPrincipal PrincipalDetails principalDetails, | ||
| PrincipalDetails principalDetails, | ||
| @Parameter(description = "문의 생성에 필요한 정보") @RequestBody @Valid InquiryCreateRequest request | ||
| ); |
There was a problem hiding this comment.
The PrincipalDetails parameter is resolved from the security context and not provided by the client in the request. To prevent it from appearing as a request body in the Swagger UI, you should annotate it with @Parameter(hidden = true).
| ResponseEntity<ApiResponseWrapper<Void>> create( | |
| @AuthenticationPrincipal PrincipalDetails principalDetails, | |
| PrincipalDetails principalDetails, | |
| @Parameter(description = "문의 생성에 필요한 정보") @RequestBody @Valid InquiryCreateRequest request | |
| ); | |
| ResponseEntity<ApiResponseWrapper<Void>> create( | |
| @Parameter(hidden = true) PrincipalDetails principalDetails, | |
| @Parameter(description = "문의 생성에 필요한 정보") @RequestBody @Valid InquiryCreateRequest request | |
| ); |
src/main/java/life/mosu/mosuserver/presentation/inquiry/InquiryControllerDocs.java
Show resolved
Hide resolved
| ResponseEntity<ApiResponseWrapper<InquiryDetailResponse>> getInquiryDetail( | ||
| PrincipalDetails principalDetails, | ||
| @Parameter(name = "postId", description = "조회할 문의의 ID", in = ParameterIn.PATH) | ||
| @PathVariable Long postId | ||
| ); |
There was a problem hiding this comment.
The PrincipalDetails parameter is resolved from the security context and is not part of the request from the client. It should be hidden from the Swagger documentation to avoid confusion for API consumers.
| ResponseEntity<ApiResponseWrapper<InquiryDetailResponse>> getInquiryDetail( | |
| PrincipalDetails principalDetails, | |
| @Parameter(name = "postId", description = "조회할 문의의 ID", in = ParameterIn.PATH) | |
| @PathVariable Long postId | |
| ); | |
| ResponseEntity<ApiResponseWrapper<InquiryDetailResponse>> getInquiryDetail( | |
| @Parameter(hidden = true) PrincipalDetails principalDetails, | |
| @Parameter(name = "postId", description = "조회할 문의의 ID", in = ParameterIn.PATH) | |
| @PathVariable Long postId | |
| ); |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis update introduces an admin controller and documentation for managing inquiries and answers, adds user-specific inquiry retrieval, enhances access control in inquiry detail retrieval, and refactors repository logic for paginated user inquiries. The user-facing inquiry controller is simplified to focus on personal inquiries, with all answer management moved to the admin context. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InquiryController
participant InquiryService
participant InquiryRepository
User->>InquiryController: GET /inquiry/my
InquiryController->>InquiryService: getMyInquiry(userId, pageable)
InquiryService->>InquiryRepository: searchMyInquiry(userId, pageable)
InquiryRepository-->>InquiryService: Page<InquiryResponse>
InquiryService-->>InquiryController: Page<InquiryResponse>
InquiryController-->>User: ApiResponseWrapper<Page<InquiryResponse>>
sequenceDiagram
participant Admin
participant AdminInquiryController
participant InquiryService
participant InquiryAnswerService
Admin->>AdminInquiryController: POST /admin/inquiry/{postId}/answer
AdminInquiryController->>InquiryAnswerService: createAnswer(postId, request)
InquiryAnswerService-->>AdminInquiryController: void
AdminInquiryController-->>Admin: ApiResponseWrapper<Void>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Documentation