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

[DDING-107] 최종 합격 지원자 동아리원 명단 등록 API 구현 #247

Merged
merged 10 commits into from
Feb 10, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ddingdong.ddingdongBE.domain.clubmember.entity.ClubMember;
import ddingdong.ddingdongBE.domain.scorehistory.entity.Score;
import ddingdong.ddingdongBE.domain.user.entity.User;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Embedded;
import jakarta.persistence.Entity;
Expand Down Expand Up @@ -40,7 +41,7 @@ public class Club extends BaseEntity {
@JoinColumn(name = "user_id")
private User user;

@OneToMany(mappedBy = "club")
@OneToMany(mappedBy = "club", cascade = CascadeType.ALL)
private List<ClubMember> clubMembers = new ArrayList<>();

private String name;
Expand Down Expand Up @@ -122,4 +123,9 @@ public BigDecimal editScore(Score score) {
this.score = score;
return this.score.getValue();
}

public void addClubMember(ClubMember clubMember) {
this.clubMembers.add(clubMember);
clubMember.setClubFormConvenience(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,8 @@ public void updateInformation(ClubMember updateClubMember) {
this.position = updateClubMember.getPosition();
this.department = updateClubMember.getDepartment();
}

public void setClubFormConvenience(Club club) {
this.club = club;
}
Comment on lines +73 to +75
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

메서드 이름과 유효성 검사 개선 필요

메서드 이름이 목적을 명확하게 표현하지 않으며, club 매개변수에 대한 null 검사가 누락되었습니다.

다음과 같이 개선하는 것을 제안합니다:

-    public void setClubFormConvenience(Club club) {
+    public void setClub(Club club) {
+        if (club == null) {
+            throw new IllegalArgumentException("Club cannot be null");
+        }
         this.club = club;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void setClubFormConvenience(Club club) {
this.club = club;
}
public void setClub(Club club) {
if (club == null) {
throw new IllegalArgumentException("Club cannot be null");
}
this.club = club;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,11 @@ FormStatisticsResponse getFormStatistics(
@PathVariable("formId") Long formId,
@AuthenticationPrincipal PrincipalDetails principalDetails
);

@Operation(summary = "동아리 최종 합격 지원자 동아리원 명단 등록API")
@ApiResponse(responseCode = "201", description = "최종 합격 지원자 동아리원 명단 등록 성공")
@ResponseStatus(HttpStatus.CREATED)
@SecurityRequirement(name = "AccessToken")
@PostMapping("/my/forms/{formId}/members/register-applicants")
void registerMembers(@PathVariable("formId") Long formId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,9 @@ public FormStatisticsResponse getFormStatistics(
FormStatisticsQuery query = facadeCentralFormService.getStatisticsByForm(user, formId);
return FormStatisticsResponse.from(query);
}

@Override
public void registerMembers(Long formId) {
facadeCentralFormService.registerApplicantAsMember(formId);
}
}
86 changes: 43 additions & 43 deletions src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,56 +23,56 @@
@Getter
public class Form extends BaseEntity {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(nullable = false)
private String title;
@Column(nullable = false)
private String title;

private String description;
private String description;

@Column(nullable = false)
private LocalDate startDate;
@Column(nullable = false)
private LocalDate startDate;

@Column(nullable = false)
private LocalDate endDate;
@Column(nullable = false)
private LocalDate endDate;

@Column(nullable = false)
private boolean hasInterview;
@Column(nullable = false)
private boolean hasInterview;

@Column(nullable = false)
@Convert(converter = StringListConverter.class)
private List<String> sections;
@Column(nullable = false)
@Convert(converter = StringListConverter.class)
private List<String> sections;

@ManyToOne(fetch = FetchType.LAZY)
private Club club;
@ManyToOne(fetch = FetchType.LAZY)
private Club club;

@Builder
private Form(
String title,
String description,
LocalDate startDate,
LocalDate endDate,
boolean hasInterview,
List<String> sections,
Club club
) {
this.title = title;
this.description = description;
this.startDate = startDate;
this.endDate = endDate;
this.hasInterview = hasInterview;
this.sections = sections;
this.club = club;
}
@Builder
private Form(
String title,
String description,
LocalDate startDate,
LocalDate endDate,
boolean hasInterview,
List<String> sections,
Club club
) {
this.title = title;
this.description = description;
this.startDate = startDate;
this.endDate = endDate;
this.hasInterview = hasInterview;
this.sections = sections;
this.club = club;
}

public void update(Form updateForm) {
this.title = updateForm.getTitle();
this.description = updateForm.getDescription();
this.startDate = updateForm.getStartDate();
this.endDate = updateForm.getEndDate();
this.sections = updateForm.getSections();
this.hasInterview = updateForm.isHasInterview();
}
public void update(Form updateForm) {
this.title = updateForm.getTitle();
this.description = updateForm.getDescription();
this.startDate = updateForm.getStartDate();
this.endDate = updateForm.getEndDate();
this.sections = updateForm.getSections();
this.hasInterview = updateForm.isHasInterview();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@ public interface FacadeCentralFormService {
FormQuery getForm(Long formId);

FormStatisticsQuery getStatisticsByForm(User user, Long formId);

void registerApplicantAsMember(Long formId);
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package ddingdong.ddingdongBE.domain.form.service;

import static ddingdong.ddingdongBE.domain.club.entity.Position.MEMBER;

import ddingdong.ddingdongBE.common.exception.AuthenticationException.NonHaveAuthority;
import ddingdong.ddingdongBE.common.exception.InvalidatedMappingException.InvalidFormPeriodException;
import ddingdong.ddingdongBE.common.utils.TimeUtils;
import ddingdong.ddingdongBE.domain.club.entity.Club;
import ddingdong.ddingdongBE.domain.club.service.ClubService;
import ddingdong.ddingdongBE.domain.clubmember.entity.ClubMember;
import ddingdong.ddingdongBE.domain.form.entity.Form;
import ddingdong.ddingdongBE.domain.form.entity.FormField;
import ddingdong.ddingdongBE.domain.form.service.dto.command.CreateFormCommand;
Expand All @@ -17,6 +20,8 @@
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormStatisticsQuery.ApplicantStatisticQuery;
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormStatisticsQuery.DepartmentStatisticQuery;
import ddingdong.ddingdongBE.domain.form.service.dto.query.FormStatisticsQuery.FieldStatisticsQuery;
import ddingdong.ddingdongBE.domain.formapplication.entity.FormApplication;
import ddingdong.ddingdongBE.domain.formapplication.service.FormApplicationService;
import ddingdong.ddingdongBE.domain.user.entity.User;
import java.time.LocalDate;
import java.util.List;
Expand All @@ -34,6 +39,7 @@ public class FacadeCentralFormServiceImpl implements FacadeCentralFormService {
private final FormFieldService formFieldService;
private final ClubService clubService;
private final FormStatisticService formStatisticService;
private final FormApplicationService formApplicationService;

@Transactional
@Override
Expand Down Expand Up @@ -102,6 +108,23 @@ public FormStatisticsQuery getStatisticsByForm(User user, Long formId) {
return new FormStatisticsQuery(totalCount, departmentStatisticQueries, applicantStatisticQueries, fieldStatisticsQuery);
}

@Override
@Transactional
public void registerApplicantAsMember(Long formId) {
List<FormApplication> finalPassedFormApplications = formApplicationService.getAllFinalPassedByFormId(formId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2)
현재 hasInterView라는 필드를 통해 서류만 볼 수 있도록 해놨고,
서류 통과는 FINAL_PASS가 아닌 FIRST_PASS인데, 이 부분도 고려해야할 것 같습니다.

제 의견은 상태 수정 API에서 hasInterview가 false일 경우 서류 통과시 First pass가 아닌 final pass로 변경되는게 좋아보입니다

Copy link
Collaborator Author

@5uhwann 5uhwann Feb 9, 2025

Choose a reason for hiding this comment

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

hasInterview가 false일 경우 합격 처리가 되면 FINAL_PASS로 변경이 아닌 FIRST_PASS라는 뜻인가요???

최종 합격 상태

  1. hasInterview = true인 경우: FINAL_PASS가 최종 합격
  2. hasInterview = false인 경우: FIRST_PASS가 최종 합격

이게 맞을까요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

네 최종합격을 판단하는 부분에서 위처럼 해도 좋을 것 같습니다.

finalPassedFormApplications.forEach(formApplication -> {
Club club = formApplication.getForm().getClub();
ClubMember clubMember = ClubMember.builder()
.name(formApplication.getName())
.studentNumber(formApplication.getStudentNumber())
.department(formApplication.getDepartment())
.phoneNumber(formApplication.getPhoneNumber())
.position(MEMBER)
.build();
club.addClubMember(clubMember);
});
}
Comment on lines +111 to +126
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

보안 및 유효성 검사 추가 필요

다음 사항들을 고려해야 합니다:

  1. 현재 사용자가 해당 폼의 클럽에 대한 권한이 있는지 확인이 필요합니다
  2. 이미 등록된 동아리원인지 중복 검사가 필요합니다

다음과 같이 수정하는 것을 제안합니다:

    @Override
    @Transactional
    public void registerApplicantAsMember(Long formId) {
+        Form form = formService.getById(formId);
+        Club club = form.getClub();
+        User currentUser = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+        validateEqualsClub(club, form);

        List<FormApplication> finalPassedFormApplications = formApplicationService.getAllFinalPassedByFormId(formId);
        finalPassedFormApplications.forEach(formApplication -> {
-            Club club = formApplication.getForm().getClub();
+            if (isDuplicateClubMember(club, formApplication.getStudentNumber())) {
+                throw new DuplicateClubMemberException(formApplication.getStudentNumber());
+            }
            ClubMember clubMember = ClubMember.builder()
                    .name(formApplication.getName())
                    .studentNumber(formApplication.getStudentNumber())
                    .department(formApplication.getDepartment())
                    .phoneNumber(formApplication.getPhoneNumber())
                    .position(MEMBER)
                    .build();
            club.addClubMember(clubMember);
        });
    }

+    private boolean isDuplicateClubMember(Club club, String studentNumber) {
+        return club.getClubMembers().stream()
+            .anyMatch(member -> member.getStudentNumber().equals(studentNumber));
+    }

Committable suggestion skipped: line range outside the PR's diff.


private FormListQuery buildFormListQuery(Form form) {
boolean isActive = TimeUtils.isDateInRange(LocalDate.now(), form.getStartDate(), form.getEndDate());
return FormListQuery.from(form, isActive);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,45 @@
@Getter
public class FormApplication extends BaseEntity {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(nullable = false)
private String name;

@Column(nullable = false)
private String studentNumber;

@Column(nullable = false)
private String department;

@Enumerated(EnumType.STRING)
@Column(nullable = false, name = "status")
private FormApplicationStatus status;

@ManyToOne(fetch = FetchType.LAZY)
private Form form;

@Builder
private FormApplication(String name, String studentNumber, String department,
FormApplicationStatus status, Form form) {
this.name = name;
this.studentNumber = studentNumber;
this.department = department;
this.status = status;
this.form = form;
}

public void updateStatus(FormApplicationStatus status) {
this.status = status;
}
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(nullable = false)
private String name;

@Column(nullable = false)
private String studentNumber;

@Column(nullable = false)
private String department;

@Column(nullable = false)
private String phoneNumber;

@Column(nullable = false)
private String email;

Comment on lines +29 to +34
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

전화번호와 이메일 필드에 유효성 검증이 필요합니다.

전화번호와 이메일 형식에 대한 유효성 검증이 없습니다. 잘못된 형식의 데이터가 저장될 수 있으므로 다음과 같은 검증을 추가하는 것이 좋겠습니다:

+    @Pattern(regexp = "^\\d{3}-\\d{3,4}-\\d{4}$", message = "올바른 전화번호 형식이 아닙니다.")
     @Column(nullable = false)
     private String phoneNumber;

+    @Email(message = "올바른 이메일 형식이 아닙니다.")
     @Column(nullable = false)
     private String email;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Column(nullable = false)
private String phoneNumber;
@Column(nullable = false)
private String email;
@Pattern(regexp = "^\\d{3}-\\d{3,4}-\\d{4}$", message = "올바른 전화번호 형식이 아닙니다.")
@Column(nullable = false)
private String phoneNumber;
@Email(message = "올바른 이메일 형식이 아닙니다.")
@Column(nullable = false)
private String email;

@Enumerated(EnumType.STRING)
@Column(nullable = false, name = "status")
private FormApplicationStatus status;

@ManyToOne(fetch = FetchType.LAZY)
private Form form;

@Builder
private FormApplication(String name, String studentNumber, String department, String phoneNumber, String email,
FormApplicationStatus status, Form form) {
this.name = name;
this.studentNumber = studentNumber;
this.department = department;
this.phoneNumber = phoneNumber;
this.email = email;
this.status = status;
this.form = form;
}

public void updateStatus(FormApplicationStatus status) {
this.status = status;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,11 @@ List<RecentFormInfo> findRecentFormByDateWithApplicationCount(
@Param("date") LocalDate date,
@Param("size") int size
);
@Query(value = """
select fa
from FormApplication fa
where fa.form.id = :formId and (fa.status = 'FINAL_PASS' or (fa.status = 'FIRST_PASS' and fa.form.hasInterview = false))
""")
List<FormApplication> findAllFinalPassedByFormId(@Param("formId") Long formId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

public interface FormApplicationService {

FormApplication create(FormApplication formApplication);
FormApplication create(FormApplication formApplication);

Slice<FormApplication> getFormApplicationPageByFormId(Long formId, int size,
Long currentCursorId);
Slice<FormApplication> getFormApplicationPageByFormId(Long formId, int size, Long currentCursorId);

FormApplication getById(Long applicationId);
FormApplication getById(Long applicationId);

List<FormApplication> getAllById(List<Long> applicationIds);
List<FormApplication> getAllById(List<Long> applicationIds);

List<FormApplication> getAllFinalPassedByFormId(Long formId);
}
Loading
Loading