Skip to content
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

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

kckc0608
Copy link
Member

@kckc0608 kckc0608 commented Feb 26, 2025

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • 스터디 출석체크 API 작성

📝 참고사항

  • 기존에는 StudyDetail 에 있는 출석체크 날짜를 기반으로 검증했는데, 바뀐 명세에 따라 StudySession 에 있는 LessonPeriod 기반으로 출석 가능 기간 검증을 진행했습니다.
  • 일단 기존 api 명세 따라서 /studies 로 뒀는데, studyId 없이 바로 /attend 로 끝내고 study session id 를 쿼리로 받으니까 어색한 것 같더라구요. /study-sessions 로 해볼까 해도 그때는 쿼리 파라미터보다 패스 파라미터로 받아서 /attend 로 끝내는 게 더 자연스러울 것 같고, /study-histories 로 빼면 과거 수강 이력에 대한 api 와 섞이는 것 같아 고민되는데 좋은 네이밍 아이디어 있을까요..

📚 기타

Summary by CodeRabbit

Summary by CodeRabbit

  • 새로운 기능
    • 스터디 출석 체크 기능이 새롭게 도입되어, 학생들이 출석 기록을 쉽게 제출할 수 있습니다.
    • 출석번호 검증, 출석 기간 확인 및 중복 체크 등의 다양한 검증 로직이 적용되어 보다 안정적인 출석 관리가 가능합니다.
    • 출석 관련 오류 상황에 대해 명확한 피드백을 제공함으로써 사용자 경험이 개선되었습니다.

@kckc0608 kckc0608 self-assigned this Feb 26, 2025
@kckc0608 kckc0608 requested a review from a team as a code owner February 26, 2025 18:11
Copy link

coderabbitai bot commented Feb 26, 2025

📝 Walkthrough

Walkthrough

이번 PR은 학생 출석체크 기능을 위한 신규 기능 추가를 포함합니다. 새로 추가된 컨트롤러와 서비스는 HTTP 요청을 처리하고 비즈니스 로직을 수행하며, 출석 관련 데이터의 검증 후 저장하는 전체 플로우를 담당합니다. 이에 따라 Repository, Domain Service, DTO 등 관련 컴포넌트와 에러코드까지 함께 도입되어 출석 처리의 일관성을 확보합니다.

Changes

파일 경로 요약 변경 사항
src/main/java/.../api/StudentStudyControllerV2.java REST 컨트롤러 추가. POST /attend 엔드포인트로 출석 요청을 처리하고, StudentStudyServiceV2로 위임함.
src/main/java/.../application/StudentStudyServiceV2.java 서비스 클래스 추가. 현재 회원 조회, 스터디/세션 검증, 출석 여부 확인, 검증 후 출석 데이터 생성 및 저장 로직 구현.
src/main/java/.../dao/AttendanceV2Repository.java JPA Repository 인터페이스 추가. 학생과 스터디 세션으로 출석 기록 존재 여부를 판단하는 메서드 구현.
src/main/java/.../domain/AttendanceValidatorV2.java 도메인 서비스 추가. 출석 기간, 출석 번호, 중복 출석, 스터디 지원 여부 등 여러 검증 조건을 확인하고 예외를 처리하는 로직 구현.
src/main/java/.../dto/request/AttendanceCreateRequest.java DTO 레코드 추가. 출석 번호 필드를 포함하며, 유효성 검사를 위한 애노테이션(@notblank, @pattern)과 API 문서화를 위한 @Schema 적용.
src/main/java/.../global/exception/ErrorCode.java ErrorCode enum에 ATTENDANCE_PERIOD_INVALID 추가. 출석 체크 가능한 강의 시간 외의 요청에 대한 예외 상황을 정의함.

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
Loading

Possibly related PRs

Suggested labels

✨ feature

Poem

나는 코드 숲을 달리는 작은 토끼,
새로운 출석 기능에 발을 맞추네,
Controller와 Service가 서로 손잡고,
Repository와 Validator가 빛을 더해,
에러코드까지 깔끔하게 정리하며,
모두 함께 즐겁게 뛰노는 날,
🐇✨ CodeRabbit의 행복한 발자국!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a957b84 and 19f2c89.

