feat: [ALT-206] 관리자 약관 관리 시스템 구현#82
Merged
Merged
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (36)
📝 WalkthroughWalkthrough관리자용 약관 관리(도메인/유스케이스/컨트롤러/DTO/영속성)와 알림 수신 동의(엔티티/레포지토리/유스케이스/컨트롤러/NotificationService 필터링), 워크스페이스 사유 조회 API 추가 및 관련 테스트들, 그리고 .gitignore 규칙이 추가되었습니다. Changes약관 관리 시스템
알림 수신 동의 (사용자/매니저 API, 도메인, 레포, 서비스 통합)
사용자 생성/소셜 및 로그아웃 연계 변경 (동의 데이터 저장/사용)
관리자 워크스페이스 사유 조회 API
예외 처리 및 기타
Sequence DiagramsequenceDiagram
participant User as User/API Client
participant Controller as UserNotificationConsentController
participant UC_Get as GetNotificationConsent
participant QueryRepo as NotificationConsentQueryRepository
participant Response as NotificationConsentResponseDto
User->>Controller: GET /app/users/me/notification-consent
Controller->>UC_Get: execute(GetNotificationConsentCommand.from(user))
UC_Get->>QueryRepo: findByUser(user)
QueryRepo-->>UC_Get: Optional<NotificationConsent>
alt consent 없음
UC_Get-->>Controller: CustomException(NOT_FOUND)
Controller-->>User: 404
else consent 존재
UC_Get-->>Controller: GetNotificationConsentResult.from(entity)
Controller-->>User: 200 + NotificationConsentResponseDto
end
sequenceDiagram
participant Client as 시스템/서비스
participant NotifSvc as NotificationService
participant ConsentRepo as NotificationConsentQueryRepository
participant DeviceRepo as DeviceTokenRepository
participant FCM as FCM 클라이언트
Client->>NotifSvc: sendMultipleNotifications(request, recipients)
NotifSvc->>ConsentRepo: findByUsers(users)
ConsentRepo-->>NotifSvc: List<NotificationConsent>
NotifSvc->>DeviceRepo: findDeviceTokensForUsers(filteredUsers)
DeviceRepo-->>NotifSvc: device tokens
NotifSvc->>FCM: sendBatch(tokens for consenting users)
FCM-->>NotifSvc: result
NotifSvc->>NotificationRepo: save notifications for all users (record)
NotifSvc-->>Client: 결과
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.gitignore (1)
58-61: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
.gitignore의 패턴 범위를 의도대로 고정하세요.현재
AGENTS.md(슬래시 없음) 패턴은 리포지토리 어느 디렉토리에 있든 이름이AGENTS.md인 파일을 전부 무시할 수 있습니다. 만약 의도가 “루트의AGENTS.md만 무시”라면/AGENTS.md로 바꿔 의도를 명확히 하는 걸 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 58 - 61, 현재 `.gitignore`에 있는 패턴 `AGENTS.md`는 리포지토리 어디에 있든 같은 이름의 파일을 모두 무시하므로 의도대로 루트 파일만 무시하려면 패턴을 `/AGENTS.md`로 변경하십시오; `.gitignore`의 해당 항목(현재 `AGENTS.md`)을 찾아 루트 전용 패턴 `/AGENTS.md`로 바꿔 의도를 고정하세요.
🤖 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/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java`:
- Line 3: TermsQueryRepositoryImpl is coupled to an inbound API DTO
(TermsListFilterDto) which breaks layer boundaries; change the repository
implementation and its port to accept a domain/application filter type (e.g.,
create TermsFilterCriteria or TermsQueryCriteria in the application/domain
layer) instead of TermsListFilterDto, update the TermsQueryRepositoryImpl method
signatures to use that new criteria type, and move the mapping from
TermsListFilterDto → TermsFilterCriteria into the inbound layer
(controller/adapter) so the outbound adapter depends only on domain/application
types.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java`:
- Around line 3-7: The AdminGetTermsList use-case currently depends on
adapter/inbound DTOs and Spring Data types (remove imports
com.dreamteam.alter.adapter.inbound.admin.terms.* and any Spring Data
Page/PageRequest usage in AdminGetTermsList), which breaks layer boundaries;
refactor AdminGetTermsList to depend only on application-level
request/response/pagination models (create or use application package DTOs such
as an ApplicationTermsListRequest/Response and an application PaginatedResponse/
PageRequest model or a port interface), convert incoming adapter DTOs and Spring
Page/PageRequest at the adapter layer (map adapter.inbound.TermsListFilterDto
and AdminTermsListItemResponseDto <-> application models) and have
AdminGetTermsList accept and return those application models and call repository
ports that use domain/application pagination types instead of Spring Data types.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java`:
- Around line 32-35: The current publish flow in AdminPublishTerms
(termsQueryRepository.findPublishedByType(...).ifPresent(Terms::deprecate);
terms.publish();) is race-prone and can produce duplicate PUBLISHED rows for the
same type; fix by making the publish operation atomic: add a DB-level protection
(unique constraint on (type, status=PUBLISHED)) and change the service to run in
a single transaction that either acquires a pessimistic lock for the given type
(e.g., add a repository method like findByTypeForUpdate/typeLock and call it
before deprecating/publishing) or performs an atomic update query that
deprecates existing PUBLISHED rows and then inserts/updates the new Terms in one
DB roundtrip; also handle unique-constraint violations gracefully (retry or
return conflict). Ensure the changes touch AdminPublishTerms,
termsQueryRepository (add lock/atomic update method), and keep Terms::deprecate
and terms.publish calls inside the same transactional/locked block.
In `@src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java`:
- Around line 71-88: Enforce state-transition invariants inside the Terms entity
by validating current status in the methods update, publish, deprecate, and
delete: check Terms#status against allowed source states for each operation
(e.g., only allow update when not DELETED, only allow publish from DRAFT or
UPDATED, only allow deprecate from PUBLISHED, only allow delete from non-DELETED
states), and throw an appropriate runtime exception (e.g.,
IllegalStateException) with a clear message when the transition is invalid; also
ensure publish sets effectiveAt and other methods do not modify it incorrectly
so all transition logic and invariants are encapsulated in the Terms class.
- Around line 5-22: The Terms class currently mixes domain logic with
infrastructure annotations (jakarta.persistence, Spring auditing, Hibernate,
Lombok) and lacks state validation in its state-transition methods; extract a
pure domain model (class Terms in src/main/java/com/dreamteam/alter/domain/**)
with no infra imports and move all JPA/ORM mappings into a separate persistence
adapter/entity (e.g., TermsEntity in the outbound/adapter layer) that converts
to/from the domain Terms, and update repositories to use that adapter; also
harden domain methods publish, deprecate, delete, and update to validate current
state before changing it (use explicit checks in Terms.publish(),
Terms.deprecate(), Terms.delete(), Terms.update() and throw a domain-level
exception like IllegalStateException or a custom DomainException when
transitions are invalid) so business invariants are enforced at the domain
level.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.java`:
- Around line 3-8: The port AdminCreateTermsUseCase currently depends on adapter
DTO AdminCreateTermsRequestDto; change the port method to accept a domain-level
command (e.g., CreateTermsCommand) instead of the adapter DTO and keep
AdminActor as the actor parameter; create a new domain command class
CreateTermsCommand in the domain package (with the fields previously carried by
AdminCreateTermsRequestDto) and update all implementations of
AdminCreateTermsUseCase (and any callers in the adapter layer) to map
AdminCreateTermsRequestDto -> CreateTermsCommand before invoking execute,
ensuring the interface signature is Long execute(CreateTermsCommand command,
AdminActor actor).
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.java`:
- Around line 3-8: The port AdminGetTermsDetailUseCase currently depends on the
adapter DTO AdminTermsDetailResponseDto; change the port to return a pure domain
response model (e.g., create a TermsDetail or TermsDetailResponse in the domain
package) instead of AdminTermsDetailResponseDto, update the
AdminGetTermsDetailUseCase.execute signature to return that domain model, and
move any Swagger/HTTP-specific annotations out of the domain; then update the
inbound adapter that implements AdminGetTermsDetailUseCase to map the domain
model to AdminTermsDetailResponseDto. Apply the same refactor pattern to other
affected ports mentioned (AdminGetTermsListUseCase, AdminUpdateTermsUseCase) and
to the outbound port TermsQueryRepository so that domain ports and repositories
use domain models only and adapters handle DTO conversion.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.java`:
- Around line 3-11: The port AdminGetTermsListUseCase currently depends on
adapter/HTTP DTOs (AdminTermsListItemResponseDto, TermsListFilterDto,
PageRequestDto, PaginatedResponseDto) which couples the domain to infra; change
the interface signature of AdminGetTermsListUseCase.execute to accept and return
pure domain models (e.g., replace TermsListFilterDto+PageRequestDto with a
single AdminTermsListQuery or TermsListQuery value object and replace
PaginatedResponseDto<AdminTermsListItemResponseDto> with a domain TermsListPage
or PaginatedTerms domain model), keeping AdminActor as-is; then update the
inbound adapter/controller to map incoming HTTP DTOs to AdminTermsListQuery and
map the domain TermsListPage back to the adapter
PaginatedResponseDto/AdminTermsListItemResponseDto.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.java`:
- Around line 3-8: The inbound port AdminUpdateTermsUseCase currently depends on
the adapter DTO AdminUpdateTermsRequestDto; change the port to accept a
domain-level command/DTO (e.g., create an UpdateTermsCommand or
AdminUpdateTermsCommand in the domain/application package) and update the
execute signature to void execute(Long id, UpdateTermsCommand command,
AdminActor actor); then move mapping from AdminUpdateTermsRequestDto ->
UpdateTermsCommand into the adapter layer (where the adapter currently
constructs the port call) so the domain port no longer imports adapter.* types
and retains a clean contract.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.java`:
- Around line 3-14: The port TermsQueryRepository is leaking adapter and Spring
types: replace the adapter DTO TermsListFilterDto with a domain-only filter
model (e.g., create and use TermsListFilter in the domain package) and replace
Spring types Page and Pageable with domain pagination abstractions (e.g.,
DomainPage<Terms> and DomainPageable) so the interface has no infra
dependencies; also change findPublishedByType(String type) to use the domain
enum TermsType. Update the method signatures on TermsQueryRepository
(findByFilter and findPublishedByType) and remove imports of TermsListFilterDto,
Page, and Pageable, referencing the new domain types instead.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.java`:
- Around line 4-7: TermsRepository currently depends on Spring Data by extending
JpaRepository, coupling the domain port to infrastructure; change
TermsRepository to a pure domain port interface that declares only the needed
method signatures (e.g., findById, save, delete) and remove any Spring/JPA
imports, then create an outbound adapter in the infrastructure layer: define a
TermsJpaRepository that extends JpaRepository<Terms, Long> and implement
TermsRepository in a TermsRepositoryImpl (or adapter) that delegates to
TermsJpaRepository; ensure domain package contains no Spring/JPA imports and
that TermsRepository is only a clean contract.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java`:
- Line 21: Rename the test class AdminCreateTermsTest to the plural
AdminCreateTermsTests (and rename the file accordingly) and reorganize related
test cases into `@Nested` inner classes annotated with `@DisplayName` to group
scenarios; specifically, create descriptive nested classes (e.g., `@Nested`
`@DisplayName`("when input is valid") and `@Nested` `@DisplayName`("when validation
fails")) and move the relevant test methods from AdminCreateTermsTests into
those nested classes, ensuring imports for org.junit.jupiter.api.Nested and
DisplayName are present and any test method references to the original class are
updated.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.java`:
- Line 23: Rename the test class AdminGetTermsDetailTest to the plural form
AdminGetTermsDetailTests and restructure the flat test methods into `@Nested`
inner classes with `@DisplayName` annotations to group related scenarios (e.g.,
SuccessCases, ValidationErrors, PermissionChecks); update the class declaration
and move related test methods into appropriately named nested static classes so
each group has a `@DisplayName` and tests remain organized per team convention.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java`:
- Around line 29-30: Rename the test class AdminGetTermsListTest to follow the
plural Tests convention (e.g., AdminGetTermsListTests) and update all referenced
filenames/imports accordingly, and convert Mockito usages in this file (and the
other indicated test classes) to BDD style: replace any when(...) stubbing with
given(...) from org.mockito.BDDMockito and replace verify(...) calls with
then(mock).should(...) verifications; ensure method names and annotations remain
unchanged while updating static imports to use BDDMockito.given and
BDDMockito.then where necessary.
- Around line 45-46: The test currently stubs termsQueryRepository.findByFilter
with any(Pageable.class) which misses verifying the 1-based→0-based page-index
conversion; update AdminGetTermsListTest to assert that findByFilter is called
with a Pageable whose getPageNumber() equals requestedPage-1 (and correct
size/sort if relevant) by capturing or using an ArgumentCaptor/argThat against
findByFilter(TermsListFilterDto, Pageable), and add the same stronger
verification for the other case referenced (lines 54-55) so the conversion
regression is caught.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java`:
- Around line 23-24: Rename the test class AdminPublishTermsTest to follow the
plural naming convention (AdminPublishTermsTests) and update all Mockito
interactions in this class to BDD style: replace any when(...) or verify(...)
usages with given(...) for stubbing and then(...).should(...) for verifications
(apply this to test helper methods and each test case in
AdminPublishTermsTests); ensure imports switch to BDDMockito static methods
where needed so tests compile and follow the project's test conventions.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java`:
- Line 23: Rename the test class AdminUpdateTermsTest to follow the repository
convention (plural) e.g., AdminUpdateTermsTests, rename the file accordingly,
and refactor the test methods into `@Nested` inner classes that group related
scenarios with `@DisplayName` annotations (create nested classes like SuccessCases
and FailureCases or ScenarioX with `@Nested` and `@DisplayName`) so each scenario
group is clearly labeled; update any imports or references to the class and
ensure test runner annotations (e.g., `@Test`) remain on the methods inside the
nested classes.
---
Outside diff comments:
In @.gitignore:
- Around line 58-61: 현재 `.gitignore`에 있는 패턴 `AGENTS.md`는 리포지토리 어디에 있든 같은 이름의 파일을
모두 무시하므로 의도대로 루트 파일만 무시하려면 패턴을 `/AGENTS.md`로 변경하십시오; `.gitignore`의 해당 항목(현재
`AGENTS.md`)을 찾아 루트 전용 패턴 `/AGENTS.md`로 바꿔 의도를 고정하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dd86ad4-50f3-4c89-af57-7aba9151d4d7
📒 Files selected for processing (30)
.gitignoresrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminCreateTermsRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsDetailResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsListItemResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminUpdateTermsRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/TermsListFilterDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetail.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.javasrc/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.javasrc/main/java/com/dreamteam/alter/domain/terms/entity/Terms.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminPublishTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/type/TermsStatus.javasrc/main/java/com/dreamteam/alter/domain/terms/type/TermsType.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java
juny0955
approved these changes
May 4, 2026
ysw789
requested changes
May 4, 2026
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (14)
src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.java (1)
3-7: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 인바운드 포트가 어댑터 DTO에 직접 의존하고 있습니다.
AdminUpdateTermsRequestDto는 어댑터 DTO이므로 포트 시그니처에 노출되면 안 됩니다. 도메인 커맨드(예:UpdateTermsCommand(title, notionUrl, required))로 분리하고 어댑터에서 매핑하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "Port interfaces (inbound/outbound) are clean contracts with no implementation details."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.java` around lines 3 - 7, The inbound port AdminUpdateTermsUseCase must not depend on adapter DTOs: change the method signature in AdminUpdateTermsUseCase from void execute(Long id, AdminUpdateTermsRequestDto request) to void execute(Long id, UpdateTermsCommand command), introduce a simple domain command class UpdateTermsCommand with fields (title, notionUrl, required) in the domain package, and update the adapter/ controller layer to map AdminUpdateTermsRequestDto → UpdateTermsCommand before calling AdminUpdateTermsUseCase.execute; ensure no references to AdminUpdateTermsRequestDto remain in the domain package.src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.java (1)
3-7: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 인바운드 포트가 어댑터 응답 DTO를 반환합니다.
AdminTermsDetailResponseDto는 어댑터 레이어의 HTTP 응답 모델(Swagger/OpenAPI 어노테이션 포함)이므로, 도메인 포트가 이를 반환 타입으로 사용하는 순간 도메인이 HTTP 표현 계층에 결합됩니다. 도메인 응답 모델(예:TermsDetailprojection)을 정의하고 어댑터가 HTTP DTO로 변환하도록 분리하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "ZERO infrastructure dependencies (no Spring, no JPA annotations, no external libs)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.java` around lines 3 - 7, The inbound domain port AdminGetTermsDetailUseCase currently returns the adapter DTO AdminTermsDetailResponseDto which couples domain to HTTP layer; change the port's return type to a pure domain projection/entity (e.g., create a new TermsDetail or TermsDetailProjection class in the domain package) and update the interface method signature execute(Long id) to return that domain type; then update the admin adapter (the component that implements AdminGetTermsDetailUseCase and any controllers) to map from the domain TermsDetail to AdminTermsDetailResponseDto for HTTP responses, ensuring no infrastructure or Swagger/OpenAPI annotations appear in the domain package.src/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.java (1)
23-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win예외 메시지에 사유 정보를 포함해 주세요.
new CustomException(ErrorCode.NOT_FOUND)/new CustomException(ErrorCode.CONFLICT)만으로는 운영 로그/디버깅에서 어떤 자원·어떤 상태 위반인지 식별이 어렵습니다. 적어도 식별자(id)와 위반 사유(예: 현재 상태)를 포함하도록 메시지를 추가하시는 것이 좋습니다.- Terms terms = termsRepository.findById(id) - .orElseThrow(() -> new CustomException(ErrorCode.NOT_FOUND)); + Terms terms = termsRepository.findById(id) + .orElseThrow(() -> new CustomException(ErrorCode.NOT_FOUND, "Terms not found. id=" + id)); if (terms.getStatus() != TermsStatus.DRAFT) { - throw new CustomException(ErrorCode.CONFLICT); + throw new CustomException(ErrorCode.CONFLICT, "DRAFT 상태에서만 수정 가능. 현재 상태=" + terms.getStatus()); }
CustomException이 message-suffix 생성자를 지원하지 않는다면 그에 맞춰 시그니처를 조정해 주세요.#!/bin/bash # CustomException 생성자 시그니처 확인 ast-grep --pattern 'class CustomException $$$ { $$$ }' rg -nP -C2 'class\s+CustomException\b' --type=java🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.java` around lines 23 - 28, The exceptions thrown in AdminUpdateTerms (the block using termsRepository.findById(id) and the TermsStatus check) currently use only ErrorCode and lack contextual details; update the two throws to include identifying info (e.g., the terms id and current status) in the CustomException message so logs show which resource and why it failed (for NOT_FOUND include the id, for CONFLICT include id plus terms.getStatus()). If CustomException does not support a message-suffix constructor, add or overload a constructor on CustomException to accept ErrorCode plus a message (or cause) and use that new constructor from AdminUpdateTerms.src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.java (1)
3-10: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 인바운드 포트가 어댑터 DTO 및 HTTP 페이지네이션 모델에 직접 의존하고 있습니다.
AdminTermsListItemResponseDto,TermsListFilterDto,PageRequestDto,PaginatedResponseDto는 모두 어댑터 레이어 DTO이며 일부는 Spring/OpenAPI 어노테이션을 포함합니다. 포트 시그니처를 도메인 쿼리/페이지 모델(예:TermsListQuery,Page<TermsListItem>)로 변경해 계층 경계를 회복하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "Port interfaces (inbound/outbound) are clean contracts with no implementation details."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.java` around lines 3 - 10, The inbound port AdminGetTermsListUseCase currently depends on adapter DTOs (AdminTermsListItemResponseDto, TermsListFilterDto, PageRequestDto, PaginatedResponseDto); change the execute signature to use domain-level query/page models (e.g., replace TermsListFilterDto with TermsListQuery and PageRequestDto/PaginatedResponseDto with a domain Page<TermsListItem> or Page<TermsListItemDomain> and AdminTermsListItemResponseDto with TermsListItemDomain) so the port has no adapter/spring/OpenAPI types; update the interface Execute method in AdminGetTermsListUseCase to the new domain types and ensure adapter layers map between adapter DTOs and the newly introduced domain models when wiring the use case.src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java (2)
5-22: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 엔티티가 JPA/Hibernate/Spring/Lombok 인프라에 직접 의존합니다.
jakarta.persistence.*,org.hibernate.annotations.*,org.springframework.data.*,lombok.*이 도메인 엔티티에 산재되어 가이드라인(ZERO infrastructure dependencies)을 위반합니다. 순수 도메인 모델Terms와 영속성 어댑터의TermsJpaEntity를 분리하고,TermsRepositoryImpl이 둘 사이를 매핑하도록 리팩터링하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "ZERO infrastructure dependencies (no Spring, no JPA annotations, no external libs)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java` around lines 5 - 22, The Terms class currently contains JPA/Spring/Lombok annotations; extract all persistence and framework concerns into a new TermsJpaEntity (create in the persistence/adapter package) annotated with `@Entity`, `@Table`, `@DynamicUpdate`, auditing annotations and the jakarta.persistence/org.hibernate/spring imports; make Terms a pure domain POJO (no jakarta.persistence, org.springframework, org.hibernate or lombok infrastructure annotations) with simple constructors/getters and domain logic only. Update TermsRepositoryImpl to map between Terms and TermsJpaEntity on read/write (convert entity -> domain on load, domain -> entity on save), move CreatedDate/LastModifiedDate and any audit fields to TermsJpaEntity, and ensure builder/constructor usage is adjusted so domain creation doesn’t depend on JPA or Lombok infrastructure.
73-90: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win상태 전이 불변식이 엔티티 내부에서 강제되지 않습니다.
update,publish,deprecate,delete모두 사전 상태 검증이 없어 잘못된 호출 경로(예:PUBLISHED상태에서update,DELETED상태에서publish)에서 비정상 전이가 발생할 수 있습니다. 현재는 use-case(AdminUpdateTerms)에서만 DRAFT 체크를 하지만, 다른 호출자가 추가될 때 실수가 누적됩니다. 엔티티가 자기 불변식을 보호하도록 옮기는 것이 안전합니다.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "Entities encapsulate business rules and invariants."♻️ 제안: 상태 전이 검증을 엔티티에 캡슐화
public void update(String title, String notionUrl, boolean required) { + if (this.status != TermsStatus.DRAFT) { + throw new IllegalStateException("DRAFT 상태인 약관만 수정할 수 있습니다. 현재 상태: " + this.status); + } this.title = title; this.notionUrl = notionUrl; this.required = required; } public void publish() { + if (this.status != TermsStatus.DRAFT) { + throw new IllegalStateException("DRAFT 상태에서만 게시할 수 있습니다. 현재 상태: " + this.status); + } this.status = TermsStatus.PUBLISHED; this.effectiveAt = LocalDateTime.now(); } public void deprecate() { + if (this.status != TermsStatus.PUBLISHED) { + throw new IllegalStateException("PUBLISHED 상태에서만 폐기할 수 있습니다. 현재 상태: " + this.status); + } this.status = TermsStatus.DEPRECATED; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java` around lines 73 - 90, The entity currently allows illegal state transitions because update, publish, deprecate, and delete do not validate the current TermsStatus; move the transition checks into the Terms entity (class Terms) by adding precondition checks at the start of each method (update, publish, deprecate, delete) that validate allowed current statuses (e.g., update only from DRAFT, publish only from DRAFT, deprecate only from PUBLISHED, disallow publish/update if status is DELETED, etc.) and throw a clear runtime/domain exception (e.g., IllegalStateException or a domain-specific exception) when an invalid transition is attempted so the entity enforces its invariants regardless of caller.src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.java (1)
6-17: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 아웃바운드 포트가 Spring Data JPA에 직접 종속되어 있습니다.
JpaRepository상속,@Lock,@Query,@Param이 도메인 포트에 위치하고 있어 도메인이 ORM 기술에 잠깁니다. 프로젝트의 다른 도메인 포트(UserRepository,AuthorizationRepository등)는 순수 인터페이스이고 어댑터의*RepositoryImpl이 Spring Data를 다루는 패턴을 따르고 있으므로,TermsRepository도 동일 패턴으로 정리하고TermsJpaRepository(어댑터)에서 잠금 쿼리를 구현하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "ZERO infrastructure dependencies (no Spring, no JPA annotations, no external libs)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.java` around lines 6 - 17, The domain port TermsRepository currently depends on Spring Data/JPA annotations; change TermsRepository into a pure domain interface (no extends JpaRepository, no `@Lock/`@Query/@Param imports) with a method signature like Optional<Terms> findPublishedByTypeWithLock(TermsType type). Then create a new adapter-side interface/class (e.g., TermsJpaRepository) that extends JpaRepository<Terms, Long> and contains the JPA-specific `@Lock/`@Query/@Param definition (the existing query that selects PUBLISHED by type). Finally implement the domain port in the adapter by wiring/delegating from your TermsRepositoryImpl to TermsJpaRepository (call the JPA method from findPublishedByTypeWithLock) so infrastructure details remain outside the domain layer.src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.java (1)
3-7: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift도메인 인바운드 포트가 어댑터 DTO에 직접 의존하고 있습니다.
AdminCreateTermsRequestDto(com.dreamteam.alter.adapter.inbound.admin.terms.dto)는 어댑터 레이어 DTO이며, 도메인 포트가 이를 시그니처에 직접 노출하면 도메인이 인바운드 어댑터 구현 세부에 결합됩니다. 도메인 커맨드 객체(예:CreateTermsCommand)를 도메인 패키지에 정의하고, 어댑터에서 매핑하도록 변경하세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/domain/**: "Port interfaces (inbound/outbound) are clean contracts with no implementation details."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.java` around lines 3 - 7, The AdminCreateTermsUseCase currently depends on the adapter DTO AdminCreateTermsRequestDto; create a domain-level command (e.g., CreateTermsCommand) in the domain package and change the AdminCreateTermsUseCase.execute signature to accept CreateTermsCommand instead of AdminCreateTermsRequestDto, remove the adapter DTO import from the port, and update the inbound adapter(s) to map AdminCreateTermsRequestDto → CreateTermsCommand before calling AdminCreateTermsUseCase.execute.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java (1)
22-22: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 컨벤션(클래스명
*Tests, BDD-style Mockito) 미준수가 그대로입니다.
AdminPublishTermsTest→AdminPublishTermsTests로 변경하고,when(...).thenReturn(...)/verify(...)호출을given(...).willReturn(...)/then(...).should(...)(BDDMockito)로 변환해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural 'Tests').", "BDD-style Mockito: given() for setup, then().should() for verification."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` at line 22, Rename the test class AdminPublishTermsTest to AdminPublishTermsTests and update all Mockito usage to BDD style: replace when(...).thenReturn(...) with given(...).willReturn(...), and replace verify(...).*(...) with then(...).should().*(...); also update static imports to use org.mockito.BDDMockito.given and org.mockito.BDDMockito.then (and remove or adjust any org.mockito.Mockito.when/verify imports) so the class and mocking follow the project's Tests naming and BDDMockito conventions.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java (2)
43-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win페이지 변환(1-based → 0-based) 검증이 여전히 누락되어 있습니다.
핵심 변환 로직(
pageRequest.page() - 1)이 회귀 시 잡히지 않습니다.ArgumentCaptor<Pageable>로getPageNumber() == 0,getPageSize() == 10을 명시적으로 검증해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java` around lines 43 - 52, Add an assertion that verifies the 1-based→0-based page conversion by capturing the Pageable passed into termsQueryRepository.findByFilter: use an ArgumentCaptor<Pageable> in AdminGetTermsListTest to capture the argument when adminGetTermsList.execute(filter, pageRequest) is called, then assert captured.getValue().getPageNumber() == 0 and captured.getValue().getPageSize() == 10 (reflecting pageRequest.page() - 1 and the requested size) so the core conversion logic (pageRequest.page() - 1) is covered.
29-29: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 컨벤션(클래스명
*Tests, BDD-style Mockito) 미준수가 그대로 유지됩니다.
AdminGetTermsListTest→AdminGetTermsListTests로 변경하고,when(...).thenReturn(...)/verify(...)호출은given(...).willReturn(...)/then(...).should(...)(BDDMockito)로 변환해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural 'Tests').", "BDD-style Mockito: given() for setup, then().should() for verification."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java` at line 29, Rename the test class AdminGetTermsListTest to AdminGetTermsListTests and update any references/imports accordingly; replace Mockito imperative style calls in this class—change when(...).thenReturn(...) usages to BDDMockito given(...).willReturn(...) and change verify(...) calls to BDDMockito then(mock).should(...) (or then(...).should(...)) for verifications, ensuring static imports for given and then are added/adjusted; run tests to confirm no broken references.src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java (1)
3-3: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift아웃바운드 어댑터가 인바운드 DTO에 결합된 문제는 여전히 남아 있습니다.
TermsListFilterDto(adapter.inbound) 임포트가 그대로 유지되어 있습니다. 이전 리뷰의 지적대로 application/domain 레이어에 별도 filter criteria 타입을 두고 인바운드 컨트롤러에서 매핑하는 구조가 안전합니다.As per coding guidelines,
src/main/java/com/dreamteam/alter/adapter/**: "DTOs are properly separated from domain entities."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java` at line 3, TermsQueryRepositoryImpl still imports the inbound DTO TermsListFilterDto, so remove that coupling: introduce/use a domain-level filter type (e.g., TermsFilterCriteria in application/domain/terms) and change TermsQueryRepositoryImpl method signatures and internal references from TermsListFilterDto to that domain filter; keep the mapping of inbound TermsListFilterDto -> TermsFilterCriteria inside the inbound controller (adapter.inbound) instead of in the outbound adapter. Ensure all references to TermsListFilterDto in TermsQueryRepositoryImpl are replaced and the import removed.src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java (2)
3-15: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift유스케이스가 어댑터 DTO와 Spring Data 타입에 직접 의존하는 문제가 그대로입니다.
AdminTermsListItemResponseDto/TermsListFilterDto(adapter.inbound) 및Page/PageRequest(Spring Data) 의존이 유지되어 있어, application 레이어가 인프라/어댑터에 끌려가는 구조입니다. 입력은 application 전용 criteria/page 모델로 받고, 매핑/페이징 변환은 인바운드/아웃바운드 어댑터에서 수행하도록 분리하는 것이 권장됩니다.As per coding guidelines,
src/main/java/com/dreamteam/alter/application/**: "No direct infrastructure dependencies (no Spring Data, no HTTP clients)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java` around lines 3 - 15, AdminGetTermsList currently depends on adapter DTOs (AdminTermsListItemResponseDto, TermsListFilterDto) and Spring Data types (Page, PageRequest, PaginatedResponseDto) which leaks infrastructure into the application layer; change the use case and repository contracts to accept and return application-level models (e.g., ApplicationTermsFilter, ApplicationTermsItem, ApplicationPageRequest/ApplicationPageResponse) by updating AdminGetTermsListUseCase and TermsQueryRepository signatures, remove imports of TermsListFilterDto, AdminTermsListItemResponseDto, Page, PageRequest, PaginatedResponseDto from AdminGetTermsList.java, and move mapping/paging conversion logic into the inbound adapter (map inbound TermsListFilterDto/PageRequestDto -> ApplicationPageRequest/ApplicationTermsFilter) and outbound adapter (map repository Page -> ApplicationPageResponse -> PaginatedResponseDto) so the application layer only uses its own POJOs and not Spring Data or adapter DTOs.
34-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
page.getTotalElements()의long → int다운캐스팅이 그대로 남아 있습니다.오버플로우 시 조용히 잘못된 값이 노출되지 않도록
Math.toIntExact()로 감싸거나,PageResponseDto가long을 받도록 시그니처를 수정하는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java` at line 34, The code in AdminGetTermsList uses a plain cast when building PageResponseDto: PageResponseDto.of(pageRequest, (int) page.getTotalElements()), which silently overflows; change this to use Math.toIntExact(page.getTotalElements()) to force an exception on overflow, or alternatively update PageResponseDto.of's signature (and PageResponseDto factory) to accept a long total (adjust PageResponseDto class and its of(...) method) so no downcast is needed; modify either the call site (AdminGetTermsList) and imports to use Math.toIntExact or refactor PageResponseDto.of and its consumers to take long.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java`:
- Around line 66-68: notDeleted() currently always excludes DELETED, which
conflicts with admin findByFilter()/statusCondition(filter.getStatus()) when the
caller requests status=DELETED; change the query logic so notDeleted() is
applied conditionally: in findByFilter() (where
statusCondition(filter.getStatus()) is composed) only add
terms.status.ne(TermsStatus.DELETED) when the incoming filter has no explicit
status (or when the explicit status is not DELETED), otherwise skip/not include
notDeleted(); locate and update the composition that calls notDeleted(), plus
adjust any helper statusCondition(...) usage to respect explicit DELETED
requests.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java`:
- Line 28: PageRequest.of(...) can throw IllegalArgumentException for page < 1
because PageRequestDto lacks validation; add `@Min`(1) (and optionally `@NotNull`)
to the PageRequestDto page field to enforce page >= 1, and update the controller
endpoint that accepts PageRequestDto to annotate the parameter with `@Valid` (and
ensure `@Validated` on the controller if needed) so DTO-level validation runs
before AdminGetTermsList constructs PageRequest in AdminGetTermsList.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java`:
- Around line 29-32: In AdminPublishTerms, wrap the save/publish flow (around
termsRepository.findPublishedByTypeWithLock(...) and terms.publish()) in a
try-catch that catches org.springframework.dao.DataIntegrityViolationException
and converts it into the same domain conflict exception used elsewhere (map to
ErrorCode.CONFLICT / the application's ConflictException), mirroring the pattern
used in UpdateNickname and CreateUserFavoritePosting; this ensures the initial
concurrent-publish loser that violates a DB unique constraint is translated to a
controlled conflict error instead of propagating a 500.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.java`:
- Around line 26-30: Move the "status != DRAFT" invariant into the domain: add
validation inside the Terms entity's update(...) method to check that
this.status == TermsStatus.DRAFT and throw a domain-specific exception (e.g.,
TermsStateException or IllegalDomainState) when violated; then simplify
AdminUpdateTerms to call terms.update(request.getTitle(),
request.getNotionUrl(), request.isRequired()) and catch the domain exception,
mapping it to new CustomException(ErrorCode.CONFLICT) so the use-case only
translates domain errors rather than enforcing the invariant itself.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.java`:
- Around line 16-17: The JPQL in TermsRepository.findPublishedByTypeWithLock
uses a string literal for enum comparison (t.status = 'PUBLISHED'); change it to
bind the enum as a parameter instead to avoid relying on EnumType.STRING
behavior — update the query to use "t.status = :status" and add a parameter
(e.g., TermsStatus status) or overload the method to accept
TermsStatus.PUBLISHED when calling; ensure the method signature and `@Query`
reference the bound parameter (retain `@Param` for "type" and add `@Param`("status")
for the enum) and be mindful that the Terms entity has `@SQLRestriction`("status
!= 'DELETED'") which will still apply.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java`:
- Line 73: The assertion uses a brittle string literal "SERVICE"; change it to
assert against the enum to make intent explicit and robust—update the assertion
in AdminGetTermsListTest to compare result.data().get(0).getType() to
TermsType.SERVICE.name() or, if getType() returns an enum, compare it directly
to TermsType.SERVICE so the test won't break if serialization/mapping changes.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java`:
- Around line 47-63: The test only asserts state changes but doesn't verify the
interaction that the code fetched the locked published terms and persisted the
deprecation; update AdminPublishTermsTest to add Mockito interaction assertions:
verify that termsRepository.findPublishedByTypeWithLock(TermsType.SERVICE) was
called, and verify that termsRepository.save(...) was invoked for the previously
published Terms instance with its status set to TermsStatus.DEPRECATED (use
verify with argThat to check the id/status or a spy on existingTerms), and
optionally verify a save call for the newly published Terms after
adminPublishTerms.execute(2L).
- Around line 77-104: The tests PUBLISHED_상태_약관_게시_시도시_TERMS_NOT_PUBLISHABLE and
DEPRECATED_상태_약관_게시_시도시_TERMS_NOT_PUBLISHABLE must also assert that no lock
query is performed; after asserting the exception from
adminPublishTerms.execute(1L) add verification that
termsRepository.findPublishedByTypeWithLock(...) was never called (use
verify(termsRepository, never()).findPublishedByTypeWithLock(any()) or
equivalent) to ensure state validation happens before attempting a DB lock.
---
Duplicate comments:
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java`:
- Line 3: TermsQueryRepositoryImpl still imports the inbound DTO
TermsListFilterDto, so remove that coupling: introduce/use a domain-level filter
type (e.g., TermsFilterCriteria in application/domain/terms) and change
TermsQueryRepositoryImpl method signatures and internal references from
TermsListFilterDto to that domain filter; keep the mapping of inbound
TermsListFilterDto -> TermsFilterCriteria inside the inbound controller
(adapter.inbound) instead of in the outbound adapter. Ensure all references to
TermsListFilterDto in TermsQueryRepositoryImpl are replaced and the import
removed.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java`:
- Around line 3-15: AdminGetTermsList currently depends on adapter DTOs
(AdminTermsListItemResponseDto, TermsListFilterDto) and Spring Data types (Page,
PageRequest, PaginatedResponseDto) which leaks infrastructure into the
application layer; change the use case and repository contracts to accept and
return application-level models (e.g., ApplicationTermsFilter,
ApplicationTermsItem, ApplicationPageRequest/ApplicationPageResponse) by
updating AdminGetTermsListUseCase and TermsQueryRepository signatures, remove
imports of TermsListFilterDto, AdminTermsListItemResponseDto, Page, PageRequest,
PaginatedResponseDto from AdminGetTermsList.java, and move mapping/paging
conversion logic into the inbound adapter (map inbound
TermsListFilterDto/PageRequestDto ->
ApplicationPageRequest/ApplicationTermsFilter) and outbound adapter (map
repository Page -> ApplicationPageResponse -> PaginatedResponseDto) so the
application layer only uses its own POJOs and not Spring Data or adapter DTOs.
- Line 34: The code in AdminGetTermsList uses a plain cast when building
PageResponseDto: PageResponseDto.of(pageRequest, (int) page.getTotalElements()),
which silently overflows; change this to use
Math.toIntExact(page.getTotalElements()) to force an exception on overflow, or
alternatively update PageResponseDto.of's signature (and PageResponseDto
factory) to accept a long total (adjust PageResponseDto class and its of(...)
method) so no downcast is needed; modify either the call site
(AdminGetTermsList) and imports to use Math.toIntExact or refactor
PageResponseDto.of and its consumers to take long.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.java`:
- Around line 23-28: The exceptions thrown in AdminUpdateTerms (the block using
termsRepository.findById(id) and the TermsStatus check) currently use only
ErrorCode and lack contextual details; update the two throws to include
identifying info (e.g., the terms id and current status) in the CustomException
message so logs show which resource and why it failed (for NOT_FOUND include the
id, for CONFLICT include id plus terms.getStatus()). If CustomException does not
support a message-suffix constructor, add or overload a constructor on
CustomException to accept ErrorCode plus a message (or cause) and use that new
constructor from AdminUpdateTerms.
In `@src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java`:
- Around line 5-22: The Terms class currently contains JPA/Spring/Lombok
annotations; extract all persistence and framework concerns into a new
TermsJpaEntity (create in the persistence/adapter package) annotated with
`@Entity`, `@Table`, `@DynamicUpdate`, auditing annotations and the
jakarta.persistence/org.hibernate/spring imports; make Terms a pure domain POJO
(no jakarta.persistence, org.springframework, org.hibernate or lombok
infrastructure annotations) with simple constructors/getters and domain logic
only. Update TermsRepositoryImpl to map between Terms and TermsJpaEntity on
read/write (convert entity -> domain on load, domain -> entity on save), move
CreatedDate/LastModifiedDate and any audit fields to TermsJpaEntity, and ensure
builder/constructor usage is adjusted so domain creation doesn’t depend on JPA
or Lombok infrastructure.
- Around line 73-90: The entity currently allows illegal state transitions
because update, publish, deprecate, and delete do not validate the current
TermsStatus; move the transition checks into the Terms entity (class Terms) by
adding precondition checks at the start of each method (update, publish,
deprecate, delete) that validate allowed current statuses (e.g., update only
from DRAFT, publish only from DRAFT, deprecate only from PUBLISHED, disallow
publish/update if status is DELETED, etc.) and throw a clear runtime/domain
exception (e.g., IllegalStateException or a domain-specific exception) when an
invalid transition is attempted so the entity enforces its invariants regardless
of caller.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.java`:
- Around line 3-7: The AdminCreateTermsUseCase currently depends on the adapter
DTO AdminCreateTermsRequestDto; create a domain-level command (e.g.,
CreateTermsCommand) in the domain package and change the
AdminCreateTermsUseCase.execute signature to accept CreateTermsCommand instead
of AdminCreateTermsRequestDto, remove the adapter DTO import from the port, and
update the inbound adapter(s) to map AdminCreateTermsRequestDto →
CreateTermsCommand before calling AdminCreateTermsUseCase.execute.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.java`:
- Around line 3-7: The inbound domain port AdminGetTermsDetailUseCase currently
returns the adapter DTO AdminTermsDetailResponseDto which couples domain to HTTP
layer; change the port's return type to a pure domain projection/entity (e.g.,
create a new TermsDetail or TermsDetailProjection class in the domain package)
and update the interface method signature execute(Long id) to return that domain
type; then update the admin adapter (the component that implements
AdminGetTermsDetailUseCase and any controllers) to map from the domain
TermsDetail to AdminTermsDetailResponseDto for HTTP responses, ensuring no
infrastructure or Swagger/OpenAPI annotations appear in the domain package.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.java`:
- Around line 3-10: The inbound port AdminGetTermsListUseCase currently depends
on adapter DTOs (AdminTermsListItemResponseDto, TermsListFilterDto,
PageRequestDto, PaginatedResponseDto); change the execute signature to use
domain-level query/page models (e.g., replace TermsListFilterDto with
TermsListQuery and PageRequestDto/PaginatedResponseDto with a domain
Page<TermsListItem> or Page<TermsListItemDomain> and
AdminTermsListItemResponseDto with TermsListItemDomain) so the port has no
adapter/spring/OpenAPI types; update the interface Execute method in
AdminGetTermsListUseCase to the new domain types and ensure adapter layers map
between adapter DTOs and the newly introduced domain models when wiring the use
case.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.java`:
- Around line 3-7: The inbound port AdminUpdateTermsUseCase must not depend on
adapter DTOs: change the method signature in AdminUpdateTermsUseCase from void
execute(Long id, AdminUpdateTermsRequestDto request) to void execute(Long id,
UpdateTermsCommand command), introduce a simple domain command class
UpdateTermsCommand with fields (title, notionUrl, required) in the domain
package, and update the adapter/ controller layer to map
AdminUpdateTermsRequestDto → UpdateTermsCommand before calling
AdminUpdateTermsUseCase.execute; ensure no references to
AdminUpdateTermsRequestDto remain in the domain package.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.java`:
- Around line 6-17: The domain port TermsRepository currently depends on Spring
Data/JPA annotations; change TermsRepository into a pure domain interface (no
extends JpaRepository, no `@Lock/`@Query/@Param imports) with a method signature
like Optional<Terms> findPublishedByTypeWithLock(TermsType type). Then create a
new adapter-side interface/class (e.g., TermsJpaRepository) that extends
JpaRepository<Terms, Long> and contains the JPA-specific `@Lock/`@Query/@Param
definition (the existing query that selects PUBLISHED by type). Finally
implement the domain port in the adapter by wiring/delegating from your
TermsRepositoryImpl to TermsJpaRepository (call the JPA method from
findPublishedByTypeWithLock) so infrastructure details remain outside the domain
layer.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java`:
- Around line 43-52: Add an assertion that verifies the 1-based→0-based page
conversion by capturing the Pageable passed into
termsQueryRepository.findByFilter: use an ArgumentCaptor<Pageable> in
AdminGetTermsListTest to capture the argument when
adminGetTermsList.execute(filter, pageRequest) is called, then assert
captured.getValue().getPageNumber() == 0 and captured.getValue().getPageSize()
== 10 (reflecting pageRequest.page() - 1 and the requested size) so the core
conversion logic (pageRequest.page() - 1) is covered.
- Line 29: Rename the test class AdminGetTermsListTest to AdminGetTermsListTests
and update any references/imports accordingly; replace Mockito imperative style
calls in this class—change when(...).thenReturn(...) usages to BDDMockito
given(...).willReturn(...) and change verify(...) calls to BDDMockito
then(mock).should(...) (or then(...).should(...)) for verifications, ensuring
static imports for given and then are added/adjusted; run tests to confirm no
broken references.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java`:
- Line 22: Rename the test class AdminPublishTermsTest to AdminPublishTermsTests
and update all Mockito usage to BDD style: replace when(...).thenReturn(...)
with given(...).willReturn(...), and replace verify(...).*(...) with
then(...).should().*(...); also update static imports to use
org.mockito.BDDMockito.given and org.mockito.BDDMockito.then (and remove or
adjust any org.mockito.Mockito.when/verify imports) so the class and mocking
follow the project's Tests naming and BDDMockito conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6982fe5a-28e7-4ae7-a2f2-95e956ee892d
📒 Files selected for processing (20)
src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetail.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.javasrc/main/java/com/dreamteam/alter/domain/terms/entity/Terms.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminPublishTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java (1)
21-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 클래스명 복수형 +
@Nested그룹화 미적용
AdminCreateTermsTest→AdminCreateTermsTests, 성공/실패 시나리오를@Nested@DisplayName`` 클래스로 분리해야 합니다.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural 'Tests')." and "@nested classes group related tests with@DisplayName."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java` at line 21, Rename the test class AdminCreateTermsTest to AdminCreateTermsTests and refactor its test methods into grouped `@Nested` inner classes with `@DisplayName` annotations to separate success and failure scenarios; locate the top-level class declaration (class AdminCreateTermsTest) and change the name to AdminCreateTermsTests, then create two inner static classes (e.g., `@Nested` `@DisplayName`("Success cases") class Success and `@Nested` `@DisplayName`("Failure cases") class Failure) to host respective test methods, moving or updating existing test methods and their setup/utility methods into the appropriate nested group.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java (3)
22-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 클래스명 복수형 + BDD 스타일 미적용
클래스명이
AdminPublishTermsTest→AdminPublishTermsTests이어야 하고,when()/verify()대신BDDMockito.given()/then().should()를 사용해야 합니다.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural 'Tests')." and "BDD-style Mockito: given() for setup, then().should() for verification."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` around lines 22 - 23, Rename the test class AdminPublishTermsTest to the plural AdminPublishTermsTests and replace Mockito imperative calls: change all when(...) usages to BDDMockito.given(...) for stubbing and change verify(...) assertions to BDD style then(mock).should() calls; update any static imports and references to the class name and ensure imports include org.mockito.BDDMockito and remove/adjust former when/verify imports as needed.
81-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
CONFLICT발생 시 lock 조회 미호출 검증 누락상태 불일치로 예외가 먼저 터져야 lock 쿼리가 발생하지 않음을 보증하는
verify(termsRepository, never()).findPublishedByTypeWithLock(any())가 빠져 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` around lines 81 - 110, The tests in AdminPublishTermsTest (publishTerms_throwsConflict_whenStatusIsPublished and publishTerms_throwsConflict_whenStatusIsDeprecated) are missing verification that the exclusive lock query is not invoked when a CONFLICT is thrown; after the assertThatThrownBy(...) assertion add a verification asserting termsRepository.findPublishedByTypeWithLock(...) was never called (use verify(termsRepository, never()).findPublishedByTypeWithLock(any()) or equivalent) so the lock lookup is not executed on early-exit error paths.
49-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDeprecate 흐름에 저장소 인터랙션 검증 누락
기존
PUBLISHED약관이DEPRECATED로 전환되는 경로에서findById(2L)및findPublishedByTypeWithLock()호출 여부를verify로 보증해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` around lines 49 - 66, Add interaction verifications for the deprecate flow: after calling adminPublishTerms.execute(2L) add verify assertions to ensure termsRepository.findById(2L) and termsRepository.findPublishedByTypeWithLock(TermsType.SERVICE) were invoked, and verify that the repository persisted the state changes (e.g., verify(termsRepository).save(existingTerms) and verify(termsRepository).save(newTerms) or equivalent save/saveAll calls) so the test guarantees the repository interactions for deprecating the previous PUBLISHED term and publishing the new one.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java`:
- Around line 34-39: AdminTermsController is missing the Lombok `@Getter` required
by our controller guideline; add the `@Getter` annotation to the
AdminTermsController class declaration (public class AdminTermsController
implements AdminTermsControllerSpec) and import lombok.Getter, keeping the
existing `@RequiredArgsConstructor` and ensuring no `@Setter` is added.
- Line 71: In AdminTermsController, avoid direct cast of the long total to int;
replace the direct cast used in the call to PageResponseDto.of(pageRequest,
(int) total) with Math.toIntExact(total) so overflow will throw an
ArithmeticException instead of producing a negative value — update the call to
PageResponseDto.of(pageRequest, Math.toIntExact(total)) and ensure the
surrounding method (in AdminTermsController) either lets the exception propagate
or handles it appropriately.
- Around line 41-54: Remove the problematic `@Resource`(name="...") annotations
from the final fields in AdminTermsController (adminGetTermsList,
adminCreateTerms, adminGetTermsDetail, adminUpdateTerms, adminPublishTerms) and
keep the fields as final so Lombok's `@RequiredArgsConstructor` can perform
constructor injection by type; this ensures you rely on pure constructor
injection and avoid misleading name-based qualifiers that won't be propagated
into the generated constructor.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsControllerSpec.java`:
- Around line 58-114: The `@ApiResponse` annotations in AdminTermsControllerSpec
incorrectly document all error examples as responseCode "400"; update the
responseCode values to match the actual ErrorCode statuses: change the examples
named "존재하지 않는 약관" (B019) to responseCode "404" and change the examples named
"수정 불가 상태 (DRAFT가 아님)" / "게시 불가 상태 (DRAFT가 아님)" (B020) to responseCode "409" so
the `@ApiResponses` for getTermsDetail, updateTerms and the publish operation
align with the error codes returned by GlobalExceptionHandler/CustomException
(use e.getErrorCode().getStatus()).
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java`:
- Around line 66-77: findPublishedByType 현재 fetchFirst()가 ORDER BY 없이 호출되어 다수
PUBLISHED 레코드가 있을 경우 비결정적 결과를 반환하므로,
queryFactory.selectFrom(terms).where(terms.type.eq(type),
terms.status.eq(TermsStatus.PUBLISHED)).fetchFirst() 호출에 최신 버전이 우선되도록 정렬을 추가하세요;
예를 들어 TermsQueryRepositoryImpl의 findPublishedByType 메서드에서 terms를 기준으로
orderBy(terms.version.desc())(또는 최신 기준인 updatedAt 필드가 있다면 그 필드의 desc())를 체인해 최신
버전이 항상 반환되게 수정합니다.
- Line 61: The query can receive page=0 which makes the offset calculation (page
- 1) * pageSize negative and causes a PostgreSQL "OFFSET must not be negative"
error; fix by adding validation annotation (e.g., `@Min`(1) or `@Positive`) to the
PageRequestDto.page field and ensure the controller accepts a validated
PageRequestDto (add `@Valid` to the pageRequest parameter), and optionally harden
getOffset() / the offset calculation used in TermsQueryRepositoryImpl (the (page
- 1) * pageSize computation) to clamp to >=0 as a defensive fallback.
In `@src/main/java/com/dreamteam/alter/domain/terms/entity/Terms.java`:
- Around line 51-54: The `@SQLRestriction`("status != 'DELETED'") is currently
placed on the non-association field status in the Terms entity which won’t work
as intended; move the `@SQLRestriction` annotation to the Terms class declaration
(above the class name) so the deletion filter applies at the entity level,
leaving the `@Enumerated/`@Column on the private TermsStatus status field intact
and ensuring the class-level constraint uses the same string literal "DELETED".
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java`:
- Around line 16-20: The tests in AdminCreateTermsTest use classic Mockito
syntax; update them to BDD-style by replacing when(...).thenReturn(...) with
BDDMockito.given(...).willReturn(...) for stubbing (e.g., in
createTerms_returnsId) and replace verify(...) calls with BDD verification like
then(termsRepository).should(times(1)).save(...) (and similar for other
interactions); ensure you import org.mockito.BDDMockito or statically import
given/then and update all stubbing and verification occurrences in
AdminCreateTermsTest accordingly.
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/GenerateNextMonthWorkspaceShiftTxTest.java`:
- Line 45: Rename the test methods to follow the action_condition_expectedResult
convention (currently named like execute_noShiftsCreated_whenSchedulesEmpty);
for example rename execute_noShiftsCreated_whenSchedulesEmpty to
execute_schedulesEmpty_returnsZeroCounts, and similarly rename the other tests
in the same test class that use the inverse or omitted-condition pattern to
match action_condition_expectedResult; update method names (and any `@DisplayName`
or usages) for GenerateNextMonthWorkspaceShiftTxTest's test methods (including
the other two/three tests with the same naming issue) so they follow the
standardized pattern.
---
Duplicate comments:
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java`:
- Line 21: Rename the test class AdminCreateTermsTest to AdminCreateTermsTests
and refactor its test methods into grouped `@Nested` inner classes with
`@DisplayName` annotations to separate success and failure scenarios; locate the
top-level class declaration (class AdminCreateTermsTest) and change the name to
AdminCreateTermsTests, then create two inner static classes (e.g., `@Nested`
`@DisplayName`("Success cases") class Success and `@Nested` `@DisplayName`("Failure
cases") class Failure) to host respective test methods, moving or updating
existing test methods and their setup/utility methods into the appropriate
nested group.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java`:
- Around line 22-23: Rename the test class AdminPublishTermsTest to the plural
AdminPublishTermsTests and replace Mockito imperative calls: change all
when(...) usages to BDDMockito.given(...) for stubbing and change verify(...)
assertions to BDD style then(mock).should() calls; update any static imports and
references to the class name and ensure imports include org.mockito.BDDMockito
and remove/adjust former when/verify imports as needed.
- Around line 81-110: The tests in AdminPublishTermsTest
(publishTerms_throwsConflict_whenStatusIsPublished and
publishTerms_throwsConflict_whenStatusIsDeprecated) are missing verification
that the exclusive lock query is not invoked when a CONFLICT is thrown; after
the assertThatThrownBy(...) assertion add a verification asserting
termsRepository.findPublishedByTypeWithLock(...) was never called (use
verify(termsRepository, never()).findPublishedByTypeWithLock(any()) or
equivalent) so the lock lookup is not executed on early-exit error paths.
- Around line 49-66: Add interaction verifications for the deprecate flow: after
calling adminPublishTerms.execute(2L) add verify assertions to ensure
termsRepository.findById(2L) and
termsRepository.findPublishedByTypeWithLock(TermsType.SERVICE) were invoked, and
verify that the repository persisted the state changes (e.g.,
verify(termsRepository).save(existingTerms) and
verify(termsRepository).save(newTerms) or equivalent save/saveAll calls) so the
test guarantees the repository interactions for deprecating the previous
PUBLISHED term and publishing the new one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1b908c3-dba6-4b2a-9543-0a793d7e838e
📒 Files selected for processing (30)
src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminCreateTermsRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminCreateTermsResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsDetailResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsListItemResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminUpdateTermsRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetail.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.javasrc/main/java/com/dreamteam/alter/domain/terms/command/AdminCreateTermsCommand.javasrc/main/java/com/dreamteam/alter/domain/terms/command/AdminUpdateTermsCommand.javasrc/main/java/com/dreamteam/alter/domain/terms/entity/Terms.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminCreateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsDetailUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminUpdateTermsUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/query/AdminGetTermsListQuery.javasrc/main/java/com/dreamteam/alter/domain/terms/type/TermsStatus.javasrc/main/java/com/dreamteam/alter/domain/terms/type/TermsType.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/GenerateNextMonthWorkspaceShiftTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/GenerateNextMonthWorkspaceShiftTxTest.java
ysw789
requested changes
May 7, 2026
Comment on lines
+23
to
+27
| @NotBlank | ||
| @Size(max = 20) | ||
| @Pattern(regexp = "^v\\d+\\.\\d+$", message = "버전 형식은 v{숫자}.{숫자} 이어야 합니다") | ||
| @Schema(description = "약관 버전", example = "v1.0") | ||
| private String version; |
Contributor
There was a problem hiding this comment.
내부적으로 버전은 숫자로만 관리하고 UI에서 버전을 보여줄 때만 앞에 v 붙여서 렌더링 해주는 게 관리하기엔 좋지 않을까 싶습니다
There was a problem hiding this comment.
Actionable comments posted: 23
♻️ Duplicate comments (12)
src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java (2)
37-38: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMockito 사용을 BDD 스타일로 통일해 주세요.
when/verify를given/then().should()로 변경해 테스트 규칙을 충족해 주세요.As per coding guidelines,
src/test/**: "BDD-style Mockito: given() for setup, then().should() for verification."Also applies to: 45-45, 56-57, 64-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java` around lines 37 - 38, Tests in AdminCreateTermsTest mix classic Mockito when/verify with BDD style; replace uses of when(...) and verify(...) with BDD equivalents: use given(...) (or MockitoBDD.given) for stubbing savedTerms.getId() and termsRepository.save(any(Terms.class)) and use then(termsRepository).should() for verifications (e.g., interactions/assertions around termsRepository.save); update all occurrences referenced (savedTerms.getId(), termsRepository.save, and any verify calls at the other noted locations) to follow given(...) and then(...).should(...) BDD-style.
21-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 네이밍/그룹 구조를 컨벤션에 맞춰 주세요.
AdminCreateTermsTests형태로 클래스/파일명을 맞추고@Nested+@DisplayName으로 시나리오를 그룹화해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural "Tests")." and "@nested classes group related tests with@DisplayName."Also applies to: 29-68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java` at line 21, Rename the test class AdminCreateTermsTest to AdminCreateTermsTests (plural) and update the file name accordingly; then refactor related test methods into logical nested groups using JUnit's `@Nested` classes and annotate each nested class with `@DisplayName` to describe the scenario (e.g., success cases, validation failures), ensuring existing test methods are moved into appropriate nested classes while preserving their test method names and assertions.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java (1)
23-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 클래스/구조가 테스트 컨벤션과 불일치합니다.
클래스(및 파일)명을
...Tests로 맞추고, 시나리오를@Nested+@DisplayName으로 그룹화해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural "Tests")." and "@nested classes group related tests with@DisplayName."Also applies to: 31-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java` at line 23, Rename the test class AdminUpdateTermsTest to AdminUpdateTermsTests (plural) to comply with test naming conventions, and refactor related scenario groupings inside this class into `@Nested` inner classes annotated with `@DisplayName` to group related test cases; locate the class declaration AdminUpdateTermsTest and replace it with the pluralized class name, then create `@Nested` static inner classes (for example AdminUpdateTermsTests.HappyPath / AdminUpdateTermsTests.ValidationErrors) with `@DisplayName`("...") and move the corresponding test methods into those nested classes so tests follow the required structure.src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java (2)
36-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
(int) total캐스팅은 오버플로우 위험이 있습니다.
Math.toIntExact(total)로 바꿔서 초과 시 명시적으로 실패하도록 처리해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java` at line 36, Replace the unsafe cast in the PageResponseDto creation by using Math.toIntExact to prevent silent overflow: change the call that uses (int) total in the PageResponseDto.of(pageRequest, (int) total) expression to use Math.toIntExact(total) instead so an OverflowException is thrown if total exceeds Integer.MAX_VALUE; update the reference where total is passed into PageResponseDto.of (in AdminGetTermsList) accordingly.
3-7:⚠️ Potential issue | 🟠 Major | ⚡ Quick win유스케이스가 adapter DTO에 직접 의존하고 있어 계층 경계가 깨집니다.
adapter.inboundDTO/Page 모델은 컨트롤러(인바운드 어댑터)에서 변환하고, 유스케이스는 애플리케이션/도메인 모델만 사용하도록 분리해 주세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/application/**: "No direct infrastructure dependencies (no Spring Data, no HTTP clients)."Also applies to: 25-31, 36-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java` around lines 3 - 7, The AdminGetTermsList use-case currently imports adapter inbound DTOs (AdminTermsListItemResponseDto, PageRequestDto, PageResponseDto, PaginatedResponseDto) which breaks layer boundaries; change the use-case signature and internal types to use application/domain models only (e.g., use Terms and new application-level request/response models or plain primitives) and move any mapping to AdminTermsController (the inbound adapter). Specifically, remove imports of AdminTermsListItemResponseDto, PageRequestDto, PageResponseDto, PaginatedResponseDto from AdminGetTermsList, introduce/use an application-level DTO or domain types as the use-case input/output, and perform conversion between application models and adapter DTOs in the controller instead of in AdminGetTermsList.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java (2)
23-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 클래스 네이밍/BddMockito 컨벤션을 맞춰 주세요.
클래스(파일)명을
...Tests로 변경하고 Mockito를given/then().should()스타일로 통일해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural "Tests")." and "BDD-style Mockito: given() for setup, then().should() for verification."Also applies to: 31-111
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` at line 23, Rename the test class AdminPublishTermsTest to AdminPublishTermsTests and update the file name accordingly, then convert all Mockito usages inside that class (lines ~31-111) to BDD style: replace Mockito.when(...) or when(...) with BDDMockito.given(...) for stubbing, and replace Mockito.verify(...) or verify(...) assertions with BDD style then(mock).should() calls; also update or add static imports for BDDMockito.given and BDDMockito.then if present so the tests compile and follow the project's BDD Mockito convention.
90-94:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win비게시 가능 상태에서는 락 조회가 호출되지 않음을 검증해 주세요.
CONFLICT예외 검증 뒤에findPublishedByTypeWithLock(...)가never()인지 확인을 추가하면 회귀를 더 잘 막을 수 있습니다.Also applies to: 106-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java` around lines 90 - 94, The test should also assert that no lock lookup occurs when publishing is disallowed: after the existing assertion on adminPublishTerms.execute(1L) throwing CustomException with ErrorCode.CONFLICT in AdminPublishTermsTest, add a verification that the mocked repository/service method findPublishedByTypeWithLock(...) was never called (verify(..., never()) or equivalent) to ensure no lock retrieval happens; apply the same addition to the other test at lines 106-110.src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java (2)
31-36: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win컨트롤러 가이드라인의
@Getter가 누락되었습니다.컨트롤러 표준에 맞게
@Getter를 추가해 주세요.As per coding guidelines,
src/main/java/com/dreamteam/alter/adapter/**: "Controllers use@Getter,@RequiredArgsConstructor(no@Setter)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java` around lines 31 - 36, The AdminTermsController class is missing the required Lombok `@Getter` per controller guidelines; add the `@Getter` annotation to the AdminTermsController class declaration (alongside existing `@RestController`, `@RequestMapping`, `@PreAuthorize`, `@RequiredArgsConstructor`, `@Validated`) and import lombok.Getter so that generated getters are available while keeping no `@Setter` usage.
38-51:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
@Resource+final+@RequiredArgsConstructor조합은 의도한 name 기반 주입이 보장되지 않습니다.이 필드들의
@Resource를 제거하고 생성자 주입(필요 시@Qualifier)으로 명시해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java` around lines 38 - 51, The fields in AdminTermsController (adminGetTermsList, adminCreateTerms, adminGetTermsDetail, adminUpdateTerms, adminPublishTerms) currently use `@Resource` on final fields which with `@RequiredArgsConstructor` won't guarantee name-based injection; remove the `@Resource` annotations and switch to explicit constructor injection (or keep `@RequiredArgsConstructor`) and apply `@Qualifier` on the constructor parameters for name-based wiring as needed so each AdminGetTermsListUseCase, AdminCreateTermsUseCase, AdminGetTermsDetailUseCase, AdminUpdateTermsUseCase, and AdminPublishTermsUseCase is injected by qualifier/name rather than field `@Resource`.src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java (1)
29-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win동시 최초 게시 충돌 시 500으로 노출될 수 있습니다.
현재 흐름은 DB 유니크 제약 위반이 발생하면
DataIntegrityViolationException이 그대로 전파될 수 있어, 패배 트랜잭션이CONFLICT가 아닌 500으로 응답될 가능성이 있습니다.CustomException(ErrorCode.CONFLICT)로 변환해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java` around lines 29 - 33, The concurrent-first-publish path in AdminPublishTerms can propagate DataIntegrityViolationException (DB unique constraint) and return 500; wrap the critical persistence call (the sequence using termsQueryRepository.findPublishedByTypeWithLock(...) and terms.publish(), and the subsequent save/commit in the same use case) in a try-catch that catches DataIntegrityViolationException and rethrows new CustomException(ErrorCode.CONFLICT). Specifically, modify the method in AdminPublishTerms that calls Terms::deprecate and terms.publish() to catch org.springframework.dao.DataIntegrityViolationException and convert it to new CustomException(ErrorCode.CONFLICT) so concurrent unique-constraint violations surface as HTTP 409.src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java (1)
23-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 네이밍/Mockito 스타일 컨벤션을 맞춰 주세요.
클래스(파일)명을
...Tests로 변경하고, Mockito를given/then().should()로 통일해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural 'Tests')." and "BDD-style Mockito: given() for setup, then().should() for verification."Also applies to: 31-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java` at line 23, Rename the test class AdminGetTermsListTest to AdminGetTermsListTests (update the class name and file name) and convert Mockito usage to BDD style: replace any when(...) stubbing with BDDMockito.given(...) (or static given(...)) and replace verify(...) assertions with BDDMockito.then(mock).should() (or static then(mock).should()) in all test methods referenced in this file (e.g., the stubbing and verification sites between the previous when/verify usages).src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.java (1)
23-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win테스트 클래스/시나리오 구조를 컨벤션에 맞춰 주세요.
클래스(파일)명을
...Tests로 변경하고@Nested+@DisplayName구조로 시나리오를 그룹화해 주세요.As per coding guidelines,
src/test/**: "Test classes follow [ClassName]Tests.java naming (plural "Tests")." and "@nested classes group related tests with@DisplayName."Also applies to: 31-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.java` at line 23, Rename the test class from AdminGetTermsDetailTest to follow the plural Tests convention (e.g., AdminGetTermsDetailTests) and update the file name accordingly; then reorganize the scenarios inside the class into `@Nested` inner classes with descriptive `@DisplayName` annotations (group the related test methods currently between lines 31-78 into appropriate nested scenario classes) so each scenario group is a `@Nested` class annotated with `@DisplayName` and contains its related `@Test` methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java`:
- Around line 53-105: The controller methods (getTermsList, createTerms,
getTermsDetail, createTerms, updateTerms, publishTerms) are not using
AdminActionContext to identify the acting admin; inject AdminActionContext into
AdminTermsController and retrieve the current admin/actor in each endpoint, then
pass that actor into the appropriate command/query or service execution calls
(e.g., AdminGetTermsListQuery, AdminCreateTermsCommand,
adminGetTermsDetail.execute, adminCreateTerms.execute, adminUpdateTerms.execute,
adminPublishTerms.execute) so all admin actions use AdminActionContext for actor
retrieval per the controller guidelines.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/workspace/controller/AdminWorkspaceReasonController.java`:
- Around line 36-41: The getWorkspaceReasonList method in
AdminWorkspaceReasonController is missing the `@Override` annotation; add
`@Override` above getWorkspaceReasonList in AdminWorkspaceReasonController to
match the interface AdminWorkspaceReasonControllerSpec, and while doing so
verify the method signature (parameter type/name and annotations, e.g.,
PathVariable Long workspaceRequestId) exactly matches the declaration in
AdminWorkspaceReasonControllerSpec so the compiler can detect any signature
mismatches.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.java`:
- Around line 80-92: The method findPublishedByTypeWithLock in
TermsQueryRepositoryImpl uses queryFactory...fetchOne(), which can throw
NonUniqueResultException if multiple PUBLISHED rows exist; change fetchOne() to
fetchFirst() so the pessimistic-lock query
(setLockMode(LockModeType.PESSIMISTIC_WRITE)) returns the first matching Terms
instead of failing, preserving the lock behavior and matching the non-locked
variant findPublishedByType.
- Around line 38-65: The entity-level `@SQLRestriction` on Terms prevents any
query from returning DELETED rows; remove the `@SQLRestriction`("status !=
'DELETED'") from the Terms entity and rely on explicit predicates (notDeleted(),
statusCondition(TermsStatus)) in repository methods, or alternatively implement
separate admin-specific repository methods that run native JPQL/SQL (via
EntityManager#createNativeQuery) to bypass the entity-level restriction; update
the code paths referencing notDeleted(), countByFilter, and findByFilter so
non-admin calls still apply notDeleted() and admin calls use the bypass/native
queries to allow fetching DELETED records.
In
`@src/main/java/com/dreamteam/alter/application/notification/NotificationService.java`:
- Around line 218-221: sendNotificationOnly currently calls
isNotificationBlocked(user) (DB lookup) and then calls sendFcmNotification which
repeats the same consent check; remove the redundant DB lookup in
sendNotificationOnly by deleting the isNotificationBlocked(...) check and its
early return so that sendFcmNotification handles the consent check and DB access
centrally; keep any surrounding logging or behavior intact and ensure
sendNotificationOnly still calls sendFcmNotification(userId, ...) as before.
- Around line 139-145: The stream's filter currently uses Optional.orElseThrow
which aborts the whole batch when a consent record is missing; instead, change
the filter for deviceTokens (building eligibleTokens) to treat missing consent
as a non-fatal case: look up NotificationConsent via
consentMap.get(dt.getUser().getId()), if null call the class logger (e.g.,
log.warn or similar) with the user id and skip by returning false, otherwise
apply the existing consent checks (consent.isNotificationConsent() &&
(consent.isNightNotificationConsent() || isDaytime())); remove the throw
(CustomException / ErrorCode.NOT_FOUND) from the filter so a missing consent
only excludes that device rather than failing the whole batch.
- Around line 291-294: isDaytime() uses LocalTime.now() which relies on the JVM
default timezone (often UTC in cloud), causing incorrect daytime checks; update
isDaytime() to obtain the current time in the intended timezone (e.g.,
ZoneId.of("Asia/Seoul")) or use the user's configured ZoneId via
ZonedDateTime.now(zone) and then check the hour, so replace
LocalTime.now().getHour() with the hour from
ZonedDateTime.now(desiredZone).getHour() (or inject a Clock/ZoneId for
testability) to ensure correct KST-based filtering.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java`:
- Around line 17-20: The AdminGetTermsList application use-case class is missing
the standard `@Slf4j` logger; add the Lombok `@Slf4j` annotation to the
AdminGetTermsList class declaration (alongside `@Service`,
`@RequiredArgsConstructor`, `@Transactional`) and import lombok.extern.slf4j.Slf4j
so the class has a log field for consistent layer logging; ensure compilation by
confirming Lombok is available in the project.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java`:
- Around line 13-16: The AdminPublishTerms class is missing the `@Slf4j` logging
annotation; add the Lombok `@Slf4j` annotation to the AdminPublishTerms class (and
import lombok.extern.slf4j.Slf4j) so the class uses the standard logging
instance (log) for consistent application use-case logging across the package
(class name: AdminPublishTerms, implements AdminPublishTermsUseCase).
In
`@src/main/java/com/dreamteam/alter/application/user/usecase/LogoutManager.java`:
- Around line 22-25: The execute method in LogoutManager currently calls
authService.revokeAllExistingAuthorizations and
notificationService.removeUserDeviceToken sequentially so an exception in
removeUserDeviceToken can roll back the authorization revoke; change
execute(ManagerActor actor) to perform revokeAllExistingAuthorizations first and
then call removeUserDeviceToken in a try/catch that swallows/logs the exception
(use the class logger and include actor.getManagerUser() context) so token
deletion is best-effort and does not propagate errors that would undo
revocation.
In
`@src/main/java/com/dreamteam/alter/application/workspace/usecase/AdminGetWorkspaceReasonList.java`:
- Line 8: AdminGetWorkspaceReasonList currently imports and returns the adapter
DTO WorkspaceReasonResponseDto—remove that adapter-layer dependency from the
application layer: change the class AdminGetWorkspaceReasonList to use the
domain/port return type defined by AdminGetWorkspaceReasonListUseCase (i.e.,
return domain objects or port DTOs, not com.dreamteam.alter.adapter.*), delete
the import of WorkspaceReasonResponseDto, and move the mapping logic found at
Line 33 into the adapter/controller layer (where controller transforms domain
objects into WorkspaceReasonResponseDto). Ensure method signatures and return
types in AdminGetWorkspaceReasonList and the AdminGetWorkspaceReasonListUseCase
port align with domain types so the adapter performs the final conversion.
In
`@src/main/java/com/dreamteam/alter/domain/notification/command/UpdateNotificationConsentCommand.java`:
- Around line 5-13: Remove all Lombok imports/annotations from
UpdateNotificationConsentCommand; declare the command's fields as private final,
add an explicit constructor that sets those fields, and provide explicit public
getter methods (or a single public accessor per field) instead of `@Getter`; also
replace `@Builder` by adding a clear static factory method (e.g., of(...) or
create(...)) or a hand-written Builder inner class if you need fluent
construction. Ensure the class remains a plain Java POJO with no external
library annotations so it conforms to the domain layer rule.
In
`@src/main/java/com/dreamteam/alter/domain/notification/entity/NotificationConsent.java`:
- Around line 1-86: The NotificationConsent domain class currently contains
JPA/Spring Data infrastructure (jakarta.persistence.*,
org.springframework.data.annotation.*, AuditingEntityListener) which violates
the zero-infrastructure rule; remove all persistence annotations and imports
from NotificationConsent so it becomes a pure domain model (keep fields,
builder, create and updateConsent methods intact), then create a new
persistence-side DTO/entity (e.g., NotificationConsentEntity) in the
persistence/adapters package that holds the `@Entity`, `@Table`, `@Id`, `@OneToOne`,
`@CreatedDate/`@LastModifiedDate, AuditingEntityListener, and mapping logic to
convert between NotificationConsent and NotificationConsentEntity; ensure
conversion methods (toDomain/fromDomain) live in the persistence layer and
update any repository code to use NotificationConsentEntity instead of the
domain class.
- Around line 67-85: The switch in NotificationConsent.updateConsent does not
handle unknown NotificationConsentType values, so add a default branch that
throws a descriptive CustomException (e.g., ErrorCode.ILLEGAL_ARGUMENT)
including the unexpected type value; update the default to throw the exception
(mention NotificationConsent.updateConsent and NotificationConsentType) so
adding new enum values fails fast rather than silently no-op.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.java`:
- Line 20: The port method name TermsQueryRepository.findPublishedByTypeWithLock
exposes an implementation detail (locking); rename it to a domain-intent name
such as TermsQueryRepository.findCurrentPublishedByType or simply
TermsQueryRepository.findPublishedByType, update the interface signature
accordingly, and move any pessimistic-lock logic into the infrastructure
adapter/implementation that implements this interface (e.g., the JPA/DB
repository class) so callers remain agnostic; also update all call sites to the
new method name and ensure adapter implementations obtain the lock in the
transaction context.
In
`@src/main/java/com/dreamteam/alter/domain/workspace/port/inbound/AdminGetWorkspaceReasonListUseCase.java`:
- Around line 5-8: The port AdminGetWorkspaceReasonListUseCase currently returns
the adapter DTO WorkspaceReasonResponseDto which inverts hexagonal dependencies;
change the interface signature to return domain objects (e.g.,
List<WorkspaceReason>) instead of List<WorkspaceReasonResponseDto>, remove the
import of
com.dreamteam.alter.adapter.inbound.general.workspace.dto.WorkspaceReasonResponseDto
from the port, and ensure the mapping to WorkspaceReasonResponseDto is moved
into the adapter/controller (e.g., in AdminWorkspaceReasonController call
adminGetWorkspaceReasonList.execute(workspaceRequestId) to get
List<WorkspaceReason> and map to WorkspaceReasonResponseDto::from before
returning the HTTP response).
In
`@src/test/java/com/dreamteam/alter/application/notification/NotificationServiceConsentTests.java`:
- Line 40: Remove the class-level `@MockitoSettings`(strictness =
Strictness.LENIENT) and instead mark only the unused stubs as lenient inside the
setUp() method: replace the common stubs for methods like
notificationRepository.findByUser(user), notificationRepository.findById(1L)
(and any findByUsers/findAllById variants used for batch tests) with
lenient().when(...) so other tests can keep STRICT_STUBS; alternatively, if
preferred, move the batch-related tests into a separate class
(SendMultipleNotificationsConsentTests) so shared setUp stubs aren’t applied to
tests that don’t need them.
- Around line 226-262: Add two missing tests under
SendNotificationOnlyConsentTests for notificationService.sendNotificationOnly:
(1) a daytime case where notificationConsentQueryRepository.findByUser(user)
returns consentWith(true, false) and LocalTime.now() is mocked to a daytime
value (e.g., localTimeAt(10)) and assert that fcmClient.sendNotification(...) is
called; (2) a missing-consent case where findByUser(user) returns
Optional.empty() and assert that calling
notificationService.sendNotificationOnly(...) throws the NOT_FOUND
CustomException (check errorCode equals ErrorCode.NOT_FOUND). Ensure you use the
existing helpers consentWith, localTimeAt, and mockStatic(LocalTime.class)
patterns and verify fcmClient interactions with then(fcmClient).should() or
assertThatThrownBy as in other tests.
In
`@src/test/java/com/dreamteam/alter/application/notification/usecase/GetNotificationConsentTests.java`:
- Around line 40-107: Add BDD-style Mockito verification to each test in
GetNotificationConsentTests: after invoking getNotificationConsent.execute(...)
in the "when" section, add a verification that
notificationConsentQueryRepository was called with the mocked user (use
then(notificationConsentQueryRepository).should().findByUser(user)). Do this in
all test methods shown (throwsNotFound_whenNoRecordExists,
returnsActualValues_whenBothTrue, returnsActualValues_whenGeneralIsFalse,
returnsActualValues_whenNightIsFalse) so the repository interaction is asserted
for the GetNotificationConsent use case.
In
`@src/test/java/com/dreamteam/alter/application/notification/usecase/UpdateNotificationConsentTests.java`:
- Around line 65-83: Test names and DisplayName text (e.g.,
savesGeneralTrue_keepsNightAsIs and its `@DisplayName`) claim an explicit save
occurred while the assertions check that notificationConsentRepository had no
interactions; update either the test expectations or the names: if production
code relies on JPA dirty‑checking, rename methods and `@DisplayName` (e.g.,
updateGeneralTrue_keepsNightAsIs_withoutRepositorySave) to state "updated via
dirty‑checking / no repository save", or if you intend to verify an explicit
persist, change the assertion to expect notificationConsentRepository.save(...)
was called after calling UpdateNotificationConsent.execute(command); apply the
same renaming or assertion change to the other affected tests referenced in the
comment (the methods around 85–103, 105–123, 145–163) so names match behavior.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.java`:
- Line 36: Replace the Mockito stubbing and verification to BDD style: change
the stubbing of termsQueryRepository.findById(1L) from when(...).thenReturn(...)
to given(termsQueryRepository.findById(1L)).willReturn(Optional.of(terms)), and
change the verification from verify(termsQueryRepository, times(1)).findById(1L)
to then(termsQueryRepository).should(times(1)).findById(1L); update any imports
if needed (given, then, willReturn, should).
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java`:
- Line 39: Replace classic Mockito stubs/verifications in AdminUpdateTermsTest
with BDD style: change any
when(termsQueryRepository.findById(...)).thenReturn(...) (and
when(termsRepository.findById(...)).thenReturn(...)) to
given(termsQueryRepository.findById(...)).willReturn(...) /
given(termsRepository.findById(...)).willReturn(...), and change
verify(termsRepository, times(1)).findById(1L) to
then(termsRepository).should(times(1)).findById(1L); also update imports to use
static org.mockito.BDDMockito.* so all stubbing and verification in methods like
those referencing termsQueryRepository.findById and termsRepository.findById
follow BDDMockito conventions.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/LogoutManagerTests.java`:
- Around line 51-52: Rename the test method
calls_revokeAllExistingAuthorizations in LogoutManagerTests to follow the
action_condition_expectedResult pattern (for example
execute_actorProvided_revokesAllExistingAuthorizations or
execute_actorProvided_removesUserDeviceToken) so the action, condition and
expected result are explicit; apply the same renaming convention to the other
test method flagged in the file (the one near lines 61-62) so all test names
follow the descriptive pattern.
---
Duplicate comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.java`:
- Around line 31-36: The AdminTermsController class is missing the required
Lombok `@Getter` per controller guidelines; add the `@Getter` annotation to the
AdminTermsController class declaration (alongside existing `@RestController`,
`@RequestMapping`, `@PreAuthorize`, `@RequiredArgsConstructor`, `@Validated`) and import
lombok.Getter so that generated getters are available while keeping no `@Setter`
usage.
- Around line 38-51: The fields in AdminTermsController (adminGetTermsList,
adminCreateTerms, adminGetTermsDetail, adminUpdateTerms, adminPublishTerms)
currently use `@Resource` on final fields which with `@RequiredArgsConstructor`
won't guarantee name-based injection; remove the `@Resource` annotations and
switch to explicit constructor injection (or keep `@RequiredArgsConstructor`) and
apply `@Qualifier` on the constructor parameters for name-based wiring as needed
so each AdminGetTermsListUseCase, AdminCreateTermsUseCase,
AdminGetTermsDetailUseCase, AdminUpdateTermsUseCase, and
AdminPublishTermsUseCase is injected by qualifier/name rather than field
`@Resource`.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.java`:
- Line 36: Replace the unsafe cast in the PageResponseDto creation by using
Math.toIntExact to prevent silent overflow: change the call that uses (int)
total in the PageResponseDto.of(pageRequest, (int) total) expression to use
Math.toIntExact(total) instead so an OverflowException is thrown if total
exceeds Integer.MAX_VALUE; update the reference where total is passed into
PageResponseDto.of (in AdminGetTermsList) accordingly.
- Around line 3-7: The AdminGetTermsList use-case currently imports adapter
inbound DTOs (AdminTermsListItemResponseDto, PageRequestDto, PageResponseDto,
PaginatedResponseDto) which breaks layer boundaries; change the use-case
signature and internal types to use application/domain models only (e.g., use
Terms and new application-level request/response models or plain primitives) and
move any mapping to AdminTermsController (the inbound adapter). Specifically,
remove imports of AdminTermsListItemResponseDto, PageRequestDto,
PageResponseDto, PaginatedResponseDto from AdminGetTermsList, introduce/use an
application-level DTO or domain types as the use-case input/output, and perform
conversion between application models and adapter DTOs in the controller instead
of in AdminGetTermsList.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.java`:
- Around line 29-33: The concurrent-first-publish path in AdminPublishTerms can
propagate DataIntegrityViolationException (DB unique constraint) and return 500;
wrap the critical persistence call (the sequence using
termsQueryRepository.findPublishedByTypeWithLock(...) and terms.publish(), and
the subsequent save/commit in the same use case) in a try-catch that catches
DataIntegrityViolationException and rethrows new
CustomException(ErrorCode.CONFLICT). Specifically, modify the method in
AdminPublishTerms that calls Terms::deprecate and terms.publish() to catch
org.springframework.dao.DataIntegrityViolationException and convert it to new
CustomException(ErrorCode.CONFLICT) so concurrent unique-constraint violations
surface as HTTP 409.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.java`:
- Around line 37-38: Tests in AdminCreateTermsTest mix classic Mockito
when/verify with BDD style; replace uses of when(...) and verify(...) with BDD
equivalents: use given(...) (or MockitoBDD.given) for stubbing
savedTerms.getId() and termsRepository.save(any(Terms.class)) and use
then(termsRepository).should() for verifications (e.g., interactions/assertions
around termsRepository.save); update all occurrences referenced
(savedTerms.getId(), termsRepository.save, and any verify calls at the other
noted locations) to follow given(...) and then(...).should(...) BDD-style.
- Line 21: Rename the test class AdminCreateTermsTest to AdminCreateTermsTests
(plural) and update the file name accordingly; then refactor related test
methods into logical nested groups using JUnit's `@Nested` classes and annotate
each nested class with `@DisplayName` to describe the scenario (e.g., success
cases, validation failures), ensuring existing test methods are moved into
appropriate nested classes while preserving their test method names and
assertions.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.java`:
- Line 23: Rename the test class from AdminGetTermsDetailTest to follow the
plural Tests convention (e.g., AdminGetTermsDetailTests) and update the file
name accordingly; then reorganize the scenarios inside the class into `@Nested`
inner classes with descriptive `@DisplayName` annotations (group the related test
methods currently between lines 31-78 into appropriate nested scenario classes)
so each scenario group is a `@Nested` class annotated with `@DisplayName` and
contains its related `@Test` methods.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.java`:
- Line 23: Rename the test class AdminGetTermsListTest to AdminGetTermsListTests
(update the class name and file name) and convert Mockito usage to BDD style:
replace any when(...) stubbing with BDDMockito.given(...) (or static given(...))
and replace verify(...) assertions with BDDMockito.then(mock).should() (or
static then(mock).should()) in all test methods referenced in this file (e.g.,
the stubbing and verification sites between the previous when/verify usages).
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.java`:
- Line 23: Rename the test class AdminPublishTermsTest to AdminPublishTermsTests
and update the file name accordingly, then convert all Mockito usages inside
that class (lines ~31-111) to BDD style: replace Mockito.when(...) or when(...)
with BDDMockito.given(...) for stubbing, and replace Mockito.verify(...) or
verify(...) assertions with BDD style then(mock).should() calls; also update or
add static imports for BDDMockito.given and BDDMockito.then if present so the
tests compile and follow the project's BDD Mockito convention.
- Around line 90-94: The test should also assert that no lock lookup occurs when
publishing is disallowed: after the existing assertion on
adminPublishTerms.execute(1L) throwing CustomException with ErrorCode.CONFLICT
in AdminPublishTermsTest, add a verification that the mocked repository/service
method findPublishedByTypeWithLock(...) was never called (verify(..., never())
or equivalent) to ensure no lock retrieval happens; apply the same addition to
the other test at lines 106-110.
In
`@src/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.java`:
- Line 23: Rename the test class AdminUpdateTermsTest to AdminUpdateTermsTests
(plural) to comply with test naming conventions, and refactor related scenario
groupings inside this class into `@Nested` inner classes annotated with
`@DisplayName` to group related test cases; locate the class declaration
AdminUpdateTermsTest and replace it with the pluralized class name, then create
`@Nested` static inner classes (for example AdminUpdateTermsTests.HappyPath /
AdminUpdateTermsTests.ValidationErrors) with `@DisplayName`("...") and move the
corresponding test methods into those nested classes so tests follow the
required structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1d9726ef-ab23-4a78-82b3-958b0dc2f0bd
📒 Files selected for processing (54)
src/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/controller/AdminTermsController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminCreateTermsRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsDetailResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/terms/dto/AdminTermsListItemResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/workspace/controller/AdminWorkspaceReasonController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/admin/workspace/controller/AdminWorkspaceReasonControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/notification/controller/UserNotificationConsentController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/notification/controller/UserNotificationConsentControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/notification/dto/NotificationConsentItemDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/notification/dto/NotificationConsentResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/notification/dto/UpdateNotificationConsentRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/notification/controller/ManagerNotificationConsentController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/notification/controller/ManagerNotificationConsentControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/outbound/notification/persistence/NotificationConsentJpaRepository.javasrc/main/java/com/dreamteam/alter/adapter/outbound/notification/persistence/NotificationConsentRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/notification/persistence/readonly/NotificationConsentQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/notification/NotificationService.javasrc/main/java/com/dreamteam/alter/application/notification/usecase/GetNotificationConsent.javasrc/main/java/com/dreamteam/alter/application/notification/usecase/UpdateNotificationConsent.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsList.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTerms.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTerms.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUser.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserTx.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTx.javasrc/main/java/com/dreamteam/alter/application/user/usecase/LogoutManager.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/AdminGetWorkspaceReasonList.javasrc/main/java/com/dreamteam/alter/domain/notification/command/GetNotificationConsentCommand.javasrc/main/java/com/dreamteam/alter/domain/notification/command/UpdateNotificationConsentCommand.javasrc/main/java/com/dreamteam/alter/domain/notification/entity/NotificationConsent.javasrc/main/java/com/dreamteam/alter/domain/notification/port/inbound/GetNotificationConsentUseCase.javasrc/main/java/com/dreamteam/alter/domain/notification/port/inbound/UpdateNotificationConsentUseCase.javasrc/main/java/com/dreamteam/alter/domain/notification/port/outbound/NotificationConsentQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/notification/port/outbound/NotificationConsentRepository.javasrc/main/java/com/dreamteam/alter/domain/notification/result/GetNotificationConsentResult.javasrc/main/java/com/dreamteam/alter/domain/notification/type/NotificationConsentType.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/AdminGetTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsRepository.javasrc/main/java/com/dreamteam/alter/domain/workspace/port/inbound/AdminGetWorkspaceReasonListUseCase.javasrc/test/java/com/dreamteam/alter/application/notification/NotificationServiceConsentTests.javasrc/test/java/com/dreamteam/alter/application/notification/usecase/GetNotificationConsentTests.javasrc/test/java/com/dreamteam/alter/application/notification/usecase/UpdateNotificationConsentTests.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminCreateTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsDetailTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminGetTermsListTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminPublishTermsTest.javasrc/test/java/com/dreamteam/alter/application/terms/usecase/AdminUpdateTermsTest.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/LogoutManagerTests.java
added 17 commits
May 7, 2026 13:55
added 13 commits
May 7, 2026 13:55
…er.inbound DTO 의존성 제거
…pter.inbound DTO 의존성 제거
ysw789
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
관련 문서
https://www.notion.so/BE-355865531628805392e6dcb4c5aa9123?v=3288655316288071b53d000c67f996d3&source=copy_link
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores