-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 스터디 출석체크 V2 API 작성 #942
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthrough이번 PR은 학생 출석체크 기능을 위한 신규 기능 추가를 포함합니다. 새로 추가된 컨트롤러와 서비스는 HTTP 요청을 처리하고 비즈니스 로직을 수행하며, 출석 관련 데이터의 검증 후 저장하는 전체 플로우를 담당합니다. 이에 따라 Repository, Domain Service, DTO 등 관련 컴포넌트와 에러코드까지 함께 도입되어 출석 처리의 일관성을 확보합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant Ctrl as StudentStudyControllerV2
participant Svc as StudentStudyServiceV2
participant Val as AttendanceValidatorV2
participant Repo as AttendanceV2Repository
participant Util as MemberUtil
participant Log as Logger
C->>Ctrl: POST /v2/studies/attend\n(studySessionId, AttendanceCreateRequest)
Ctrl->>Svc: attend(studySessionId, request)
Svc->>Util: getCurrentMember()
Svc->>Svc: find StudyV2 and StudySessionV2
Svc->>Val: validateAttendance(studySession, attendanceNumber, isApplied, isAttended)
Svc->>Repo: save(AttendanceV2)
Svc->>Log: info(출석 체크 정보 기록)
Svc-->>Ctrl: HTTP 200 OK
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @kckc0608, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request implements the Study Attendance Check V2 API. It includes a new controller, service, repository interfaces, a validator, and a request DTO. The changes involve validating attendance based on the StudySession
's LessonPeriod
instead of the StudyDetail
's attendance date, as per the updated specifications. It also includes checks for study application status and existing attendance records.
Highlights
- New API Endpoint: A new API endpoint
/v2/studies/{studyId}/attend
is created to handle student attendance checks for studies. - Attendance Validation: Attendance validation is now based on
StudySession
and itsLessonPeriod
, ensuring checks for attendance period, attendance number, study application status, and existing attendance records. - Data Access Layers: New repository interfaces (
AttendanceV2Repository
,StudyHistoryV2Repository
) are added to handle data access for attendance and study history.
Changelog
Click here to see the changelog
- src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyControllerV2.java
- Created a new
StudentStudyControllerV2
to handle the attendance check API endpoint. - Implemented the
attend
method to receivestudyId
,studySessionId
, andAttendanceCreateRequest
. - The controller uses the
StudentStudyServiceV2
to process the attendance check.
- Created a new
- src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java
- Created a new
StudentStudyServiceV2
to handle the business logic for attendance checks. - Implemented the
attend
method to validate and record student attendance. - The service retrieves study and session information, validates attendance, and saves the attendance record.
- Created a new
- src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/AttendanceV2Repository.java
- Created
AttendanceV2Repository
interface to manageAttendanceV2
entities. - Added a method to check if a student has already attended a specific study session.
- Created
- src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java
- Added
existsByStudentAndStudy
method to check if a student has applied to a study.
- Added
- src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java
- Created
AttendanceValidatorV2
to encapsulate attendance validation logic. - Implemented
validateAttendance
method to check attendance period, number, study application, and existing attendance.
- Created
- src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/AttendanceCreateRequest.java
- Created
AttendanceCreateRequest
record to encapsulate the attendance number. - Added validation constraints to ensure the attendance number matches the expected format.
- Created
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
- Added
ATTENDANCE_PERIOD_INVALID
error code for invalid attendance periods.
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Trivia time!
What is the purpose of the @Transactional
annotation in Spring?
Click here for the answer
The `@Transactional` annotation in Spring is used to define the scope of a transaction. When a method is annotated with `@Transactional`, Spring automatically begins a new transaction before the method execution and commits the transaction upon successful completion of the method. In the event of an exception, the transaction is rolled back.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The code changes introduce a new API endpoint for student attendance check-in. The implementation is generally well-structured, but I've left a few suggestions for improvement. It's recommended to address these comments before requesting a review from someone else. Feel free to request another review from Gemini via /gemini review
when you have addressed these comments and I'll take another look! Remember to have others review and approve this code before merging.
Summary of Findings
- Exception Handling: Consider centralizing exception handling or providing more specific error messages for better debugging.
- Code Readability: Improve code readability by adding comments to explain complex logic or non-obvious operations.
- Logging: Ensure logging is consistent and provides sufficient information for monitoring and troubleshooting.
Assessment
The code introduces a new API endpoint for student attendance check-in, which is a valuable addition to the application. The implementation appears to be well-structured and includes necessary validations. However, there are a few areas where improvements can be made to enhance code clarity and maintainability. Addressing these comments before merging is recommended, and it's crucial to have another team member review and approve this code before merging.
.filter(studySession -> Objects.equals(studySession.getId(), studySessionId)) | ||
.findFirst() | ||
.orElseThrow(() -> new CustomException(STUDY_NOT_UPDATABLE_SESSION_NOT_FOUND)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a more descriptive message to the CustomException
to provide better context when the StudySession
is not found. This can aid in debugging and troubleshooting.
For example, include the studyId
and studySessionId
in the error message.
.orElseThrow(() -> new CustomException(STUDY_NOT_UPDATABLE_SESSION_NOT_FOUND, "StudySession not found for studyId: " + studyId + ", studySessionId: " + studySessionId));
@Pattern(regexp = ATTENDANCE_NUMBER, message = "출석번호는 " + ATTENDANCE_NUMBER + " 형식이어야 합니다.") | ||
@Schema(description = "출석번호") | ||
String attendanceNumber) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting the message "출석번호는 " + ATTENDANCE_NUMBER + " 형식이어야 합니다." to a constant to avoid duplication and improve maintainability. This also makes it easier to update the message in one place if needed.
import static com.gdschongik.gdsc.global.common.constant.RegexConstant.ATTENDANCE_NUMBER;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Pattern;
public record AttendanceCreateRequest(
@NotBlank
@Pattern(regexp = ATTENDANCE_NUMBER, message = AttendanceCreateRequest.ATTENDANCE_NUMBER_MESSAGE)
@Schema(description = "출석번호")
String attendanceNumber) {
private static final String ATTENDANCE_NUMBER_MESSAGE = "출석번호는 " + ATTENDANCE_NUMBER + " 형식이어야 합니다.";
}
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyControllerV2.java (1)
17-33
: 컨트롤러 구현이 잘 되어 있습니다.컨트롤러 구현이 RESTful 원칙을 잘 따르고 있으며, 관심사 분리가 명확합니다. 컨트롤러는 HTTP 요청 처리만 담당하고 비즈니스 로직은 서비스 계층에 위임하고 있습니다.
한 가지 개선점으로는 Swagger 문서에 발생 가능한 오류 응답에 대한 정보를 추가하는 것이 좋겠습니다. API 사용자가 어떤 에러코드를 기대할 수 있는지 알 수 있도록
@ApiResponse
애노테이션을 추가하는 것을 고려해보세요.src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java (1)
14-35
: 검증 로직이 명확하게 구현되어 있습니다.검증 메서드가 책임 분리 원칙에 따라 명확하게 구현되어 있으며 필요한 모든 검증을 수행하고 있습니다.
다만 26번 라인의 주석이 실제 검증 내용과 일치하지 않습니다. 해당 부분은 출석체크 번호를 검증하는 것이 아니라 출석 중복 여부를 검증하는 로직입니다.
주석을 다음과 같이 수정하는 것이 좋겠습니다:
- // 출석체크 번호 검증 + // 출석 중복 검증 if (isAlreadyAttended) { throw new CustomException(STUDY_DETAIL_ALREADY_ATTENDED); }src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java (1)
39-42
: 스터디 세션 조회 방식 개선 제안현재 코드는 스터디를 조회한 후 메모리에서 특정 세션을 필터링하는 방식으로 구현되어 있습니다. 스터디 세션이 많은 경우 성능 저하가 발생할 수 있습니다.
레포지토리에 스터디 ID와 세션 ID로 직접 세션을 조회하는 메서드를 추가하여 DB 수준에서 필터링하는 것이 더 효율적일 수 있습니다.
// StudySessionV2Repository에 다음 메서드 추가 제안 Optional<StudySessionV2> findByIdAndStudyId(Long sessionId, Long studyId);그리고 서비스 코드를 다음과 같이 수정:
- StudySessionV2 session = study.getStudySessions().stream() - .filter(studySession -> Objects.equals(studySession.getId(), studySessionId)) - .findFirst() - .orElseThrow(() -> new CustomException(STUDY_NOT_UPDATABLE_SESSION_NOT_FOUND)); + StudySessionV2 session = studySessionV2Repository.findByIdAndStudyId(studySessionId, studyId) + .orElseThrow(() -> new CustomException(STUDY_NOT_UPDATABLE_SESSION_NOT_FOUND));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyControllerV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/AttendanceV2Repository.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/AttendanceCreateRequest.java
(1 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
(1 hunks)
🔇 Additional comments (5)
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
140-140
: 적절한 오류 코드 추가.
ATTENDANCE_PERIOD_INVALID
오류 코드 추가는StudySession
의LessonPeriod
를 기반으로 출석 체크의 유효성을 검증하는 새로운 로직을 잘 지원합니다. 기존의ATTENDANCE_DATE_INVALID
와 함께 출석 체크에 대한 보다 세밀한 검증이 가능해졌습니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java (1)
12-12
: 메서드 추가 적절.
existsByStudentAndStudy
메서드는 Spring Data JPA 명명 규칙을 잘 따르고 있으며, 기존findByStudentAndStudy
메서드와 일관성을 유지합니다. 특정 학생과 스터디의 연결 여부를 확인하기 위한 효율적인 방법을 제공합니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/AttendanceCreateRequest.java (1)
1-13
: 유효성 검증이 포함된 적절한 요청 클래스.출석번호에 대한 유효성 검증(
@NotBlank
,@Pattern
)을 포함하고 있어 잘못된 입력을 방지할 수 있습니다.@Schema
어노테이션을 통해 API 문서화도 잘 지원하고 있습니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/AttendanceV2Repository.java (1)
8-11
: 적절한 저장소 인터페이스 구현.
AttendanceV2Repository
인터페이스는 Spring Data JPA 규칙을 잘 따르고 있으며,existsByStudentAndStudySession
메서드는 특정 학생과 스터디 세션에 대한 출석 여부를 효율적으로 확인할 수 있게 해줍니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java (1)
34-57
: 트랜잭션 처리 및 비즈니스 로직 구현이 잘 되어 있습니다.
attend
메서드의 전반적인 구현이 좋습니다. 적절한 트랜잭션 경계 설정, 예외 처리, 로깅 등이 잘 구현되어 있습니다. 특히 검증 로직을 별도의 클래스로 분리하여 단일 책임 원칙을 잘 지키고 있습니다.API의 핵심 목표인 "출석체크 날짜를
StudyDetail
에서StudySession
의LessonPeriod
로 전환하여 검증하는 기능"이 잘 구현되어 있습니다.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java (2)
44-45
: 변수명 및 로직 의도 명확화 권장
isAppliedToStudy
와isAlreadyAttended
라는 변수명은 간결하지만, 로직 의도를 더 명확하게 드러내려면hasJoinedStudy
나hasAttendedSession
등으로 표현을 고려해 보세요.
50-51
: 새 엔티티 생성 후 반환값 처리 고려
출석 정보를 생성한 뒤 바로 저장만 하고 있습니다. 이후 작업을 위해 필요한 경우save()
이후 생성된 엔티티(예: 출석 ID)를 반환하는 방안을 고려해 보시면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyControllerV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
- src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyControllerV2.java
🔇 Additional comments (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyServiceV2.java (3)
39-39
: 에러 메시지에 식별자를 포함하는 것 고려하기
이미 이전 리뷰에서 제안되었듯이,CustomException(STUDY_NOT_FOUND)
를 던질 때studySessionId
또는studyId
를 포함한 구체적인 에러 메시지를 제공하면 문제 파악에 유리합니다.
40-40
:getStudySession
결과 검증 필요성 확인
study.getStudySession(studySessionId)
호출 시, 내부적으로 세션이 존재하지 않는 경우 예외가 던져지는지, 또는 null이 반환되는지 확인이 필요합니다. null 반환 시 예외 처리가 누락될 수 있습니다.
47-48
: 동시성 처리 여부 확인
attendanceValidatorV2.validateAttendance()
내부에서 이미 출석 여부를 검사하더라도, 동시에 여러 요청이 들어올 경우 중복 출석을 막는 로직이 충분히 처리되는지 재확인이 필요합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다~
|
||
// 출석체크 번호 검증 | ||
if (isAlreadyAttended) { | ||
throw new CustomException(STUDY_DETAIL_ALREADY_ATTENDED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STUDY_DETAIL_ALREADY_ATTENDED
도 수정이 필요해 보입니다!
Optional<StudyHistoryV2> optionalStudyHistory = | ||
studyHistoryV2Repository.findByStudentAndStudy(currentMember, study); | ||
|
||
boolean isAppliedToStudy = optionalStudyHistory.isPresent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 exists 메서드로 처리하지 않은 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
원래 exists 로 처리했었는데, #938 로직과 비슷하게 가져가는 게 가독성이 좋을 것 같아서 통일했습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
938에서 exists를 사용하지 않은 이유는 존재 여부를 검증한 뒤 -> 내부 엔티티 정보를 나중에 get()
으로 가져와야 했기 때문입니다. 여기서는 엔티티 내부 정보가 필요없고 존재성만 검증하며 되기 때문에 exists를 사용하는 게 좋을 듯 합니다.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java (3)
26-27
: 중복된 주석을 수정해주세요.두번째 검증 로직의 주석이 "출석체크 번호 검증"으로 첫번째 검증 로직과 동일합니다. 실제로는 출석 중복 여부를 확인하는 로직이므로 해당 주석을 "출석체크 중복 검증"과 같이 수정하는 것이 좋겠습니다.
- // 출석체크 번호 검증 + // 출석체크 중복 검증 if (isAlreadyAttended) { throw new CustomException(STUDY_SESSION_ALREADY_ATTENDED); }
14-15
: 파라미터 명명 개선 고려출석 유효성 검증에 사용되는 불리언 파라미터들의 이름(
isAppliedToStudy
,isAlreadyAttended
)이 의미를 잘 전달하고 있습니다만, 더 일관된 네이밍을 위해isEnrolledInStudy
와 같이 수정하는 것도 고려해 볼 수 있습니다.
14-35
: 검증 로직 순서 개선 고려현재 검증 순서는 논리적이지만, 성능 최적화를 위해 가장 빠르게 실패할 수 있는 검증을 먼저 수행하는 것이 좋습니다. 예를 들어, 데이터베이스 조회 없이 검증할 수 있는
isAlreadyAttended
나attendanceNumber
검증을 먼저 수행하면 성능상 이점이 있을 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AttendanceValidatorV2.java (2)
1-36
: 코드 구조가 잘 정리되어 있습니다.도메인 서비스로서 출석 유효성 검증 로직이 캡슐화되어 있고, 각 검증 단계마다 적절한 예외 처리가 되어 있어 좋습니다.
DomainService
어노테이션을 통해 레이어 구분이 명확합니다.
27-29
: ErrorCode 수정 필요이전 리뷰에서 언급된 것처럼
STUDY_DETAIL_ALREADY_ATTENDED
를STUDY_SESSION_ALREADY_ATTENDED
로 수정해야 합니다. 현재 코드에서는 이미STUDY_SESSION_ALREADY_ATTENDED
를 사용하고 있으므로, 이와 관련된 ErrorCode 클래스의 상수 정의도 확인해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하나만 더 확인해주세요
미리 어푸루브 해뒀습니다
ATTENDANCE_NUMBER_MISMATCH(HttpStatus.CONFLICT, "출석번호가 일치하지 않습니다."), | ||
STUDY_DETAIL_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디입니다."), | ||
STUDY_SESSION_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디입니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STUDY_SESSION_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디입니다."), | |
STUDY_SESSION_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디 회차입니다."), |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
} | ||
|
||
// 스터디 신청 여부 검증 | ||
if (!isAppliedToStudy) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로직 상 변경은 아니지만, 신청여부 -> 중복출석 -> 출석가능여부 -> 출석번호 순으로 검증하는 게 의미 상으로 더 적절할 것 같습니다
public void validateAttendance( | ||
StudySessionV2 studySession, String attendanceNumber, boolean isAppliedToStudy, boolean isAlreadyAttended) { | ||
// 출석체크 기간 검증 | ||
if (!studySession.getLessonPeriod().isOpen()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isOpen
은 deprecation 처리하는 게 좋아보입니다.
대신 isWithin
같이 유즈케이스로부터 독립적인 메서드를 추가하고, now를 인자로 받아서 처리하면 좋을 것 같습니다. '주어진 값이 시작일시와 종료일시 사이에 있다' 라는 걸 '기간이 열려있다' 라고 하지는 않잖아요?
'열려있다' 라는 건 Recruitment 도메인 구현하면서 잘못 추출된 워딩입니다.
해당 모집회차가 '열려있다' 라는 표현이 여기로 넘어온 것이죠.
'기한'은 '열려있다' 라는 표현보다는 '이내에 있다' 라는 표현이 적절하고, 따라서 isWithin
이 적절한 네이밍으로 보입니다. 사실 전에 assignmentPeriod 구현하면서 별도로 이슈 파서 추가할까 고민했는데 선반영하면 좋을 것 같아서 남깁니다.
그리고 기존 isOpen
로직은 오른쪽 끝에 대해서 inclusive하게 동작하는데, exclusive하게 구현을 변경하는 게 더 올바를 것 같아요. 1일 00시 제출 건을 받게 되어버리면 일자가 바뀌기 때문에...
한편 도메인 서비스도 마찬가지로 now를 바깥에서 받아야 합니다.
과제 제출하기 로직 참고해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> 이 부분 #913 하면서 제가 처리하겠습니다.
StudySessionV2 studySession, String attendanceNumber, boolean isAppliedToStudy, boolean isAlreadyAttended) { | ||
// 출석체크 기간 검증 | ||
if (!studySession.getLessonPeriod().isOpen()) { | ||
throw new CustomException(ATTENDANCE_PERIOD_INVALID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제 봤는데 에러코드 네이밍이 조금 이상하네요.
출석기한 이내에 있지 않는데, 이 에러코드만 보면 출석기한 자체가 invalid한 것처럼 네이밍이 되어있어요. 여기서 처리할 내용은 아닌 것 같고 투두 남기고 이슈 별도로 파면 좋을 것 같습니다.
-> 저번 과제 제출 PR 네이밍 참고해주세요. 네이밍이 스터디 쪽 API스럽지 않은 문제는 앞에
-> 스터디회차는 스터디 애그리거트의 자식 엔티티이므로, 단독으로 노출할 수 없습니다.
-> 앞에서 말했듯 (일반적으로) 애그리거트 하나 당 리소스 하나로 보고 있으니 해당 컨벤션을 따르면 될 것 같습니다. 출석이력은 수강이력과 별개입니다. |
Optional<StudyHistoryV2> optionalStudyHistory = | ||
studyHistoryV2Repository.findByStudentAndStudy(currentMember, study); | ||
|
||
boolean isAppliedToStudy = optionalStudyHistory.isPresent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
938에서 exists를 사용하지 않은 이유는 존재 여부를 검증한 뒤 -> 내부 엔티티 정보를 나중에 get()
으로 가져와야 했기 때문입니다. 여기서는 엔티티 내부 정보가 필요없고 존재성만 검증하며 되기 때문에 exists를 사용하는 게 좋을 듯 합니다.
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
StudyDetail
에 있는 출석체크 날짜를 기반으로 검증했는데, 바뀐 명세에 따라StudySession
에 있는LessonPeriod
기반으로 출석 가능 기간 검증을 진행했습니다./studies
로 뒀는데, studyId 없이 바로/attend
로 끝내고 study session id 를 쿼리로 받으니까 어색한 것 같더라구요./study-sessions
로 해볼까 해도 그때는 쿼리 파라미터보다 패스 파라미터로 받아서/attend
로 끝내는 게 더 자연스러울 것 같고,/study-histories
로 빼면 과거 수강 이력에 대한 api 와 섞이는 것 같아 고민되는데 좋은 네이밍 아이디어 있을까요..📚 기타
Summary by CodeRabbit
Summary by CodeRabbit