-
Notifications
You must be signed in to change notification settings - Fork 1
큐레이션 수정 & 챗봇 이미지 적용 #71
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
Conversation
WalkthroughUpdates introduce type-driven curation retrieval via a new CurationType enum and required request parameter, refactor conversion to separate place/festival DTO builders, and adjust chat history mapping to select profile images based on message type. ChatMongoService gains a guestImage constructor parameter (unused in shown code). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant HC as HomeController
participant HQ as HomeQueryService
participant PD as Place data source
participant FD as Festival data source
participant CV as CurationConverter
C->>HC: GET /api/home/curation?type=PLACE|FESTIVAL
HC->>HQ: getCurations(type)
alt type == PLACE
HQ->>PD: fetch random 3 places
PD-->>HQ: List<Place> (size>=3?)
alt size < 3
HQ-->>HC: throw PLACE_NOT_FOUND
HC-->>C: Error response
else size >= 3
HQ->>CV: placeListToDto(places)
CV-->>HQ: CurationList (places)
end
else type == FESTIVAL
HQ->>FD: fetch random 3 festivals
FD-->>HQ: List<Festival> (size>=3?)
alt size < 3
HQ-->>HC: throw FESTIVAL_NOT_FOUND
HC-->>C: Error response
else size >= 3
HQ->>CV: festivalListToDto(festivals)
CV-->>HQ: CurationList (festivals)
end
end
HQ-->>HC: ApiResponse<CurationList>
HC-->>C: 200 OK with CurationList
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt (4)
86-99: OpenAI is called twice for one bot request (duplicated API call).
openAiService.chatToOpenAI(...)is invoked inbuildReceiveDto(...)and again here, doubling latency/cost and risking mismatched saved/returned content. Remove the second call:- if (receiveDto.type == MessageType.BOT_RESPONSE) { - // openai 요청 - openAiService.chatToOpenAI(chat.message) + if (receiveDto.type == MessageType.BOT_RESPONSE) { + // 응답은 buildReceiveDto에서 생성됨 // 챗봇 대답 저장 chatMongoRepository.save( ChatMessage( type = MessageType.BOT_RESPONSE, userId = currentUser.id, message = receiveDto.message, - time = chat.time + time = now ) ) }
58-62: Potential crash on empty message; use startsWith instead of indexing.
message[0]throws on empty strings. Replace with a safe prefix check:- val type: MessageType = if(message[0] == '/'){ - MessageType.BOT_REQUEST - }else{ - MessageType.CHAT - } + val type: MessageType = if (message.startsWith("/")) MessageType.BOT_REQUEST else MessageType.CHAT
137-149: pageSize=0 passes validation but breaks PageRequest (must be ≥1). Clamp or reject.Current checks allow 0, which will throw
IllegalArgumentExceptioninPageRequest.of. Suggest clamping to [1,30]:- // pageSize - if(pageSize < 0){ - throw GeneralException(ErrorStatus.INVALID_PAGE_SIZE_MINUS) - }else if (pageSize > 30) { - throw GeneralException(ErrorStatus.INVALID_PAGE_SIZE_BIG) - } + // Clamp pageSize to [1, 30] + val clampedSize = pageSize.coerceIn(1, 30) ... - val pageable = PageRequest.of(0, pageSize, Sort.by(Sort.Direction.DESC, "time")) + val pageable = PageRequest.of(0, clampedSize, Sort.by(Sort.Direction.DESC, "time"))
51-53: Prefer DI for AuthService over manual construction.
AuthService()should be injected to honor security context configuration and ease testing/mocking.Also applies to: 144-146
🧹 Nitpick comments (7)
src/main/kotlin/busanVibe/busan/domain/home/converter/CurationConverter.kt (2)
31-34: Inline the mapping to reduce noise.Return directly instead of a temp list.
- fun placeListToDto(placeList: List<Place>): HomeResponseDTO.CurationList { - val placeDtoList = placeList.map { placeToDto(it) } - return HomeResponseDTO.CurationList(placeDtoList) - } + fun placeListToDto(placeList: List<Place>): HomeResponseDTO.CurationList = + HomeResponseDTO.CurationList(placeList.map(::placeToDto))
36-39: Same in festival list converter.- fun festivalListToDto(festivalList: List<Festival>): HomeResponseDTO.CurationList { - val festivalDtoList = festivalList.map { festivalToDto(it) } - return HomeResponseDTO.CurationList(festivalDtoList) - } + fun festivalListToDto(festivalList: List<Festival>): HomeResponseDTO.CurationList = + HomeResponseDTO.CurationList(festivalList.map(::festivalToDto))src/main/kotlin/busanVibe/busan/domain/home/service/HomeQueryService.kt (1)
42-43: Consider extracting resultCount as a constant/config.Makes it easier to tune without code edits.
src/main/kotlin/busanVibe/busan/domain/home/controller/HomeController.kt (2)
30-37: Fix minor Korean copy typos and clarity in OpenAPI description.- @Operation(summary = "큐레이션 조회 API", - description = - """ - 명소 혹은 축제에 대한 큐레이션 정보를 반환합니다. - type이 필요합니다. - [ PLACE, FESTIVAL ] - ( 이미지 없는것은 베재, 장소는 관광지만 조회 ) - """ - ) + @Operation( + summary = "큐레이션 조회 API", + description = """ + 명소 혹은 축제에 대한 큐레이션 정보를 반환합니다. + type이 필요합니다. - [PLACE, FESTIVAL] + (이미지가 없는 항목은 배제, 장소는 관광지만 조회) + """ + )
38-40: Document allowed values on the parameter for Swagger UI.- fun getHomeCuration(@RequestParam("type", required = true) type: CurationType): ApiResponse<HomeResponseDTO.CurationList>{ + fun getHomeCuration( + @io.swagger.v3.oas.annotations.Parameter( + description = "큐레이션 타입", + example = "PLACE" + ) + @RequestParam("type", required = true) type: CurationType + ): ApiResponse<HomeResponseDTO.CurationList> {src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt (2)
91-98: Bot reply saved with user message timestamp.Use
nowto preserve chronology.- time = chat.time + time = now
33-35: Remove or use the unused injectedguestImagein ChatMongoService.kt
guestImageis injected in src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt (lines 34–35) but never referenced; either remove the constructor parameter or use it as the fallback for missing user/profile images. application.yml definesguest: ${IMAGE_GUEST}(src/main/resources/application.yml:69).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt(2 hunks)src/main/kotlin/busanVibe/busan/domain/home/controller/HomeController.kt(2 hunks)src/main/kotlin/busanVibe/busan/domain/home/converter/CurationConverter.kt(1 hunks)src/main/kotlin/busanVibe/busan/domain/home/enums/CurationType.kt(1 hunks)src/main/kotlin/busanVibe/busan/domain/home/service/HomeQueryService.kt(2 hunks)
🔇 Additional comments (4)
src/main/kotlin/busanVibe/busan/domain/home/enums/CurationType.kt (1)
3-8: Enum addition looks good.Scoped and clear; matches controller and service usage.
src/main/kotlin/busanVibe/busan/domain/home/converter/CurationConverter.kt (1)
31-34: Confirm image list guarantees to avoid NSEE from first().placeToDto/festivalToDto call .first() on image lists. Given service filters by “image not null,” confirm it guarantees at least one image per entity (non-empty list), not just non-null fields.
Also applies to: 36-39
src/main/kotlin/busanVibe/busan/domain/home/controller/HomeController.kt (1)
38-40: Enum binding is case-sensitive by default; confirm FE sends uppercase.If needed, we can add a custom Converter to accept case-insensitive values.
src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt (1)
121-130: Move OpenAI call out of DTO builderDo not perform I/O inside the DTO builder. Call openAiService.chatToOpenAI(chat.message) in saveAndPublish and pass the returned bot message into a pure ChatMessageReceiveDTO(...) to remove hidden side effects, simplify testing, and avoid duplicated logic.
Location: src/main/kotlin/busanVibe/busan/domain/chat/service/ChatMongoService.kt (around lines 121–130)
| val profileImage = when (chat.type) { | ||
| MessageType.CHAT, MessageType.BOT_REQUEST -> { | ||
| user?.profileImageUrl | ||
| } | ||
| MessageType.BOT_RESPONSE -> { | ||
| chatBotImage | ||
| } | ||
| } |
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.
🛠️ Refactor suggestion
Unify BOT_RESPONSE rendering and add guest fallback for missing user images.
- Show “챗봇” and
isMy=falsefor BOT_RESPONSE to match live send path. - Fallback to
guestImagewhen user/profile image is null.
Apply:
- val profileImage = when (chat.type) {
- MessageType.CHAT, MessageType.BOT_REQUEST -> {
- user?.profileImageUrl
- }
- MessageType.BOT_RESPONSE -> {
- chatBotImage
- }
- }
- ChatMessageResponseDTO.ChatInfoDto(
- userName = user?.nickname ?: "알 수 없음",
- userImage = profileImage,
- dateTime = chat.time,
- content = chat.message,
- isMy = user?.id == currentUser.id,
- type = chat.type
- )
+ val (userName, userImage, isMy) = when (chat.type) {
+ MessageType.BOT_RESPONSE ->
+ Triple("챗봇", chatBotImage, false)
+ MessageType.CHAT, MessageType.BOT_REQUEST ->
+ Triple(user?.nickname ?: "알 수 없음",
+ user?.profileImageUrl ?: guestImage,
+ user?.id == currentUser.id)
+ }
+ ChatMessageResponseDTO.ChatInfoDto(
+ userName = userName,
+ userImage = userImage,
+ dateTime = chat.time,
+ content = chat.message,
+ isMy = isMy,
+ type = chat.type
+ )Also applies to: 181-185
| fun getCurations(type: CurationType): HomeResponseDTO.CurationList{ | ||
|
|
||
| // 자원 생성 | ||
| val placeCount = 1 | ||
| val festivalCount = 2 | ||
|
|
||
| // 명소 조회 | ||
| val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회 | ||
| val randomPlace: List<Place> = placeList | ||
| .takeIf { it.size >= placeCount } | ||
| ?.shuffled() | ||
| ?.take(placeCount) | ||
| ?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | ||
|
|
||
|
|
||
| // 축제 조회 | ||
| val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회 | ||
| val randomFestival: List<Festival> = festivalList | ||
| .takeIf { it.size >= festivalCount } | ||
| ?.shuffled() | ||
| ?.take(festivalCount) | ||
| ?: throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | ||
|
|
||
| // DTO 생성 및 반환 | ||
| return CurationConverter().toListDto(randomPlace, randomFestival) | ||
| val resultCount = 3 | ||
|
|
||
| // 조건에 따라 조회 후 DTO 반환 | ||
| return when (type) { | ||
| // 명소 조회 | ||
| CurationType.PLACE -> { | ||
| val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회 | ||
| val randomPlace: List<Place> = placeList | ||
| .takeIf { it.size >= resultCount } | ||
| ?.shuffled() | ||
| ?.take(resultCount) | ||
| ?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | ||
| CurationConverter().placeListToDto(randomPlace) | ||
| } | ||
|
|
||
| // 축제 조회 | ||
| CurationType.FESTIVAL -> { | ||
| val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회 | ||
| val randomFestival: List<Festival> = festivalList | ||
| .takeIf { it.size >= resultCount } | ||
| ?.shuffled() | ||
| ?.take(resultCount) | ||
| ?: throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | ||
| CurationConverter().festivalListToDto(randomFestival) | ||
| } | ||
|
|
||
| } |
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.
🛠️ Refactor suggestion
Don’t fail when available items < 3; degrade gracefully (align comment with behavior).
Current logic throws unless size ≥ 3, but the comment says “list empty -> exception.” Prefer returning up to 3 and only error on empty.
- val resultCount = 3
+ val resultCount = 3
+ val converter = CurationConverter()
@@
- CurationType.PLACE -> {
- val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회
- val randomPlace: List<Place> = placeList
- .takeIf { it.size >= resultCount }
- ?.shuffled()
- ?.take(resultCount)
- ?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리
- CurationConverter().placeListToDto(randomPlace)
- }
+ CurationType.PLACE -> {
+ val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회
+ val randomPlace = placeList.shuffled().take(resultCount)
+ if (randomPlace.isEmpty()) throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 리스트 비어있을 때만 예외
+ converter.placeListToDto(randomPlace)
+ }
@@
- CurationType.FESTIVAL -> {
- val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회
- val randomFestival: List<Festival> = festivalList
- .takeIf { it.size >= resultCount }
- ?.shuffled()
- ?.take(resultCount)
- ?: throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리
- CurationConverter().festivalListToDto(randomFestival)
- }
+ CurationType.FESTIVAL -> {
+ val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회
+ val randomFestival = festivalList.shuffled().take(resultCount)
+ if (randomFestival.isEmpty()) throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 리스트 비어있을 때만 예외
+ converter.festivalListToDto(randomFestival)
+ }📝 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.
| fun getCurations(type: CurationType): HomeResponseDTO.CurationList{ | |
| // 자원 생성 | |
| val placeCount = 1 | |
| val festivalCount = 2 | |
| // 명소 조회 | |
| val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회 | |
| val randomPlace: List<Place> = placeList | |
| .takeIf { it.size >= placeCount } | |
| ?.shuffled() | |
| ?.take(placeCount) | |
| ?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | |
| // 축제 조회 | |
| val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회 | |
| val randomFestival: List<Festival> = festivalList | |
| .takeIf { it.size >= festivalCount } | |
| ?.shuffled() | |
| ?.take(festivalCount) | |
| ?: throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | |
| // DTO 생성 및 반환 | |
| return CurationConverter().toListDto(randomPlace, randomFestival) | |
| val resultCount = 3 | |
| // 조건에 따라 조회 후 DTO 반환 | |
| return when (type) { | |
| // 명소 조회 | |
| CurationType.PLACE -> { | |
| val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회 | |
| val randomPlace: List<Place> = placeList | |
| .takeIf { it.size >= resultCount } | |
| ?.shuffled() | |
| ?.take(resultCount) | |
| ?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | |
| CurationConverter().placeListToDto(randomPlace) | |
| } | |
| // 축제 조회 | |
| CurationType.FESTIVAL -> { | |
| val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회 | |
| val randomFestival: List<Festival> = festivalList | |
| .takeIf { it.size >= resultCount } | |
| ?.shuffled() | |
| ?.take(resultCount) | |
| ?: throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 그 중 랜덤한 항목 가져옴. list 비어있을 시 예외처리 | |
| CurationConverter().festivalListToDto(randomFestival) | |
| } | |
| } | |
| fun getCurations(type: CurationType): HomeResponseDTO.CurationList{ | |
| // 자원 생성 | |
| val resultCount = 3 | |
| val converter = CurationConverter() | |
| // 조건에 따라 조회 후 DTO 반환 | |
| return when (type) { | |
| // 명소 조회 | |
| CurationType.PLACE -> { | |
| val placeList: List<Place> = placeRepository.findPlaceImageNotNull() // 이미지 데이터가 있는 Place 목록 조회 | |
| val randomPlace = placeList.shuffled().take(resultCount) | |
| if (randomPlace.isEmpty()) throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND) // 리스트 비어있을 때만 예외 | |
| converter.placeListToDto(randomPlace) | |
| } | |
| // 축제 조회 | |
| CurationType.FESTIVAL -> { | |
| val festivalList = festivalRepository.findFestivalImageNotNull() // 이미지 데이터가 있는 Festival 목록 조회 | |
| val randomFestival = festivalList.shuffled().take(resultCount) | |
| if (randomFestival.isEmpty()) throw ExceptionHandler(ErrorStatus.FESTIVAL_NOT_FOUND) // 리스트 비어있을 때만 예외 | |
| converter.festivalListToDto(randomFestival) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/busanVibe/busan/domain/home/service/HomeQueryService.kt
around lines 39 to 68, the current code throws if the list size is less than 3
but the comment implies only empty lists should error; change the logic so that
you only throw ExceptionHandler(ErrorStatus.*_NOT_FOUND) when the repository
result is empty, otherwise shuffle and take up to resultCount (i.e.,
take(resultCount) on the shuffled list regardless of size), and update the
inline comments to state "return up to 3 random items; throw only if list is
empty."
Summary by CodeRabbit
New Features
Documentation
Bug Fixes