20260227 #246 AttendanceResponse에 라운드 및 세션 정보 추가#251
Merged
discipline24 merged 2 commits intomainfrom Feb 28, 2026
Hidden character warning
The head ref may contain hidden characters: "20260227-#246-AttendanceResponse-\ub77c\uc6b4\ub4dc-\uc138\uc158-\uc815\ubcf4-\ucd94\uac00"
Merged
Conversation
요약AttendanceResponse DTO에 sessionId, sessionTitle, roundName, roundDate, roundStartAt, roundLocation 필드를 추가하고, 팩토리 메서드를 from(Attendance)에서 from(Attendance, AttendanceSession, AttendanceRound)로 변경했습니다. AttendanceRound 엔티티에 roundName, locationName, qrSecret 필드를 추가하고 attendances 필드를 제거했으며, AttendanceService의 모든 호출 지점을 새로운 메서드 시그니처에 맞게 업데이트했습니다. 변경 사항
예상 코드 리뷰 소요 시간🎯 3 (중간 난이도) | ⏱️ ~20분 관련 가능성이 있는 이슈
관련 가능성이 있는 PR
제안 리뷰어
축하시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (1)
96-103: 동일 라운드 조회 시 불필요한attendance.getAttendanceRound()호출이 메서드는 이미 90번 라인에서
round객체를 가져왔습니다. 모든 출석 기록이 동일한 라운드에 속하므로, 매핑에서attendance.getAttendanceRound()를 다시 호출할 필요 없이 기존round변수를 재사용할 수 있습니다.♻️ 제안된 수정
+ AttendanceSession session = round.getAttendanceSession(); + return attendanceRepository.findByAttendanceRound_RoundId(roundId) .stream() - .map(attendance -> AttendanceResponse.from( - attendance, - attendance.getAttendanceRound() != null ? attendance.getAttendanceRound().getAttendanceSession() : null, - attendance.getAttendanceRound() - )) + .map(attendance -> AttendanceResponse.from(attendance, session, round)) .toList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java` around lines 96 - 103, The mapping unnecessarily calls attendance.getAttendanceRound() for each record even though the method already retrieved the round variable; update the stream mapping in AttendanceService so it reuses the existing round variable: pass round (and round.getAttendanceSession() where the session is needed) into AttendanceResponse.from instead of attendance.getAttendanceRound() and attendance.getAttendanceRound().getAttendanceSession(), keeping the call to attendanceRepository.findByAttendanceRound_RoundId(roundId) and the AttendanceResponse.from invocation but replacing per-attendance round lookups with the outer round reference.backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.java (2)
54-60:locationName필드에@Column어노테이션 누락
roundName과qrSecret은@Column어노테이션이 있지만locationName은 없습니다. nullable 여부와 컬럼 제약조건의 일관성을 위해@Column어노테이션 추가를 권장합니다.♻️ 제안된 수정
`@Column`(name = "round_name", length = 255, nullable = false) private String roundName; // 라운드 이름 (예: "1차 정기모임", "OT" 등) - private String locationName; // 장소 이름 (예: "세종대학교 310동") + `@Column`(name = "location_name") + private String locationName; // 장소 이름 (예: "세종대학교 310동")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.java` around lines 54 - 60, Add a JPA `@Column` annotation for the locationName field in AttendanceRound to make column constraints consistent with roundName/qrSecret; annotate the field (locationName) with `@Column` specifying nullable (e.g., nullable = false or true per domain rules) and a length (e.g., length = 255) so the DB mapping is explicit and consistent with other fields.
65-80:changeStatus메서드의 무시(silent return) 패턴 검토 필요현재 구현은 잘못된 상태 전환(CLOSED에서 변경, ACTIVE→UPCOMING)을 조용히 무시합니다. 이는 호출자가 상태 변경이 실제로 적용되었는지 알 수 없어 디버깅을 어렵게 만들 수 있습니다.
의도적인 설계라면 현재 상태를 유지해도 되지만, 잘못된 전환 시 예외를 던지거나 boolean을 반환하는 방식도 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.java` around lines 65 - 80, The changeStatus method currently silently ignores invalid transitions (e.g., when this.roundStatus == RoundStatus.CLOSED or ACTIVE -> UPCOMING) which hides failures; update AttendanceRound.changeStatus to surface these cases by throwing a clear IllegalStateException (include current and requested statuses in the message) for invalid transitions while still returning early (no-op) when newStatus equals current status to avoid unnecessary DB updates. Locate the changeStatus method and replace the silent returns for CLOSED and ACTIVE->UPCOMING with thrown exceptions (or alternatively change the method to return boolean success and return false on invalid transitions), ensuring callers will be informed of failed state changes.backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceResponse.java (1)
30-50:attendance파라미터 null 체크 고려
session과round에 대한 null 체크는 잘 되어 있지만,attendance파라미터가 null일 경우 NPE가 발생할 수 있습니다. 방어적 코딩을 위해 null 체크를 추가하거나, 메서드 진입점에서 null이 아님을 보장하는 것이 좋습니다.🛡️ 방어적 코딩 예시
public static AttendanceResponse from(Attendance attendance, AttendanceSession session, AttendanceRound round) { + if (attendance == null) { + throw new IllegalArgumentException("attendance must not be null"); + } return new AttendanceResponse(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceResponse.java` around lines 30 - 50, The from(...) factory (AttendanceResponse.from) currently dereferences attendance without checking for null; add a defensive null check at the start of the method (e.g., if attendance == null) and handle it consistently—either throw a clear IllegalArgumentException/NullPointerException with a descriptive message (e.g., "attendance must not be null") or return null according to your API contract—so subsequent calls like attendance.getAttendanceId() in AttendanceResponse.from won't cause an NPE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceResponse.java`:
- Around line 30-50: The from(...) factory (AttendanceResponse.from) currently
dereferences attendance without checking for null; add a defensive null check at
the start of the method (e.g., if attendance == null) and handle it
consistently—either throw a clear IllegalArgumentException/NullPointerException
with a descriptive message (e.g., "attendance must not be null") or return null
according to your API contract—so subsequent calls like
attendance.getAttendanceId() in AttendanceResponse.from won't cause an NPE.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.java`:
- Around line 54-60: Add a JPA `@Column` annotation for the locationName field in
AttendanceRound to make column constraints consistent with roundName/qrSecret;
annotate the field (locationName) with `@Column` specifying nullable (e.g.,
nullable = false or true per domain rules) and a length (e.g., length = 255) so
the DB mapping is explicit and consistent with other fields.
- Around line 65-80: The changeStatus method currently silently ignores invalid
transitions (e.g., when this.roundStatus == RoundStatus.CLOSED or ACTIVE ->
UPCOMING) which hides failures; update AttendanceRound.changeStatus to surface
these cases by throwing a clear IllegalStateException (include current and
requested statuses in the message) for invalid transitions while still returning
early (no-op) when newStatus equals current status to avoid unnecessary DB
updates. Locate the changeStatus method and replace the silent returns for
CLOSED and ACTIVE->UPCOMING with thrown exceptions (or alternatively change the
method to return boolean success and return false on invalid transitions),
ensuring callers will be informed of failed state changes.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java`:
- Around line 96-103: The mapping unnecessarily calls
attendance.getAttendanceRound() for each record even though the method already
retrieved the round variable; update the stream mapping in AttendanceService so
it reuses the existing round variable: pass round (and
round.getAttendanceSession() where the session is needed) into
AttendanceResponse.from instead of attendance.getAttendanceRound() and
attendance.getAttendanceRound().getAttendanceSession(), keeping the call to
attendanceRepository.findByAttendanceRound_RoundId(roundId) and the
AttendanceResponse.from invocation but replacing per-attendance round lookups
with the outer round reference.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceResponse.javabackend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.javabackend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java
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
2 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.
Summary by CodeRabbit
개선 사항