📒 Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2)

141-141: 새로운 에러 코드 추가가 적절합니다.

강의 시간에 대한 검증을 위한 새로운 에러 코드를 추가한 것은 좋은 접근입니다. 이는 PR 목표에 명시된 대로 StudyDetail의 출석 체크 날짜에서 StudySessionLessonPeriod로 검증 방식을 변경하는 것과 일치합니다.


144-144: 이미 출석 처리된 스터디 회차에 대한 에러 처리가 적절합니다.

중복 출석 체크를 방지하기 위한 에러 코드 추가가 적절히 이루어졌습니다.

✨ Finishing Touches
  • 📝 Generate Docstrings

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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 its LessonPeriod, 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 receive studyId, studySessionId, and AttendanceCreateRequest.
    • The controller uses the StudentStudyServiceV2 to process the attendance check.
  • 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.
  • src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/AttendanceV2Repository.java
    • Created AttendanceV2Repository interface to manage AttendanceV2 entities.
    • Added a method to check if a student has already attended a specific study session.
  • src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java
    • Added existsByStudentAndStudy method to check if a student has applied to a study.
  • 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.
  • 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.
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
    • Added ATTENDANCE_PERIOD_INVALID error code for invalid attendance periods.
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

  1. 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.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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.

Comment on lines 40 to 42
.filter(studySession -> Objects.equals(studySession.getId(), studySessionId))
.findFirst()
.orElseThrow(() -> new CustomException(STUDY_NOT_UPDATABLE_SESSION_NOT_FOUND));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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));

Comment on lines +11 to +13
@Pattern(regexp = ATTENDANCE_NUMBER, message = "출석번호는 " + ATTENDANCE_NUMBER + " 형식이어야 합니다.")
@Schema(description = "출석번호")
String attendanceNumber) {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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 + " 형식이어야 합니다.";
}

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between c45e9b2 and 23e6554.

📒 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 오류 코드 추가는 StudySessionLessonPeriod를 기반으로 출석 체크의 유효성을 검증하는 새로운 로직을 잘 지원합니다. 기존의 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에서 StudySessionLessonPeriod로 전환하여 검증하는 기능"이 잘 구현되어 있습니다.

@kckc0608 kckc0608 changed the title featt: 스터디 출석체크 V2 API 작성 feat: 스터디 출석체크 V2 API 작성 Feb 26, 2025
Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a 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: 변수명 및 로직 의도 명확화 권장
isAppliedToStudyisAlreadyAttended라는 변수명은 간결하지만, 로직 의도를 더 명확하게 드러내려면 hasJoinedStudyhasAttendedSession 등으로 표현을 고려해 보세요.


50-51: 새 엔티티 생성 후 반환값 처리 고려
출석 정보를 생성한 뒤 바로 저장만 하고 있습니다. 이후 작업을 위해 필요한 경우 save() 이후 생성된 엔티티(예: 출석 ID)를 반환하는 방안을 고려해 보시면 좋겠습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23e6554 and 733bfd0.

📒 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() 내부에서 이미 출석 여부를 검사하더라도, 동시에 여러 요청이 들어올 경우 중복 출석을 막는 로직이 충분히 처리되는지 재확인이 필요합니다.

Copy link
Member

@Sangwook02 Sangwook02 left a 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STUDY_DETAIL_ALREADY_ATTENDED도 수정이 필요해 보입니다!

Comment on lines +41 to +44
Optional<StudyHistoryV2> optionalStudyHistory =
studyHistoryV2Repository.findByStudentAndStudy(currentMember, study);

boolean isAppliedToStudy = optionalStudyHistory.isPresent();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 exists 메서드로 처리하지 않은 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

원래 exists 로 처리했었는데, #938 로직과 비슷하게 가져가는 게 가독성이 좋을 것 같아서 통일했습니다

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

938에서 exists를 사용하지 않은 이유는 존재 여부를 검증한 뒤 -> 내부 엔티티 정보를 나중에 get() 으로 가져와야 했기 때문입니다. 여기서는 엔티티 내부 정보가 필요없고 존재성만 검증하며 되기 때문에 exists를 사용하는 게 좋을 듯 합니다.

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a 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: 검증 로직 순서 개선 고려

현재 검증 순서는 논리적이지만, 성능 최적화를 위해 가장 빠르게 실패할 수 있는 검증을 먼저 수행하는 것이 좋습니다. 예를 들어, 데이터베이스 조회 없이 검증할 수 있는 isAlreadyAttendedattendanceNumber 검증을 먼저 수행하면 성능상 이점이 있을 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 733bfd0 and a957b84.

📒 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_ATTENDEDSTUDY_SESSION_ALREADY_ATTENDED로 수정해야 합니다. 현재 코드에서는 이미 STUDY_SESSION_ALREADY_ATTENDED를 사용하고 있으므로, 이와 관련된 ErrorCode 클래스의 상수 정의도 확인해주세요.

Copy link
Member

@Sangwook02 Sangwook02 left a 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, "이미 출석 처리된 스터디입니다."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
STUDY_SESSION_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디입니다."),
STUDY_SESSION_ALREADY_ATTENDED(HttpStatus.CONFLICT, "이미 출석 처리된 스터디 회차입니다."),

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

}

// 스터디 신청 여부 검증
if (!isAppliedToStudy) {
Copy link
Member

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()) {
Copy link
Member

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를 바깥에서 받아야 합니다.
과제 제출하기 로직 참고해주세요.

Copy link
Member

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이제 봤는데 에러코드 네이밍이 조금 이상하네요.
출석기한 이내에 있지 않는데, 이 에러코드만 보면 출석기한 자체가 invalid한 것처럼 네이밍이 되어있어요. 여기서 처리할 내용은 아닌 것 같고 투두 남기고 이슈 별도로 파면 좋을 것 같습니다.

@uwoobeat
Copy link
Member

일단 기존 api 명세 따라서 /studies 로 뒀는데, studyId 없이 바로 /attend 로 끝내고 study session id 를 쿼리로 받으니까 어색한 것 같더라구요.

-> 저번 과제 제출 PR 네이밍 참고해주세요. 네이밍이 스터디 쪽 API스럽지 않은 문제는 앞에 /study 세그먼트를 붙이거나 하는 식으로 고민 중이긴 한데 이것도 나중에 시간 나면 논의해보면 좋을 것 같습니다.

/study-sessions 로 해볼까 해도 그때는 쿼리 파라미터보다 패스 파라미터로 받아서 /attend 로 끝내는 게 더 자연스러울 것 같고,

-> 스터디회차는 스터디 애그리거트의 자식 엔티티이므로, 단독으로 노출할 수 없습니다.
저번 PR에서 설명했듯이 애그리거트 당 하나의 레포지터리만이 존재하고, 스터디 애그리거트는 스터디 루트 엔티티와 스터디세션 자식 엔티티로 구성되기 때문에, 스터디세션만 단독으로 노출하는 것을 지양해야 합니다. /study-details 엔드포인트를 제거하기로 논의했던 것과 마찬가지로 /study-session 도 존재해서는 안됩니다.

/study-histories 로 빼면 과거 수강 이력에 대한 api 와 섞이는 것 같아 고민되는데 좋은 네이밍 아이디어 있을까요..

-> 앞에서 말했듯 (일반적으로) 애그리거트 하나 당 리소스 하나로 보고 있으니 해당 컨벤션을 따르면 될 것 같습니다. 출석이력은 수강이력과 별개입니다.

Comment on lines +41 to +44
Optional<StudyHistoryV2> optionalStudyHistory =
studyHistoryV2Repository.findByStudentAndStudy(currentMember, study);

boolean isAppliedToStudy = optionalStudyHistory.isPresent();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

938에서 exists를 사용하지 않은 이유는 존재 여부를 검증한 뒤 -> 내부 엔티티 정보를 나중에 get() 으로 가져와야 했기 때문입니다. 여기서는 엔티티 내부 정보가 필요없고 존재성만 검증하며 되기 때문에 exists를 사용하는 게 좋을 듯 합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 스터디 출석체크 V2 API 구현
3 participants