Skip to content

Comments

[feature] 동아리 지원서 관련 기능 구현#406

Merged
Zepelown merged 110 commits intodevelop/befrom
feature/#382-club-application-save
May 21, 2025
Merged

[feature] 동아리 지원서 관련 기능 구현#406
Zepelown merged 110 commits intodevelop/befrom
feature/#382-club-application-save

Conversation

@lepitaaar
Copy link
Contributor

@lepitaaar lepitaaar commented May 14, 2025

#️⃣연관된 이슈

#382

📝작업 내용

동아리 지원서 & 지원하기 구현

중점적으로 리뷰받고 싶은 부분(선택)

클럽 질문 가져올때 코드가 중복됨, 따라서 따로 메서드 분리가 필요할까요??
ClubApplicationQuestion의 Options와 items를 레코드를 엔티티에 바로 연결하는건 좋지않은 것 같습니다 각각 엔티티를 나눠야할까요 아니면 Map 자료형 변경해서 수동으로 Map 만들어서 넣어주는게 좋을까요?
구현 아이디어 있으시면 의견주세요!

논의하고 싶은 부분(선택)

현재 ClubApplication, ClubQuestion 등 지원서 관련 클래스 명이 너무 복잡합니다.. 리네임 명 추천받아요
지원에 관련해서 형식 관련없이 value로만 값을 받고 세세한 글자수 검증은 구지 필요하지 않다고 생각이듭니다. 꼭 지원서 응답에 관한 검증이 필요할까요?

🫡 참고사항

현재 지원서를 단일로 받아 post와 put 메서드 로직이 동일합니다.

Summary by CodeRabbit

  • 신규 기능
    • 동아리 지원서 생성, 수정, 조회, 제출 기능이 추가되었습니다.
    • 다양한 질문 유형(단답형, 장문형, 선택형, 전화번호, 이메일, 이름 등) 및 필수 응답 여부 설정이 가능합니다.
    • 지원서 질문 및 답변에 대한 상세한 유효성 검사와 길이 제한이 적용되었습니다.
    • 지원서 관련 오류 메시지가 세분화되어 사용자에게 명확한 안내를 제공합니다.

Zepelown and others added 27 commits May 3, 2025 13:56
TODO: 모집 정보 테스트코드를 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것
- 1시간마다 DB 확인
- RecruitmentScheduler.java -> RecruitmentStateChecker 이름 변경
Copy link
Collaborator

@Due-IT Due-IT left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다.
커밋기록이 중복돼서 리뷰 이후 변경사항을 파악하기 어려운 감이 있네요.
앞으로 리베이스 사용시 유의해야할거 같습니다 :)

Comment on lines 126 to 144
List<ClubApplicationQuestion> newQuestions = request.questions().stream()
.map(question -> ClubApplicationQuestion.builder()
.id(question.id())
.title(question.title())
.description(question.description())
.type(question.type())
.options(
ClubQuestionOption.builder()
.required(question.options().required())
.build()
)
.items(question.items().stream()
.map(item ->
ClubQuestionItem.builder()
.value(item.value())
.build())
.toList())
.build())
.toList();
Copy link
Collaborator

Choose a reason for hiding this comment

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

builder를 중첩해서 한번에 생성하는 것과 필요한 것들을 준비해서 변수를 통해 생성하는 방법은 어떤 차이가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

가시성에서는 따로 나누는게 오히려 좋을것으로 생각이됩니다. 제가 저렇게 작성한건 이미 stream으로 배열을 다루고있어서 한거였습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

궁금해서 stream일때와 for loop일때 성능 차이를 비교해봤습니다
case 10개
스크린샷 2025-05-19 17 29 28
case 100만개
스크린샷 2025-05-19 17 32 30
오히려 stream을 사용한게 퍼포먼스적으로 느리다는점을 알수있었습니다.

퍼포먼스적으로도 그렇고 가시성에서도 for-loop로 나누는게 좋을듯하여 리팩토링 진행하겠습니다~

참고:
https://sigridjin.medium.com/java-stream-api는-왜-for-loop보다-느릴까-50dec4b9974b

Copy link
Member

@Zepelown Zepelown left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.
커맨트 남겼으니 확인 부탁드립니다.

Comment on lines 20 to 31

@Id
private String id;

private String questionId;

@Builder.Default
private List<ClubQuestionAnswer> answers = new ArrayList<>();

@Builder.Default
LocalDateTime createdAt = ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime();
}
Copy link
Member

Choose a reason for hiding this comment

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

혹시 여기에 status 만 하나 추가해주실 수 있으실까요?
Enum으로 선언하시면 될 것 같습니다.

