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: Recruitment 생성 API 구현 #345

Merged
merged 21 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
444386b
test: 모집기간이 중복되는 케이스에 대한 검증 추가
Sangwook02 May 17, 2024
157743b
test: 서비스를 테스트 하도록 수정
Sangwook02 May 21, 2024
823db4c
rename: ErrorCode 수정
Sangwook02 May 21, 2024
77db96c
resolve: merge conflict
Sangwook02 May 22, 2024
e688ec2
style: spotless apply
Sangwook02 May 22, 2024
11604e9
test: 모집 기간 중복을 검증하기 위한 테스트 추가
Sangwook02 May 22, 2024
3ab5f5a
feat: Recruitment 생성 api 구현
Sangwook02 May 22, 2024
b617659
docs: 용어 변경
Sangwook02 May 26, 2024
5c0ac34
refactor: 학년도와 학기를 클라이언트로부터 입력받도록 수정
Sangwook02 May 29, 2024
de7d072
test: 리쿠르팅 생성 로직에 대한 테스트 추가
Sangwook02 Jun 1, 2024
d9182d4
feat: 리쿠르트먼트 생성 로직 검증 메서드 추가
Sangwook02 Jun 1, 2024
9e969ae
refactor: SemesterType이 학기 시작일을 상수로 가지도록 수정
Sangwook02 Jun 2, 2024
afe6a55
style: 개행 제거
Sangwook02 Jun 2, 2024
5bad8d5
refactor: 메서드 분리 복구
Sangwook02 Jun 2, 2024
2c9d15e
feat: 시간과 기간에 대한 상수 클래스 추가
Sangwook02 Jun 2, 2024
c6f6d3a
refactor: 학기 타입 매핑 로직을 리쿠르팅 서비스로 이동
Sangwook02 Jun 3, 2024
88d0290
rename: ErrorCode 수정
Sangwook02 Jun 3, 2024
4d6daa6
fix: api 경로 수정
Sangwook02 Jun 4, 2024
1b6e87d
rename: 상수 이름 수정
Sangwook02 Jun 4, 2024
4aa1ea5
refactor: 상수를 java.time.Month로 대체
Sangwook02 Jun 4, 2024
862cd80
fix: ErrorCode 메시지 수정
Sangwook02 Jun 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package com.gdschongik.gdsc.domain.common.model;

import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.global.exception.ErrorCode;
import java.time.LocalDateTime;
import java.time.Month;
import lombok.AllArgsConstructor;
import lombok.Getter;

Expand All @@ -10,4 +14,30 @@ public enum SemesterType {
SECOND("2학기");

private final String value;

public static SemesterType from(LocalDateTime dateTime) {
Copy link
Member

Choose a reason for hiding this comment

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

해당 메서드가 적절한 책임을 가지지 못하는 것 같습니다.

저라면 이 메서드를 보고 "대충 enum 멤버로 학기 시작일이 있으니까, 인자로 받은 dateTime이 학기 시작일 ~ 종료일 사이이면 해당 학기타입을 반환하겠구나?" 라고 생각할 것 같아요.

그런데 실제 로직을 보면 학기 시작일 2주 전에 해당하는 dateTime도 해당 학기타입으로 매핑해주고 있습니다. 이렇게 하면 처음 보는 사람은 조금 혼란스러울 것 같다는 생각이 듭니다.

저희가 이렇게 했던 이유가 뭐였나요? 인자로 들어오는 값이 리쿠르팅의 시작일과 마감일이었기 때문이었죠.

여기서 두 가지 문제가 있는데요,

  1. SemesterType의 내부 로직이, 이러한 리쿠르팅에 의존적인 컨텍스트 정보를 알 필요가 없고, 안다 하더라도 리쿠르팅이 가져야 할 책임을 학기타입이 가져가는 것이기 때문에 SRP 위반이고요,
  2. SemesterType::from 으로 들어오는 값이 항상 리쿠르팅의 시작일과 마감일이라는 보장이 없다는 겁니다. 다른 로직에서 언제든지 실수하고 아, LocalDateTime을 SemesterType으로 매핑해주는 정적 메서드구나 하고 쓸 여지가 많습니다.

2번의 경우 인자를 Recruitment로 받거나 하면 해결될 수 있지만 1번을 해결 못했기 때문에 근본적인 해결책은 아니죠.
결국 학기타입 매핑 로직은 리쿠르팅 서비스로 빼내는 것이 적절해보입니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇네요.
저도 SemesterType.from이 항상 LocalDateTime만 받는게 맞을지, 그리고 받는 LocalDateTime에 대해 항상 같은 로직으로 SemesterType을 반환하는게 맞을지에 대한 고민이 있었는데 서비스로 옮기니 마음에 걸리던데 해소된 느낌이네요👍

return getSemesterType(dateTime);
Copy link
Member

Choose a reason for hiding this comment

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

분리하신 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 조건도 추출해서 의미있는 이름으로 만들면 좋을듯 합니다.

분리하자는 의미 아니었나요?

}

private static SemesterType getSemesterType(LocalDateTime dateTime) {
int year = dateTime.getYear();
LocalDateTime firstSemesterStartDate = LocalDateTime.of(year, 3, 1, 0, 0, 0);
Copy link
Member

Choose a reason for hiding this comment

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

매직 넘버는 상수로 추출해주세요~
ex) 3월 1일, 2주, 7월

Copy link
Member Author

Choose a reason for hiding this comment

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

3월 1일과 9월 1은 아래와 같이 처리하면 좋을 것 같네요👍👍

public enum SemesterType {
    FIRST("1학기", MonthDay.of(3, 1)),
    SECOND("2학기", MonthDay.of(9, 1));

    private final String value;
    private final MonthDay startDate;
}

LocalDateTime secondSemesterStartDate = LocalDateTime.of(year, 9, 1, 0, 0, 0);

if (dateTime.isAfter(firstSemesterStartDate.minusWeeks(2)) && dateTime.getMonthValue() < 7) {
return FIRST;
}

if (dateTime.isAfter(secondSemesterStartDate.minusWeeks(2))) {
return SECOND;
}
throw new CustomException(ErrorCode.SEMESTER_TYPE_INVALID_FOR_DATE);
}

