Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough게시판에 displayOrder 도입 및 재정렬 API 추가, 게시판 이름 중복 예외 도입, 게시글 생성 시 기수 자동 채움, 게시글 응답에 게시판 식별 정보 추가, 클럽 생성 시 기본 공지 게시판 자동 생성 등 도메인·레포지토리·DTO·컨트롤러·테스트 전반 변경입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant BoardAdminCtrl as "BoardAdminController"
participant UseCase as "ManageBoardUseCase"
participant Repo as "BoardRepository"
participant DB as "Database"
Client->>BoardAdminCtrl: PUT /api/.../boards/order {boardIds}
BoardAdminCtrl->>UseCase: reorder(clubId, request, userId)
UseCase->>Repo: findAllByClubIdOrderByDisplayOrderAscIdAsc(clubId)
Repo-->>UseCase: List<Board>
UseCase->>UseCase: validate duplicates / fixed/deleted / ownership
UseCase->>Repo: save(updatedBoard) [여러 회]
Repo-->>DB: persist 변경된 displayOrder
DB-->>Repo: ack
Repo-->>UseCase: saved entities
UseCase-->>BoardAdminCtrl: Unit
BoardAdminCtrl-->>Client: 200 BOARD_REORDERED_SUCCESS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt (1)
96-96: 사용되지 않는 변수user
delete메서드에서user변수가 선언되었지만 사용되지 않습니다. 불필요한 DB 조회를 유발하므로 제거를 권장합니다.♻️ 제안된 수정
`@Transactional` fun delete( clubId: Long, postId: Long, userId: Long, ) { val member = clubMemberPolicy.getActiveMember(clubId, userId) - val user = userReader.getById(userId) val post = findPost(postId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt` at line 96, Remove the unused local variable `user` and the associated database call `userReader.getById(userId)` from the `delete` method in `ManagePostUseCase` since it is not referenced; update the `delete` method to rely on existing checks (or parameters) instead of performing this redundant lookup and ensure no remaining references to `user` remain in that method.src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt (2)
83-89:toDetailResponse테스트에도 board 필드 검증 추가를 고려하세요.마찬가지로
toDetailResponse테스트에서도boardId와boardName에 대한 명시적 assertion을 추가하면 매핑 검증이 완전해집니다.♻️ 제안하는 수정
response.id shouldBe 1L + response.boardId shouldBe 10L + response.boardName shouldBe "일반 게시판" response.commentCount shouldBe 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt` around lines 83 - 89, Add explicit assertions for the board mapping in the PostMapperTest: after calling mapper.toDetailResponse(post, authorMember, comments, files) assert response.boardId equals the expected board id and response.boardName equals the expected board name so the toDetailResponse mapping is fully verified; locate these assertions in the same test method that calls mapper.toDetailResponse and reference response.boardId and response.boardName when adding the checks.
47-55: 새 필드에 대한 명시적 검증 추가를 고려하세요.
boardId와boardName필드가 mock으로 설정되었지만,toListResponse테스트에서 해당 필드들에 대한 명시적 assertion이 없습니다. 매핑이 올바르게 동작하는지 확인하기 위해 추가하는 것이 좋습니다.♻️ 제안하는 수정
response.id shouldBe 1L + response.boardId shouldBe 10L + response.boardName shouldBe "일반 게시판" response.hasFile shouldBe true response.isNew shouldBe true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt` around lines 47 - 55, The test for mapper.toListResponse omits assertions for the newly added boardId and boardName fields; update the test in PostMapperTest (inside the "toListResponse" spec) to assert that response.boardId and response.boardName equal the expected mock values (the ones used to build post/authorMember), keeping the existing checks for id, hasFile, and isNew so the mapping for boardId/boardName is explicitly verified.src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.kt (1)
82-90: 기본 공지사항 게시판 스펙은 별도 팩토리/헬퍼로 분리하는 편을 권장합니다.Line 86-89의 하드코딩 스펙은 향후 기본 게시판 정책 변경 시 해당 유스케이스의 수정 포인트를 늘릴 수 있습니다. 생성 규칙을 한 곳으로 모아두면 변경 결합도를 줄일 수 있습니다.
예시 리팩터링
class ManageClubUseCase( @@ ) { + private companion object { + const val DEFAULT_NOTICE_BOARD_NAME = "공지사항" + } @@ - // 공지사항 게시판 자동 생성 (관리자만 작성 가능, displayOrder=0) - val noticeBoard = - Board( - club = club, - name = "공지사항", - type = BoardType.NOTICE, - config = BoardConfig(writePermission = MemberRole.ADMIN), - ) + // 공지사항 게시판 자동 생성 + val noticeBoard = createDefaultNoticeBoard(club) boardRepository.save(noticeBoard) @@ + private fun createDefaultNoticeBoard(club: Club): Board = + Board( + club = club, + name = DEFAULT_NOTICE_BOARD_NAME, + type = BoardType.NOTICE, + config = BoardConfig(writePermission = MemberRole.ADMIN), + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.kt` around lines 82 - 90, Extract the hardcoded creation of the default notice board in ManageClubUseCase (the block that constructs Board with type BoardType.NOTICE and config BoardConfig(writePermission = MemberRole.ADMIN) and then calls boardRepository.save(noticeBoard)) into a single factory/helper method so the creation policy is centralized; implement a factory function (e.g., BoardFactory.createDefaultNoticeBoard(club) or a Board companion factory like Board.createNoticeBoard(club)) that returns the preconfigured Board and update ManageClubUseCase to call that factory and save the result, ensuring all default specs (name "공지사항", type BoardType.NOTICE, config writePermission = MemberRole.ADMIN, and any future defaults) are maintained in one place.src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt (1)
164-170: 공지사항 게시판 권한 속성도 함께 검증하면 회귀 방지에 더 좋습니다.현재는 Line 167-169에서 타입/이름/순서만 확인합니다. 생성 의도가 “관리자만 작성 가능”이라면
board.config.writePermission == MemberRole.ADMIN도 같이 검증해두면 정책 변경 시 빠르게 감지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt` around lines 164 - 170, The test's verify block for boardRepository.save (the match lambda in ManageClubUseCaseTest) currently asserts only BoardType, name and displayOrder; extend that match to also assert the created board's permission config by checking board.config.writePermission == MemberRole.ADMIN (and ensure board.config is non-null) so the test verifies the "admin-only write" policy and prevents regressions if permissions change.src/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.kt (1)
24-25: JPQL 쿼리에 soft-delete 필터 추가 권장
findMaxDisplayOrderByClubId쿼리가 삭제된 게시판의displayOrder도 포함하여 최대값을 계산합니다. 새 게시판 생성 시 삭제된 게시판의 순서값 때문에 불필요한 간격이 발생할 수 있습니다.♻️ 제안된 수정
- `@Query`("SELECT COALESCE(MAX(b.displayOrder), -1) FROM Board b WHERE b.club.id = :clubId") + `@Query`("SELECT COALESCE(MAX(b.displayOrder), -1) FROM Board b WHERE b.club.id = :clubId AND b.isDeleted = false") fun findMaxDisplayOrderByClubId(clubId: Long): Int🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.kt` around lines 24 - 25, The JPQL in findMaxDisplayOrderByClubId currently includes soft-deleted boards; update the query on Board to exclude deleted entries by adding the soft-delete predicate (e.g., change WHERE b.club.id = :clubId to WHERE b.club.id = :clubId AND b.deleted = false or AND b.deletedAt IS NULL depending on your Board entity soft-delete field) so the MAX(b.displayOrder) ignores deleted boards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/com/weeth/domain/board/application/mapper/PostMapper.kt`:
- Around line 28-29: The repository method findAllActiveByBoardId() is only
fetching "user" via `@EntityGraph`(attributePaths = ["user"]), causing N+1 when
PostMapper accesses post.board.id and post.board.name; update the PostRepository
entry for findAllActiveByBoardId() (and/or findByIdAndIsDeletedFalse() if used
similarly) to include board in the entity graph (e.g.
`@EntityGraph`(attributePaths = ["user","board"])) so PostMapper can read
post.board without triggering lazy loads, or alternatively mirror the fix used
in findAllActiveByBoardIds().
In
`@src/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalRepository.kt`:
- Around line 54-60: The current derived query method findByClubIdAndStatus
returning a single Cardinal can throw IncorrectResultSizeDataAccessException if
multiple IN_PROGRESS rows exist; update CardinalRepository to either (a) replace
findByClubIdAndStatus(clubId: Long, status: CardinalStatus): Cardinal? with
findFirstByClubIdAndStatus(...) to explicitly return the first match, or (b)
change it to return List<Cardinal> and then update override fun
findInProgressByClubId(clubId: Long): Cardinal? to call the list-returning
method and safely return firstOrNull(); also adjust callers such as
CardinalStatusPolicy.activateExclusively to use the new API so club-scoped
IN_PROGRESS handling avoids runtime errors.
In `@src/test/kotlin/com/weeth/domain/board/fixture/BoardTestFixture.kt`:
- Around line 18-26: The Board test fixture's multiline Board constructor must
be reformatted to satisfy ktlint: start the multiline expression on a new line
and add a trailing comma before the closing parenthesis in the Board(...) call;
update the Board creation in the function in BoardTestFixture.kt (the Board(...)
instantiation where you later call ReflectionTestUtils.setField(board, "id",
id)) so each argument is on its own line, the opening parenthesis is followed by
a newline, and the final argument line ends with a comma before the closing
parenthesis, then run ./gradlew ktlintFormat to apply formatting.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt`:
- Line 96: Remove the unused local variable `user` and the associated database
call `userReader.getById(userId)` from the `delete` method in
`ManagePostUseCase` since it is not referenced; update the `delete` method to
rely on existing checks (or parameters) instead of performing this redundant
lookup and ensure no remaining references to `user` remain in that method.
In `@src/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.kt`:
- Around line 24-25: The JPQL in findMaxDisplayOrderByClubId currently includes
soft-deleted boards; update the query on Board to exclude deleted entries by
adding the soft-delete predicate (e.g., change WHERE b.club.id = :clubId to
WHERE b.club.id = :clubId AND b.deleted = false or AND b.deletedAt IS NULL
depending on your Board entity soft-delete field) so the MAX(b.displayOrder)
ignores deleted boards.
In
`@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.kt`:
- Around line 82-90: Extract the hardcoded creation of the default notice board
in ManageClubUseCase (the block that constructs Board with type BoardType.NOTICE
and config BoardConfig(writePermission = MemberRole.ADMIN) and then calls
boardRepository.save(noticeBoard)) into a single factory/helper method so the
creation policy is centralized; implement a factory function (e.g.,
BoardFactory.createDefaultNoticeBoard(club) or a Board companion factory like
Board.createNoticeBoard(club)) that returns the preconfigured Board and update
ManageClubUseCase to call that factory and save the result, ensuring all default
specs (name "공지사항", type BoardType.NOTICE, config writePermission =
MemberRole.ADMIN, and any future defaults) are maintained in one place.
In `@src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt`:
- Around line 83-89: Add explicit assertions for the board mapping in the
PostMapperTest: after calling mapper.toDetailResponse(post, authorMember,
comments, files) assert response.boardId equals the expected board id and
response.boardName equals the expected board name so the toDetailResponse
mapping is fully verified; locate these assertions in the same test method that
calls mapper.toDetailResponse and reference response.boardId and
response.boardName when adding the checks.
- Around line 47-55: The test for mapper.toListResponse omits assertions for the
newly added boardId and boardName fields; update the test in PostMapperTest
(inside the "toListResponse" spec) to assert that response.boardId and
response.boardName equal the expected mock values (the ones used to build
post/authorMember), keeping the existing checks for id, hasFile, and isNew so
the mapping for boardId/boardName is explicitly verified.
In
`@src/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt`:
- Around line 164-170: The test's verify block for boardRepository.save (the
match lambda in ManageClubUseCaseTest) currently asserts only BoardType, name
and displayOrder; extend that match to also assert the created board's
permission config by checking board.config.writePermission == MemberRole.ADMIN
(and ensure board.config is non-null) so the test verifies the "admin-only
write" policy and prevents regressions if permissions change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbd7f4e8-1e9d-4f2a-951a-d2d901a03538
📒 Files selected for processing (34)
src/main/kotlin/com/weeth/domain/board/application/dto/request/CreatePostRequest.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/request/ReorderBoardsRequest.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/BoardDetailResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/PostDetailResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/PostListResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/BoardErrorCode.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/DuplicateBoardIdException.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/DuplicateBoardNameException.ktsrc/main/kotlin/com/weeth/domain/board/application/mapper/BoardMapper.ktsrc/main/kotlin/com/weeth/domain/board/application/mapper/PostMapper.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryService.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.ktsrc/main/kotlin/com/weeth/domain/board/domain/entity/Board.ktsrc/main/kotlin/com/weeth/domain/board/domain/entity/Post.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.ktsrc/main/kotlin/com/weeth/domain/board/presentation/BoardAdminController.ktsrc/main/kotlin/com/weeth/domain/board/presentation/BoardResponseCode.ktsrc/main/kotlin/com/weeth/domain/board/presentation/PostController.ktsrc/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalReader.ktsrc/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalRepository.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCase.ktsrc/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/board/domain/entity/BoardEntityTest.ktsrc/test/kotlin/com/weeth/domain/board/domain/entity/PostEntityTest.ktsrc/test/kotlin/com/weeth/domain/board/fixture/BoardTestFixture.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubUseCaseTest.kt
💤 Files with no reviewable changes (4)
- src/main/kotlin/com/weeth/domain/board/application/dto/request/CreatePostRequest.kt
- src/main/kotlin/com/weeth/domain/board/domain/entity/Post.kt
- src/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.kt
- src/test/kotlin/com/weeth/domain/board/domain/entity/PostEntityTest.kt
src/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalRepository.kt
Outdated
Show resolved
Hide resolved
| import jakarta.validation.constraints.NotEmpty | ||
|
|
||
| data class ReorderBoardsRequest( | ||
| @field:Schema(description = "원하는 순서대로 정렬된 게시판 ID 목록", example = "[3, 1, 2]") |
There was a problem hiding this comment.
설명이 조금 더 이해하기 쉽도록 바꿔볼 수 있을까용??
"새롭게 정렬된 순서로 게시판 ID를 리스트로 보내주세요" 같이요!
There was a problem hiding this comment.
order도 1부터 시작하니까 이점도 보여지면 좋겠네요!
| ) | ||
|
|
||
| @GetMapping("/posts") | ||
| @Operation(summary = "전체 게시글 조회", description = "클럽 내 접근 가능한 모든 게시판의 게시글을 최신순으로 조회합니다.") |
There was a problem hiding this comment.
대시보드에서도 클럽 내 접근 가능한 최신 피드로 보여지고 있나용??
There was a problem hiding this comment.
앗 대시보드는 필터링없이 보여주고 있어서 수정하겟습니다!!
|
|
||
| clubRepository.save(club) | ||
|
|
||
| // 공지사항 게시판 자동 생성 (관리자만 작성 가능, displayOrder=0) |
There was a problem hiding this comment.
전체 게시판은 실물이 있는 건 아니라서..혹시 어떻게 처리되고 있나요??
어드민 사이드에서 순서 변경이나 통계는 지원해줘야하고, 유저사이드에서는 새로 구현된 API로 조회는 가능한데 순서 변경도 가능한 상태일까욤??
There was a problem hiding this comment.
전체게시판은 실물이 없어서 프론트랑 논의를 해봐야합니다!
일단 제 생각으로는 공지랑 전체는 고정이니까 공지 0번 / a게시판 1번 / b게시판 2번으로 들어오면
프론트에서 공지 넣고 (전체는 위치 고정) 이 다음에 순서대로 넣어주는 방향으로 될 것 같슴니다ㅅ
There was a problem hiding this comment.
아니면 백에서 전체게시판을 가상항목으로 넣어서 응답을 보내는 방법도 있습니당
There was a problem hiding this comment.
실물 vs 가상 처리 중 어떤 방식이 효율적일까요??
현재도 게시글 통계가 있지만, 추후에 확장될 수도 있고. 순서도 그렇고 어떤 방식이 나을지 고민을 해봐야겠네요..
There was a problem hiding this comment.
실물방식
장점
- displayOrder 기반 순서 변경 로직을 그대로 재사용 가능
- 어드민 통계/순서 API가 기존 Board CRUD와 동일하게 동작
- 추후 전체게시판 전용 설정 추가 시 컬럼 확장만 하면 됨
단점 - "전체게시판"은 글이 실제로 저장되는 게시판이 아니라서 글 작성 시 선택 불가 예외 처리 필요
- 삭제/수정 방지 로직 추가 필요 (공지, 전체는 고정 항목)
- findRecentByBoardIds 등 쿼리에서 전체게시판 ID가 섞이지 않도록 필터 주의
가상 방식
장점
- 도메인 순수성 유지 — "전체게시판"은 뷰 개념이므로 DB에 넣지 않는 게 의미상 정확
- 글 작성 선택지에서 자연스럽게 제외됨
단점 - 순서 저장 위치가 없음 — displayOrder를 어디에 저장할지 별도 설계 필요
- 어드민 순서 변경 API에 "가상 항목" 처리 분기가 추가됨
- 통계 확장 시 매번 별도 처리 로직 필요
- 코드 곳곳에 if (isVirtualBoard) 분기가 생길 수 있음
라고하는데 실물기반이 더 좋을 것 같긴합니다욤..
There was a problem hiding this comment.
- 공지사항/전체 게시판은 기본으로 제공하고 순서를 변경하는 것을 제공하지 않는다. 2. 통계/가상 집계 뷰로만 사용한다 -> 가상 방식이 나음이긴 할 것 같네요..
There was a problem hiding this comment.
이를 인지하고 있으면 나중에 꼬일 일도 없을 것 같고. 일단 이렇게 갈까요??
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/kotlin/com/weeth/domain/board/domain/repository/PostReader.kt (1)
30-33: 빈boardIds입력 계약을 여기서도 명시해 두는 편이 안전합니다.현재 구현은 이 메서드를 바로
IN :boardIds쿼리로 내려보내서, 다른 호출자가emptyList()를 넘기면 런타임 SQL 오류로 이어질 수 있습니다. 이번 PR의GetDashboardQueryService는 가드하고 있지만, 인터페이스 계약 차원에서 non-empty 전제를 문서화하거나 구현에서 빈Slice를 반환하도록 맞춰 두면 재사용 시 더 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/domain/repository/PostReader.kt` around lines 30 - 33, The public reader method findRecentByBoardIds in PostReader currently allows callers to pass an empty boardIds list which can lead to runtime SQL errors when translated to an IN () clause; update the contract/implementation so empty input is handled safely—either document in PostReader that boardIds must be non-empty and validate/throw IllegalArgumentException, or (preferable) make the implementation return an empty Slice<Post> when boardIds.isEmpty() (so callers like GetDashboardQueryService need not guard), and ensure the repository/implementation method early-returns an empty Slice for empty lists to avoid generating an empty IN clause.src/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.kt (1)
175-183:NOTICE제외 조건은 아직 회귀 테스트로 고정되지 않았습니다.테스트명은 “공지 제외”인데, 실제 fixture에는
BoardType.NOTICE게시판이 없어 Line 80의it.type != BoardType.NOTICE가 빠져도 그대로 통과합니다. 일반 게시판과 공지 게시판을 함께 반환하도록 만들어,postReader.findRecentByBoardIds(...)에 일반 게시판 id만 전달되는지까지 묶어 두는 편이 좋겠습니다.테스트 보강 예시
it("공지 제외한 접근 가능한 게시판의 최신 게시글을 반환한다") { val board = BoardTestFixture.create(id = 10L, type = BoardType.GENERAL) + val noticeBoard = BoardTestFixture.create(id = 11L, type = BoardType.NOTICE) val post = PostTestFixture.create(board = board, user = user) val pageable = PageRequest.of(0, 10) val slice = SliceImpl(listOf(post), pageable, false) every { clubMemberReader.findByClubIdAndUserId(clubId, userId) } returns memberWithUser - every { boardReader.findAllActiveByClubId(clubId) } returns listOf(board) + every { boardReader.findAllActiveByClubId(clubId) } returns listOf(board, noticeBoard) every { postReader.findRecentByBoardIds(listOf(board.id), any()) } returns slice🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.kt` around lines 175 - 183, The test currently only creates a GENERAL board so the "exclude NOTICE" behavior isn't asserted; update the test in GetDashboardQueryServiceTest (the it("공지 제외한 접근 가능한 게시판의 최신 게시글을 반환한다") case) to create two fixtures via BoardTestFixture.create: one with type = BoardType.GENERAL and one with type = BoardType.NOTICE, return both from boardReader.findAllActiveByClubId (use listOf(generalBoard, noticeBoard)), and then set the mock expectation on postReader.findRecentByBoardIds to expect only the GENERAL board id (every { postReader.findRecentByBoardIds(listOf(generalBoard.id), any()) } returns slice) so the test verifies that BoardType.NOTICE is excluded; keep existing memberWithUser setup unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/kotlin/com/weeth/domain/board/domain/repository/PostReader.kt`:
- Around line 30-33: The public reader method findRecentByBoardIds in PostReader
currently allows callers to pass an empty boardIds list which can lead to
runtime SQL errors when translated to an IN () clause; update the
contract/implementation so empty input is handled safely—either document in
PostReader that boardIds must be non-empty and validate/throw
IllegalArgumentException, or (preferable) make the implementation return an
empty Slice<Post> when boardIds.isEmpty() (so callers like
GetDashboardQueryService need not guard), and ensure the
repository/implementation method early-returns an empty Slice for empty lists to
avoid generating an empty IN clause.
In
`@src/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.kt`:
- Around line 175-183: The test currently only creates a GENERAL board so the
"exclude NOTICE" behavior isn't asserted; update the test in
GetDashboardQueryServiceTest (the it("공지 제외한 접근 가능한 게시판의 최신 게시글을 반환한다") case) to
create two fixtures via BoardTestFixture.create: one with type =
BoardType.GENERAL and one with type = BoardType.NOTICE, return both from
boardReader.findAllActiveByClubId (use listOf(generalBoard, noticeBoard)), and
then set the mock expectation on postReader.findRecentByBoardIds to expect only
the GENERAL board id (every {
postReader.findRecentByBoardIds(listOf(generalBoard.id), any()) } returns slice)
so the test verifies that BoardType.NOTICE is excluded; keep existing
memberWithUser setup unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d380667f-e5c9-4be5-9ec4-ad7d4465d30f
📒 Files selected for processing (8)
src/main/kotlin/com/weeth/domain/board/application/dto/request/ReorderBoardsRequest.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/BoardReader.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostReader.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.ktsrc/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalRepository.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryService.ktsrc/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.kt
✅ Files skipped from review due to trivial changes (1)
- src/main/kotlin/com/weeth/domain/board/application/dto/request/ReorderBoardsRequest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/kotlin/com/weeth/domain/cardinal/domain/repository/CardinalRepository.kt
- src/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.kt
- src/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/com/weeth/domain/board/application/dto/response/BoardListResponse.kt (1)
7-8: OpenAPI에서id필드의 nullable을 명시적으로 설정해야 합니다.springdoc-openapi는 Kotlin의 nullable 타입(
Long?)을 자동으로 OpenAPI 스키마의nullable: true로 변환하지 않습니다. 현재 코드는 설명에만 null 가능성을 명시하고 있어서, 클라이언트 코드 생성 시 nullable 속성이 누락될 수 있습니다.@Schema(nullable = true)설정을 추가하여 생성된 OpenAPI 스펙에nullable: true가 명확히 포함되도록 하십시오.♻️ 제안 diff
- `@field`:Schema(description = "게시판 ID (전체 게시판은 null)") + `@field`:Schema(description = "게시판 ID (전체 게시판은 null)", nullable = true) val id: Long?,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/application/dto/response/BoardListResponse.kt` around lines 7 - 8, The OpenAPI schema for the BoardListResponse.id field isn't marked nullable; update the `@field`:Schema annotation on the id property in BoardListResponse (the val id: Long? field) to include nullable = true so springdoc-openapi emits "nullable: true" in the generated spec; also ensure the import for io.swagger.v3.oas.annotations.media.Schema is present if missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt`:
- Around line 141-143: The reorder loop in ManageBoardUseCase assigns zero-based
indices to movable boards which can collide with fixed NOTICE boards; update the
loop to apply an offset equal to the count of fixed boards so movable boards
start after fixed ones. Compute val fixedOffset = fixedBoards.size and use
fixedOffset + index when calling boardById.getValue(boardId).reorder(...),
keeping references to request.boardIds, boardById, reorder and fixedBoards so
the new displayOrder preserves the intended ordering.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/board/application/dto/response/BoardListResponse.kt`:
- Around line 7-8: The OpenAPI schema for the BoardListResponse.id field isn't
marked nullable; update the `@field`:Schema annotation on the id property in
BoardListResponse (the val id: Long? field) to include nullable = true so
springdoc-openapi emits "nullable: true" in the generated spec; also ensure the
import for io.swagger.v3.oas.annotations.media.Schema is present if missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6d44134-7078-44d4-b23e-05cd3512feb9
📒 Files selected for processing (11)
src/main/kotlin/com/weeth/domain/board/application/dto/request/ReorderBoardsRequest.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/BoardListResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/BoardErrorCode.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/FixedBoardNotReorderableException.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryService.ktsrc/main/kotlin/com/weeth/domain/board/domain/entity/Board.ktsrc/main/kotlin/com/weeth/domain/board/domain/enums/BoardType.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/board/domain/entity/BoardEntityTest.kt
✅ Files skipped from review due to trivial changes (2)
- src/main/kotlin/com/weeth/domain/board/application/dto/request/ReorderBoardsRequest.kt
- src/main/kotlin/com/weeth/domain/board/domain/enums/BoardType.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryService.kt
- src/test/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryServiceTest.kt
- src/main/kotlin/com/weeth/domain/board/application/exception/BoardErrorCode.kt
- src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.kt
- src/test/kotlin/com/weeth/domain/board/domain/entity/BoardEntityTest.kt
| request.boardIds.forEachIndexed { index, boardId -> | ||
| boardById.getValue(boardId).reorder(index) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how boards are queried/ordered to verify if displayOrder collision is handled
# Find repository methods that query boards with ordering
rg -n "findAll.*ByClubId.*Order" --type=kt -A 3
# Check if there's any sorting by type before displayOrder
rg -n "BoardType.NOTICE|type.*displayOrder|displayOrder.*type" --type=kt -C 2Repository: Team-Weeth/weeth-server
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Find the ManageBoardUseCase.kt file first
find . -name "ManageBoardUseCase.kt" -type f
# List Board-related files
find . -name "*Board*.kt" -type f | grep -E "(repository|entity|domain)" | head -20Repository: Team-Weeth/weeth-server
Length of output: 1857
🏁 Script executed:
# Search for board repository methods and ordering logic without -type flag
rg -n "findAll.*ByClubId" -A 3 . --glob="*.kt"
# Check for BoardType.NOTICE and displayOrder usage
rg -n "BoardType\.NOTICE|displayOrder" -C 2 . --glob="*.kt" | head -100Repository: Team-Weeth/weeth-server
Length of output: 50379
🏁 Script executed:
# Examine the ManageBoardUseCase.kt file directly
cat -n src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt | sed -n '100,150p'Repository: Team-Weeth/weeth-server
Length of output: 1977
🏁 Script executed:
cat src/main/kotlin/com/weeth/domain/board/domain/entity/Board.ktRepository: Team-Weeth/weeth-server
Length of output: 2822
🏁 Script executed:
# Check for displayOrder handling in Board entity or repository
rg -n "displayOrder" src/main/kotlin/com/weeth/domain/board --glob="*.kt" -B 2 -A 2Repository: Team-Weeth/weeth-server
Length of output: 4139
재정렬 가능한 게시판의 displayOrder 오프셋 적용 권장
현재 코드에서 재정렬 가능한 게시판에 0부터 시작하는 인덱스를 할당하면, NOTICE 타입 게시판의 기본 displayOrder(0)과 동일한 값을 가집니다.
비록 저장소 쿼리가 displayOrder 다음 id로 정렬하고, 조회 로직이 NOTICE 게시판을 별도로 필터링하므로 기능상 문제는 없지만, 설계 의도와의 일관성을 위해 다음과 같이 고정 게시판 수만큼 오프셋을 적용하는 것을 권장합니다:
제안된 변경
val fixedOffset = fixedBoards.size
request.boardIds.forEachIndexed { index, boardId ->
boardById.getValue(boardId).reorder(fixedOffset + index)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt`
around lines 141 - 143, The reorder loop in ManageBoardUseCase assigns
zero-based indices to movable boards which can collide with fixed NOTICE boards;
update the loop to apply an offset equal to the count of fixed boards so movable
boards start after fixed ones. Compute val fixedOffset = fixedBoards.size and
use fixedOffset + index when calling boardById.getValue(boardId).reorder(...),
keeping references to request.boardIds, boardById, reorder and fixedBoards so
the new displayOrder preserves the intended ordering.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.kt`:
- Around line 189-200: The query in countActivePostsByBoardIds (Post p) only
filters p.isDeleted = false but omits filtering out posts whose parent board is
deleted; update the JPQL in the countActivePostsByBoardIds method to also
include AND p.board.isDeleted = false so it mirrors other PostRepository queries
and groups by p.board.id as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d832e951-5c7b-4d24-8368-0ce215e9494f
📒 Files selected for processing (22)
src/main/kotlin/com/weeth/domain/board/application/dto/response/BoardDetailResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/PostDetailResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/PostListResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/BoardErrorCode.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/DeletedBoardNotReorderableException.ktsrc/main/kotlin/com/weeth/domain/board/application/exception/FixedBoardNotRenamableException.ktsrc/main/kotlin/com/weeth/domain/board/application/mapper/BoardMapper.ktsrc/main/kotlin/com/weeth/domain/board/application/mapper/PostMapper.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryService.ktsrc/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.ktsrc/main/kotlin/com/weeth/domain/board/domain/entity/Post.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/BoardPostCount.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/BoardRepository.ktsrc/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.ktsrc/main/kotlin/com/weeth/domain/board/presentation/BoardResponseCode.ktsrc/main/kotlin/com/weeth/domain/board/presentation/PostController.ktsrc/main/kotlin/com/weeth/domain/session/domain/entity/Session.ktsrc/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetBoardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.kt
💤 Files with no reviewable changes (1)
- src/main/kotlin/com/weeth/domain/board/domain/entity/Post.kt
✅ Files skipped from review due to trivial changes (4)
- src/main/kotlin/com/weeth/domain/board/application/exception/DeletedBoardNotReorderableException.kt
- src/main/kotlin/com/weeth/domain/board/application/dto/response/PostListResponse.kt
- src/main/kotlin/com/weeth/domain/board/application/dto/response/PostDetailResponse.kt
- src/main/kotlin/com/weeth/domain/board/domain/repository/BoardPostCount.kt
🚧 Files skipped from review as they are similar to previous changes (8)
- src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt
- src/main/kotlin/com/weeth/domain/board/application/mapper/PostMapper.kt
- src/main/kotlin/com/weeth/domain/board/application/mapper/BoardMapper.kt
- src/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.kt
- src/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.kt
- src/main/kotlin/com/weeth/domain/board/presentation/PostController.kt
- src/main/kotlin/com/weeth/domain/board/presentation/BoardResponseCode.kt
- src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.kt
| @Query( | ||
| """ | ||
| SELECT new com.weeth.domain.board.domain.repository.BoardPostCount(p.board.id, COUNT(p)) | ||
| FROM Post p | ||
| WHERE p.board.id IN :boardIds | ||
| AND p.isDeleted = false | ||
| GROUP BY p.board.id | ||
| """, | ||
| ) | ||
| fun countActivePostsByBoardIds( | ||
| @Param("boardIds") boardIds: List<Long>, | ||
| ): List<BoardPostCount> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how countActivePostsByBoardIds is used and whether deleted board filtering is needed
# Find usages of countActivePostsByBoardIds
rg -n "countActivePostsByBoardIds" --type=kt -B 3 -A 5
# Check if deleted boards are included when calling this method
rg -n "findAllByClubIdOrderByDisplayOrderAscIdAsc" --type=kt -A 10Repository: Team-Weeth/weeth-server
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Find usages of countActivePostsByBoardIds with correct file type
rg -n "countActivePostsByBoardIds" --type=kotlin -B 3 -A 5
# Also search without type restriction to be safe
echo "=== Additional search without file type restriction ==="
rg -n "countActivePostsByBoardIds" -B 3 -A 5Repository: Team-Weeth/weeth-server
Length of output: 17258
🏁 Script executed:
#!/bin/bash
# Check if board.isDeleted filtering is used in other queries
rg -n "board\.isDeleted" --type=kotlin -B 2 -A 2
# Check PostRepository for similar queries
rg -n "SELECT.*FROM Post p" --type=kotlin -B 1 -A 10Repository: Team-Weeth/weeth-server
Length of output: 8322
🏁 Script executed:
#!/bin/bash
# Check Board entity to understand soft delete pattern
fd -e "kt" | xargs grep -l "class Board"
# Check if there's documentation or tests for this method
rg -n "countActivePostsByBoardIds\|BoardPostCount" --type=kotlinRepository: Team-Weeth/weeth-server
Length of output: 1378
countActivePostsByBoardIds 쿼리에 p.board.isDeleted 필터링 추가 필요
현재 쿼리는 p.isDeleted = false만 필터링하고 있으나, PostRepository의 다른 모든 post 조회 쿼리는 p.board.isDeleted = false도 함께 필터링하고 있습니다. 일관성을 위해 이 메서드도 동일하게 board 삭제 상태를 필터링해야 합니다.
권장 수정
SELECT new com.weeth.domain.board.domain.repository.BoardPostCount(p.board.id, COUNT(p))
FROM Post p
WHERE p.board.id IN :boardIds
AND p.isDeleted = false
AND p.board.isDeleted = false
GROUP BY p.board.id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.kt`
around lines 189 - 200, The query in countActivePostsByBoardIds (Post p) only
filters p.isDeleted = false but omits filtering out posts whose parent board is
deleted; update the JPQL in the countActivePostsByBoardIds method to also
include AND p.board.isDeleted = false so it mirrors other PostRepository queries
and groups by p.board.id as before.
There was a problem hiding this comment.
어드민은 삭제된 게시판의 잔여 게시글도 볼 수 잇어
There was a problem hiding this comment.
@soo0711 이해했습니다! countActivePostsByBoardIds는 어드민 전용 쿼리이므로, 삭제된 게시판의 잔여 게시글도 포함해서 보여주는 것이 의도된 동작이군요. 제 코멘트가 잘못된 지적이었습니다. 감사합니다.
✏️ Learnings added
Learnt from: soo0711
Repo: Team-Weeth/weeth-server PR: 37
File: src/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.kt:189-200
Timestamp: 2026-03-25T09:04:29.749Z
Learning: In `src/main/kotlin/com/weeth/domain/board/domain/repository/PostRepository.kt`, the `countActivePostsByBoardIds` query intentionally omits `p.board.isDeleted = false`. It is used exclusively in admin-facing queries (`findBoardDetailForAdmin`, `findAllBoardsForAdmin` in `GetBoardQueryService`), where admins are allowed to see post counts from deleted boards as well.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: hyxklee
Repo: Team-Weeth/weeth-server PR: 32
File: src/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.kt:60-66
Timestamp: 2026-03-23T06:19:04.286Z
Learning: In this codebase’s Kotlin/Spring soft-delete model (e.g., storing inactive/withdrawn rows instead of hard-deleting), repository queries must filter out inactive/soft-deleted rows when the caller expects only active entities. If a repository method (e.g., a JPQL `findAllBy...` query) does not include a `memberStatus`/active-status predicate, treat returned entities as including soft-deleted records and ensure the calling service either (a) filters by status before use or (b) can prove it’s safe under the soft-delete policy.
📌 Summary
게시판 기본 기능을 수정했습니다.
📝 Changes
What
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
ManageClubUseCase가BoardRepository를 직접 의존합니다.(아키텍처 규치상 어긋나지 않지만 공지사항 게시판의 생성 조건이 club 도메인에 노출되고, 초기 게시판 스펙이 바뀌면 club UseCase도 수정해야하는 단점이 있습니다. 다른 방법인 boardUsecase로 의존하면 현재 방법보다 단점이 더 많아서 일단 repo직접 의존으로 진행햇습니다!)
✅ Checklist
Summary by CodeRabbit
새로운 기능
개선사항