public enum ApplicationStatus {
    DRAFT,                     // 작성 중
    SUBMITTED,                 // 제출 완료
    SCREENING,                 // 서류 심사 중
    SCREENING_PASSED,          // 서류 통과
    SCREENING_FAILED,          // 서류 탈락
    INTERVIEW_SCHEDULED,       // 면접 일정 확정
    INTERVIEW_IN_PROGRESS,     // 면접 진행 중
    INTERVIEW_PASSED,          // 면접 통과
    INTERVIEW_FAILED,          // 면접 탈락
    OFFERED,                   // 최종 합격 제안
    ACCEPTED,                  // 제안 수락
    DECLINED,                  // 제안 거절
    CANCELED_BY_APPLICANT      // 지원자 자진 철회
}

위 상태처럼 만들어주시면 감사하겠습니다.

Copy link
Contributor

@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

♻️ Duplicate comments (1)
backend/src/main/java/moadong/club/entity/ClubApplication.java (1)

22-38: 🛠️ Refactor suggestion

사용자 ID 필드가 필요합니다.

현재 설계에서는 지원서를 제출한 사용자를 식별할 수 있는 userId 필드가 누락되어 있습니다. 이 필드는 지원서와 사용자를 연결하는 중요한 요소이며, 지원자 관리 및 통계 분석에 필수적입니다.

public class ClubApplication {
    @Id
    private String id;

    private String questionId;
+   
+   private String userId;

    @Enumerated(EnumType.STRING)
    @Builder.Default
    ApplicationStatus status = ApplicationStatus.SUBMITTED;
    
    // ...
}
🧹 Nitpick comments (3)
backend/src/main/java/moadong/club/entity/ClubApplication.java (3)

30-31: 기본 상태값 검토 필요

현재 status의 기본값이 SUBMITTED로 설정되어 있습니다. 일반적인 지원서 워크플로우에서는 사용자가 작성 중인 상태를 의미하는 DRAFT가 초기 상태로 더 적합할 수 있습니다. 사용자가 명시적으로 제출 버튼을 클릭할 때 상태를 SUBMITTED로 변경하는 것이 논리적입니다.

@Enumerated(EnumType.STRING)
@Builder.Default
-ApplicationStatus status = ApplicationStatus.SUBMITTED;
+ApplicationStatus status = ApplicationStatus.DRAFT;

37-37: 접근 제한자 추가 필요

createdAt 필드에 접근 제한자(private)가 누락되어 있어 패키지 수준 접근이 가능합니다. 일관성 유지와 캡슐화를 위해 접근 제한자를 추가하는 것이 좋습니다.

@Builder.Default
-LocalDateTime createdAt = ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime();
+private LocalDateTime createdAt = ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime();

22-23: 클래스에 JavaDoc 주석 추가 권장

ClubApplication 클래스의 목적과 역할에 대한 JavaDoc 주석이 없습니다. 코드 가독성과 유지보수성 향상을 위해 클래스 목적, 주요 필드의 의미, 사용 방법 등을 설명하는 JavaDoc 주석을 추가하는 것이 좋습니다.

+/**
+ * 동아리 지원서 정보를 나타내는 엔티티 클래스입니다.
+ * MongoDB의 "club_applications" 컬렉션에 저장됩니다.
+ * 지원서의 기본 정보와 답변 목록을 포함합니다.
+ */
@Document("club_applications")
@AllArgsConstructor
@Getter
@Builder(toBuilder = true)
public class ClubApplication {
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 476e50b and a79977b.

📒 Files selected for processing (4)
  • backend/src/main/java/moadong/club/entity/ClubApplication.java (1 hunks)
  • backend/src/main/java/moadong/club/enums/ApplicationStatus.java (1 hunks)
  • backend/src/main/java/moadong/club/service/ClubApplyService.java (1 hunks)
  • backend/src/main/java/moadong/global/exception/ErrorCode.java (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • backend/src/main/java/moadong/club/enums/ApplicationStatus.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/src/main/java/moadong/club/service/ClubApplyService.java
  • backend/src/main/java/moadong/global/exception/ErrorCode.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/main/java/moadong/club/entity/ClubApplication.java (2)
backend/src/main/java/moadong/club/entity/ClubApplicationQuestion.java (1)
  • AllArgsConstructor (13-37)
backend/src/main/java/moadong/club/entity/ClubQuestion.java (1)
  • Document (16-53)

Copy link
Collaborator

@PororoAndFriends PororoAndFriends left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

Copy link
Member

@Zepelown Zepelown left a comment

Choose a reason for hiding this comment

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

고생하셨습니다

@Zepelown Zepelown merged commit e8ca199 into develop/be May 21, 2025
3 checks passed
@lepitaaar lepitaaar deleted the feature/#382-club-application-save branch September 20, 2025 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants