Conversation
…-220-BE-API-응답-개선
…-220-BE-API-응답-개선
WalkthroughAttendance DTOs and services were restructured: AttendanceRoundRequest gains a mandatory sessionId (UUID); AttendanceRoundResponse and AttendanceSessionResponse fields were simplified/replaced with new fields (status, availableMinutes, LocationInfo, defaultStartTime, defaultAvailableMinutes, isVisible); RoundStatus enum adds API string values; AttendanceRoundService now saves the session after creating a round; tests updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant RoundService
participant RoundRepo
participant SessionRepo
Client->>Controller: Create AttendanceRound(request with sessionId)
Controller->>RoundService: createRound(request)
RoundService->>RoundRepo: save(AttendanceRound)
RoundRepo-->>RoundService: savedRound
Note right of RoundService `#D6EAF8`: session association updated
RoundService->>SessionRepo: save(AttendanceSession) %% new save to persist relation
SessionRepo-->>RoundService: savedSession
RoundService-->>Controller: createdRoundResponse
Controller-->>Client: 201 Created
sequenceDiagram
participant Service
participant SessionEntity
participant ResponseDTO
Service->>SessionEntity: read fields (startsAt, windowSeconds, location, visibility)
Note right of Service `#F9E79F`: mapping changed
Service->>ResponseDTO: set location (LocationInfo) if present
Service->>ResponseDTO: set defaultStartTime (startsAt -> LocalTime)
Service->>ResponseDTO: set defaultAvailableMinutes (windowSeconds -> minutes)
Service->>ResponseDTO: set isVisible (visibility == PUBLIC)
ResponseDTO-->>Service: response ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 분
Possibly related PRs
Suggested reviewers
리뷰 요약개요출석 관리 모듈의 API 계약을 재구성합니다. 요청/응답 DTO에 세션 참조, 위치 정보 구조화, 상태 필드를 추가하고, 불필요한 세부 통계 및 시간 관련 필드를 제거하여 API 응답 구조를 단순화합니다. 변경 사항
코드 리뷰 예상 난이도🎯 4 (복잡함) | ⏱️ ~60분 추가 검토 필요 영역:
관련 가능 PR
추천 리뷰어
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundRequest.java (1)
27-34: sessionId 필드 제약 및 스키마 정의 시 추가로 확인할 점
@NotNull로 세션 ID를 필수값으로 강제하고,@Schema에 예시/형식을 명시한 부분은 일관되고 좋습니다. 다만:
- 만약 API URL 경로에서 이미
sessionId(예:/sessions/{sessionId}/rounds)를 받고 있다면, DTO에서 다시 body로 받는 구조가 중복이 아닌지 한 번 더 점검해 주세요.- 사용 중인 springdoc/OpenAPI 버전에 따라, 필요 시
requiredMode = Schema.RequiredMode.REQUIRED등을 활용해 스키마 상에서도 명시적으로 required 처리가 되는지 확인하는 것도 고려할 수 있습니다.기능적으로는 문제가 없어 보이고, 설계 선택이 맞는지만 확인해 보면 좋겠습니다.
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (1)
55-59: status 필드에 enum 사용을 고려해보세요.status를 String으로 정의하면 런타임에 잘못된 값이 할당될 수 있습니다. 가능한 상태값("opened", "upcoming", "closed" 등)을 enum으로 정의하면 타입 안정성이 향상되고 API 문서도 더 명확해집니다.
예시:
public enum RoundStatus { OPENED, UPCOMING, CLOSED }그런 다음 DTO에서:
- private String status; + private RoundStatus status;그리고 fromEntity 메서드에서:
- String statusString = round.calculateCurrentStatus().toString().toLowerCase(); + RoundStatus status = round.calculateCurrentStatus();backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionResponse.java (1)
7-73: 세션 응답 모델 구조 변경 방향은 일관적이며 서비스 로직과 잘 맞습니다
location을LocationInfo중첩 클래스로 감싼 것,defaultStartTime/defaultAvailableMinutes/isVisible필드 추가가 서비스의convertToResponse구현과 잘 대응되고 있습니다.@JsonFormat(pattern = "HH:mm:ss")로 시간 포맷을 명시한 것도 API 계약 측면에서 좋습니다.- 위치 정보가 없을 때
location을null로 둘 수 있는 구조라, 선택적 위치 정보 표현에도 무리가 없어 보입니다.다만, Request DTO에서는
latitude/longitude이름을 쓰고, Response DTO에서는lat/lng로 쓰고 있어서 필드 명이 약간 불일치합니다. 추후 클라이언트 입장에서 혼동을 줄이려면, 가능하다면 Request/Response 간 필드 이름을 통일하는 것도 고려해볼 만합니다.backend/src/test/java/org/sejongisc/backend/attendance/controller/AttendanceSessionControllerTest.java (1)
19-245: 새 응답 스키마에 맞게 테스트를 잘 정비하셨습니다
createSession_success에서location.lat/location.lng,isVisible,defaultAvailableMinutes를 직접 검증하도록 바꾼 부분이 새 DTO 구조를 잘 커버하고 있습니다.- 다른 조회/수정 테스트들도
defaultStartTime,defaultAvailableMinutes,isVisible을 빌더로 세팅해두어 서비스 계약 변경에 맞게 정리되어 있습니다.선택 사항이지만, 계약을 더 단단히 하려면 아래 정도를 추가로 고려해볼 수 있습니다.
defaultStartTime에 대해HH:mm:ss포맷으로 직렬화되는지jsonPath("$.defaultStartTime")까지 한 테스트에서 한 번은 검증.getSession_success/getPublicSessions_success/getActivateSessions_success에서도 최소한isVisible이나defaultAvailableMinutes같은 핵심 필드를 하나씩 더 점검하면 회귀를 잡는 데 도움이 됩니다.필수는 아니고, 현재 테스트만으로도 주요 변경 사항은 잘 커버되고 있습니다.
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java (1)
17-171:convertToResponse리팩터링 로직은 DTO 변경과 잘 일치합니다 (몇 가지 전제만 확인 필요)
LocationInfo를 세션의Location엔티티에서lat/lng만 추출해서 생성하고, 없으면null로 두는 방식은 새 DTO 설계와 일관적입니다.defaultStartTime = session.getStartsAt().toLocalTime()으로 시간만 분리하는 부분도AttendanceSessionResponse의LocalTime필드와 잘 맞습니다.defaultAvailableMinutes = session.getWindowSeconds() / 60는 초 단위windowSeconds를 분 단위로 변환하려는 의도에 부합하고, Request 레벨에서 최소 300초 이상을 강제하고 있어(스니펫 기준) 값 자체도 자연스럽습니다.isVisible = (session.getVisibility() == SessionVisibility.PUBLIC)으로 공개 여부를 Boolean으로 매핑한 것도 직관적입니다.visibility가null이어도 NPE 없이false가 되는 점도 괜찮은 기본값 선택입니다.다만 몇 가지 점은 한 번만 확인해 두시면 좋겠습니다.
session.getWindowSeconds()/session.getStartsAt()가 null이 아니라고 강하게 가정하고 있습니다. 이미getActiveSessions에서 동일한 전제를 두고 있으므로 새 버그는 아니지만, 엔티티/DB 제약이나 Bean Validation에서 not null이 보장되는지 한 번 확인해 두는 것을 권장합니다.- 클래스 상단의
getSessionById/getSessionByCode메소드 Javadoc에는 아직 “남은 시간, 참여자 수 등 계산된 정보 포함”이라는 예전 설명이 남아 있는데, 이제는 단순한 설정 정보 위주의 응답으로 바뀌었으므로 주석도 최신 스펙에 맞게 정리해 두면 유지보수에 도움이 될 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundRequest.java(2 hunks)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java(2 hunks)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionResponse.java(3 hunks)backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java(2 hunks)backend/src/main/resources/application.yml(1 hunks)backend/src/test/java/org/sejongisc/backend/attendance/controller/AttendanceSessionControllerTest.java(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionResponse.java (3)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (1)
Getter(15-77)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceCheckInResponse.java (1)
Getter(15-35)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionRequest.java (1)
Getter(10-106)
🔇 Additional comments (2)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundRequest.java (1)
14-14: UUID import 추가는 적절합니다.
sessionId를UUID타입으로 도입했으므로 해당 import는 필요하고, 불필요한 의존성도 없어 보입니다.backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (1)
21-21: 문서가 새로운 구조를 정확히 반영합니다.클래스 설명이 단순화된 응답 구조를 적절히 설명하고 있습니다.
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java (1)
72-76: 세션save호출은 대부분 불필요한 중복 DB 쓰기일 가능성이 큽니다
attendanceSessionRepository.findById(sessionId)로 조회한session은 트랜잭션 안에서 이미 영속 상태이기 때문에,session.getRounds().add(saved);만 호출해도 flush 시점에 컬렉션 변경이 감지되어 DB에 반영됩니다.
그 뒤에 다시attendanceSessionRepository.save(session);를 호출하면 동일 엔티티에 대해 한 번 더 UPDATE 쿼리가 나갈 수 있어 성능상 오버헤드만 생기는 경우가 많습니다.
현재 구조상 세션이 detach 상태로 들어오는 경우는 없어 보이므로, 특별한 이유가 없다면
save(session)제거를 고려해 보시는 것을 권장합니다. 만약 특정 JPA 설정/케이스 때문에 꼭 필요한 호출이라면, 그 이유를 주석으로 남겨 두면 이후 유지보수 시 혼동을 줄일 수 있습니다.또한
session.getRounds().add(saved);/session.getRounds().remove(round);패턴이 여러 곳에서 반복되므로,AttendanceSession엔티티에addRound(AttendanceRound round),removeRound(AttendanceRound round)같은 양방향 편의 메서드를 두고 여기서는 그 메서드만 호출하는 것도 가독성과 일관성 측면에서 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (2)
61-69:calculateCurrentStatus().getValue()사용으로 status 변환 로직이 훨씬 안전해졌습니다.기존
toString().toLowerCase()방식 대신 enum에 정의된 명시적 문자열을 쓰도록 바뀌어, enum 이름 변경 시에도 API 응답 포맷이 깨지지 않겠습니다. 이전 리뷰에서 제안되었던 개선사항이 잘 반영된 것으로 보입니다.
40-47:startTime포맷 변경(HH:mm→HH:mm:ss)은 여전히 브레이킹 체인지일 수 있습니다.이전 리뷰에서 지적되었던 것처럼, 기존 클라이언트가
HH:mm포맷으로 파싱하고 있다면 예제와@JsonFormat을HH:mm:ss로 바꾸는 것은 파싱 실패를 유발할 수 있습니다. 실제로 모든 소비자가 초 단위 포맷을 수용하도록 수정되었는지, 아니면 구 포맷 유지(예:HH:mm)·버전 분리 등 단계적 마이그레이션이 필요한지 한 번 더 확인해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java(2 hunks)backend/src/main/java/org/sejongisc/backend/attendance/entity/RoundStatus.java(1 hunks)
🔇 Additional comments (4)
backend/src/main/java/org/sejongisc/backend/attendance/entity/RoundStatus.java (2)
7-17: RoundStatus에 API 응답용value필드를 추가한 설계가 적절합니다.각 enum 상수에 명시적인 문자열을 부여해 두어서, 추후 enum 이름 변경/추가가 있어도 API 응답 포맷을 안정적으로 유지할 수 있을 것 같습니다. 현재 구현에서는 별다른 문제점은 보이지 않습니다.
23-31:getValue()메서드와 Javadoc이 역할을 명확하게 정의하고 있습니다.
toString().toLowerCase()대신 별도 필드로 API 응답 값을 노출하도록 한 점이 명확하고, lowercase 정책을 주석으로 남겨 둔 것도 좋습니다. 이 값이 외부 컨트랙트라는 것만 인지하고, 상태 추가/변경 시 여기만 일관되게 관리하면 될 것 같습니다.backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (2)
19-22: 클래스 설명이 변경된 응답 구조를 잘 반영하고 있습니다.날짜/시간/상태 중심의 간결한 DTO 설계와 일치하는 설명이라 이해하기 수월합니다.
49-54:availableMinutes↔allowedMinutes매핑의 도메인 의미만 재확인해 주세요.
round.getAllowedMinutes()를 그대로availableMinutes로 노출하고 있는데, 이름 변화가 도메인 상 의미 변화(예: “지각 허용 시간” vs “전체 출석 가능 시간”)를 동반하는 것은 아닌지만 한 번 점검하면 좋겠습니다. 구현 자체는 단순 매핑이라 기능적 문제는 없어 보입니다.Also applies to: 70-75
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java
Outdated
Show resolved
Hide resolved
2 participants
Summary by CodeRabbit
릴리스 노트
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.