public static Month getStartMonth(SemesterType semesterType) {
if (semesterType == FIRST) {
return Month.MARCH;
}
return Month.SEPTEMBER;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.gdschongik.gdsc.domain.recruitment.api;

import com.gdschongik.gdsc.domain.recruitment.application.AdminRecruitmentService;
import com.gdschongik.gdsc.domain.recruitment.dto.request.RecruitmentCreateRequest;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@Tag(name = "Admin Recruitment", description = "어드민 리쿠르팅 관리 API입니다.")
@RestController
Copy link
Member

Choose a reason for hiding this comment

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

저희 근데 리크루팅으로 하자고 하지 않았나용? 기억이 안나네욥

Copy link
Member Author

@Sangwook02 Sangwook02 May 26, 2024

Choose a reason for hiding this comment

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

너무 길다고 '리쿠르팅' 쓰기로 했던 것 같아요.
테이블 명이랑 안 맞는게 좀 아쉽긴하지만 swagger 문서에는 모두 리쿠르팅으로 변경하겠습니다.

++ 영문은 recruitment로 두는게 api 경로랑 혼동이 적겠죠?

Copy link
Member

Choose a reason for hiding this comment

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

넵넵 리크루팅은 단지 프론트/기획/백엔드 용어 통일을 위해 정한 것 같고 영어로 recruitment는 이게 리쿠르팅이다 라는것을 저희가 백끼리 이해만 하면될거같아요

@RequestMapping("/admin/recruitment")
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
@RequestMapping("/admin/recruitment")
@RequestMapping("/admin/recruitments")

@RequiredArgsConstructor
public class AdminRecruitmentController {

private final AdminRecruitmentService adminRecruitmentService;

@Operation(summary = "리쿠르팅 생성", description = "새로운 리쿠르팅(모집 기간)를 생성합니다.")
@PostMapping
public ResponseEntity<Void> createRecruitment(@Valid @RequestBody RecruitmentCreateRequest request) {
adminRecruitmentService.createRecruitment(request);
return ResponseEntity.ok().build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package com.gdschongik.gdsc.domain.recruitment.application;

import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

import com.gdschongik.gdsc.domain.common.model.SemesterType;
import com.gdschongik.gdsc.domain.recruitment.dao.RecruitmentRepository;
import com.gdschongik.gdsc.domain.recruitment.domain.Recruitment;
import com.gdschongik.gdsc.domain.recruitment.dto.request.RecruitmentCreateRequest;
import com.gdschongik.gdsc.global.exception.CustomException;
import java.time.LocalDateTime;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class AdminRecruitmentService {

private final RecruitmentRepository recruitmentRepository;

@Transactional
public void createRecruitment(RecruitmentCreateRequest request) {
validatePeriodMatchesAcademicYear(request.startDate(), request.endDate(), request.academicYear());
validatePeriodMatchesSemesterType(request.startDate(), request.endDate(), request.semesterType());
validatePeriodWithinTwoWeeks(
request.startDate(), request.endDate(), request.academicYear(), request.semesterType());
validatePeriodOverlap(request.academicYear(), request.semesterType(), request.startDate(), request.endDate());

Recruitment recruitment = Recruitment.createRecruitment(
request.name(), request.startDate(), request.endDate(), request.academicYear(), request.semesterType());
recruitmentRepository.save(recruitment);
// todo: recruitment 모집 시작 직전에 멤버 역할 수정하는 로직 필요.
}

private void validatePeriodMatchesAcademicYear(
LocalDateTime startDate, LocalDateTime endDate, Integer academicYear) {
if (academicYear.equals(startDate.getYear()) && academicYear.equals(endDate.getYear())) {
return;
}

throw new CustomException(RECRUITMENT_PERIOD_MISMATCH_ACADEMIC_YEAR);
}

private void validatePeriodMatchesSemesterType(
LocalDateTime startDate, LocalDateTime endDate, SemesterType semesterType) {
if (SemesterType.from(startDate).equals(semesterType)
&& SemesterType.from(endDate).equals(semesterType)) {
return;
}

throw new CustomException(RECRUITMENT_PERIOD_MISMATCH_SEMESTER_TYPE);
}

private void validatePeriodWithinTwoWeeks(
LocalDateTime startDate, LocalDateTime endDate, Integer academicYear, SemesterType semesterType) {
LocalDateTime semesterStartDate =
LocalDateTime.of(academicYear, SemesterType.getStartMonth(semesterType), 1, 0, 0);

if (semesterStartDate.minusWeeks(2).isAfter(startDate)
|| semesterStartDate.plusWeeks(2).isBefore(startDate)) {
throw new CustomException(RECRUITMENT_PERIOD_NOT_WITHIN_TWO_WEEKS);
}

if (semesterStartDate.minusWeeks(2).isAfter(endDate)
|| semesterStartDate.plusWeeks(2).isBefore(endDate)) {
throw new CustomException(RECRUITMENT_PERIOD_NOT_WITHIN_TWO_WEEKS);
}
}

private void validatePeriodOverlap(
Integer academicYear, SemesterType semesterType, LocalDateTime startDate, LocalDateTime endDate) {
List<Recruitment> recruitments =
recruitmentRepository.findAllByAcademicYearAndSemesterType(academicYear, semesterType);

recruitments.forEach(recruitment -> recruitment.validatePeriodOverlap(startDate, endDate));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package com.gdschongik.gdsc.domain.recruitment.dao;

import com.gdschongik.gdsc.domain.common.model.SemesterType;
import com.gdschongik.gdsc.domain.recruitment.domain.Recruitment;
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;

public interface RecruitmentRepository extends JpaRepository<Recruitment, Long>, RecruitmentCustomRepository {}
public interface RecruitmentRepository extends JpaRepository<Recruitment, Long>, RecruitmentCustomRepository {

List<Recruitment> findAllByAcademicYearAndSemesterType(Integer academicYear, SemesterType semesterType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,8 @@ public static Recruitment createRecruitment(
public boolean isOpen() {
return this.period.isOpen();
}

public void validatePeriodOverlap(LocalDateTime startDate, LocalDateTime endDate) {
this.period.validatePeriodOverlap(startDate, endDate);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.gdschongik.gdsc.domain.recruitment.domain.vo;

import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.global.exception.ErrorCode;
import jakarta.persistence.Embeddable;
import java.time.LocalDateTime;
import java.util.Objects;
Expand Down Expand Up @@ -31,7 +32,7 @@ public static Period createPeriod(LocalDateTime startDate, LocalDateTime endDate

private static void validatePeriod(LocalDateTime startDate, LocalDateTime endDate) {
if (startDate.isAfter(endDate) || startDate.isEqual(endDate)) {
throw new CustomException(ErrorCode.DATE_PRECEDENCE_INVALID);
throw new CustomException(DATE_PRECEDENCE_INVALID);
}
}

Expand All @@ -41,6 +42,13 @@ public boolean isOpen() {
&& (now.isBefore(this.endDate) || now.isEqual(startDate));
}

public void validatePeriodOverlap(LocalDateTime startDate, LocalDateTime endDate) {
if (this.endDate.isBefore(startDate) || this.startDate.isAfter(endDate)) {
return;
}
throw new CustomException(RECRUITMENT_PERIOD_OVERLAP);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.gdschongik.gdsc.domain.recruitment.dto.request;

import static com.gdschongik.gdsc.global.common.constant.RegexConstant.*;

import com.gdschongik.gdsc.domain.common.model.SemesterType;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.Future;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import java.time.LocalDateTime;

public record RecruitmentCreateRequest(
@NotBlank @Schema(description = "이름") String name,
Copy link
Member

Choose a reason for hiding this comment

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

학년 / 학기 정보를 클라이언트로부터 받는게 낫지 않을까요?
그리고 시작일 / 종료일이 해당 학년 / 학기 정보에 대하여 유효한지 validate만 하면 될 것 같습니다.

@Future @Schema(description = "모집기간 시작일", pattern = DATETIME) LocalDateTime startDate,
@Future @Schema(description = "모집기간 종료일", pattern = DATETIME) LocalDateTime endDate,
@NotNull(message = "학년도는 null이 될 수 없습니다.") @Schema(description = "학년도", pattern = ACADEMIC_YEAR)
Integer academicYear,
@NotNull(message = "학기는 null이 될 수 없습니다.") @Schema(description = "학기") SemesterType semesterType) {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ public class RegexConstant {
public static final String NICKNAME = "[ㄱ-ㅣ가-힣]{1,6}$";
public static final String DEPARTMENT = "^D[0-9]{3}$";
public static final String HONGIK_EMAIL = "^[^\\W&=+'-+,<>]+(\\.[^\\W&=+'-+,<>]+)*@g\\.hongik\\.ac\\.kr$";
public static final String DATETIME = "yyyy-MM-dd'T'HH:mm:ss";
public static final String ACADEMIC_YEAR = "^[0-9]{4}$";

private RegexConstant() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public enum ErrorCode {
DISCORD_NICKNAME_NOTNULL(HttpStatus.INTERNAL_SERVER_ERROR, "닉네임은 빈 값이 될 수 없습니다."),
DISCORD_MEMBER_NOT_FOUND(HttpStatus.NOT_FOUND, "디스코드 멤버를 찾을 수 없습니다."),

// SemesterType
SEMESTER_TYPE_INVALID_FOR_DATE(HttpStatus.CONFLICT, "해당 날짜가 포함되는 학기가 없습니다."),

// Membership
PAYMENT_NOT_VERIFIED(HttpStatus.CONFLICT, "회비 납부가 완료되지 않았습니다."),
MEMBERSHIP_NOT_APPLICABLE(HttpStatus.CONFLICT, "멤버십 가입을 신청할 수 없는 회원입니다."),
Expand All @@ -66,7 +69,11 @@ public enum ErrorCode {
// Recruitment
DATE_PRECEDENCE_INVALID(HttpStatus.BAD_REQUEST, "종료일이 시작일과 같거나 앞설 수 없습니다."),
RECRUITMENT_NOT_OPEN(HttpStatus.CONFLICT, "리크루트먼트 모집기간이 아닙니다."),
RECRUITMENT_NOT_FOUND(HttpStatus.NOT_FOUND, "열려있는 리크루트먼트가 없습니다.");
RECRUITMENT_NOT_FOUND(HttpStatus.NOT_FOUND, "열려있는 리크루트먼트가 없습니다."),
Copy link
Member

Choose a reason for hiding this comment

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

한글 워딩은 '리쿠르팅'으로 통일해야 합니다. 해당 건은 별도 이슈로 작업하셔도 됩니다

Copy link
Member Author

Choose a reason for hiding this comment

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

전범위로 검색해보니 여기에만 있네요
자잘한 부분이니 여기서 처리할게요
꼼꼼히 봐주셔서 감사드려요

RECRUITMENT_PERIOD_OVERLAP(HttpStatus.BAD_REQUEST, "모집 기간이 중복됩니다."),
RECRUITMENT_PERIOD_MISMATCH_ACADEMIC_YEAR(HttpStatus.BAD_REQUEST, "모집 시작일과 종료일의 연도가 학년도와 일치하지 않습니다."),
RECRUITMENT_PERIOD_MISMATCH_SEMESTER_TYPE(HttpStatus.BAD_REQUEST, "모집 시작일과 종료일의 입력된 학기가 일치하지 않습니다."),
RECRUITMENT_PERIOD_NOT_WITHIN_TWO_WEEKS(HttpStatus.BAD_REQUEST, "모집 시작일과 종료일이 학기 시작일로부터 2주 이내에 있지 않습니다.");

private final HttpStatus status;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.gdschongik.gdsc.domain.recruitment.application;

import static com.gdschongik.gdsc.global.common.constant.RecruitmentConstant.*;
import static com.gdschongik.gdsc.global.exception.ErrorCode.*;
import static org.assertj.core.api.Assertions.*;

import com.gdschongik.gdsc.domain.common.model.SemesterType;
import com.gdschongik.gdsc.domain.recruitment.dao.RecruitmentRepository;
import com.gdschongik.gdsc.domain.recruitment.domain.Recruitment;
import com.gdschongik.gdsc.domain.recruitment.dto.request.RecruitmentCreateRequest;
import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.integration.IntegrationTest;
import java.time.LocalDateTime;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

class AdminRecruitmentServiceTest extends IntegrationTest {

@Autowired
private AdminRecruitmentService adminRecruitmentService;

@Autowired
private RecruitmentRepository recruitmentRepository;

private void createRecruitment() {
Recruitment recruitment =
Recruitment.createRecruitment(RECRUITMENT_NAME, START_DATE, END_DATE, ACADEMIC_YEAR, SEMESTER_TYPE);
recruitmentRepository.save(recruitment);
}

@Nested
class 모집기간_생성시 {
@Test
void 기간이_중복되는_Recruitment가_있다면_실패한다() {
// given
createRecruitment();
RecruitmentCreateRequest request =
new RecruitmentCreateRequest(RECRUITMENT_NAME, START_DATE, END_DATE, ACADEMIC_YEAR, SEMESTER_TYPE);

// when & then
assertThatThrownBy(() -> adminRecruitmentService.createRecruitment(request))
.isInstanceOf(CustomException.class)
.hasMessage(RECRUITMENT_PERIOD_OVERLAP.getMessage());
}

@Test
void 모집_시작일과_종료일의_연도가_입력된_학년도와_다르다면_실패한다() {
// given
RecruitmentCreateRequest request =
new RecruitmentCreateRequest(RECRUITMENT_NAME, START_DATE, END_DATE, 2025, SEMESTER_TYPE);

// when & then
assertThatThrownBy(() -> adminRecruitmentService.createRecruitment(request))
.isInstanceOf(CustomException.class)
.hasMessage(RECRUITMENT_PERIOD_MISMATCH_ACADEMIC_YEAR.getMessage());
}

@Test
void 모집_시작일과_종료일의_학기가_입력된_학기와_다르다면_실패한다() {
// given
RecruitmentCreateRequest request = new RecruitmentCreateRequest(
RECRUITMENT_NAME, START_DATE, END_DATE, ACADEMIC_YEAR, SemesterType.SECOND);

// when & then
assertThatThrownBy(() -> adminRecruitmentService.createRecruitment(request))
.isInstanceOf(CustomException.class)
.hasMessage(RECRUITMENT_PERIOD_MISMATCH_SEMESTER_TYPE.getMessage());
}

@Test
void 모집_시작일과_종료일이_학기_시작일로부터_2주_이내에_있지_않다면_실패한다() {
// given
RecruitmentCreateRequest request = new RecruitmentCreateRequest(
RECRUITMENT_NAME, START_DATE, LocalDateTime.of(2024, 4, 10, 00, 00), ACADEMIC_YEAR, SEMESTER_TYPE);

// when & then
assertThatThrownBy(() -> adminRecruitmentService.createRecruitment(request))
.isInstanceOf(CustomException.class)
.hasMessage(RECRUITMENT_PERIOD_NOT_WITHIN_TWO_WEEKS.getMessage());
}
}
}