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

refactor: Period를 외부에서 생성된 상태로 받도록 수정 #793

Merged
merged 6 commits into from
Oct 12, 2024

Conversation

uwoobeat
Copy link
Member

@uwoobeat uwoobeat commented Oct 1, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

📝 참고사항

📚 기타

Summary by CodeRabbit

  • 새로운 기능

    • 모집 라운드 생성 및 업데이트 로직 개선: 기간 정보를 Period 객체로 캡슐화하여 날짜 처리 간소화.
    • 새로운 상수 START_TO_END_PERIOD, ROUND_TWO_START_TO_END_PERIOD, STUDY_ONGOING_PERIOD 추가로 기간 관리 개선.
  • 버그 수정

    • 모집 라운드 생성 시 유효성 검사 로직 강화.
  • 문서화

    • 테스트 케이스에서 Period 사용을 통해 코드 가독성 및 유지보수성 향상.

@uwoobeat uwoobeat requested a review from a team as a code owner October 1, 2024 14:08
@uwoobeat uwoobeat self-assigned this Oct 1, 2024
Copy link

coderabbitai bot commented Oct 1, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

이번 변경 사항은 RecruitmentRound 객체의 생성 및 업데이트 로직을 개선하는 데 중점을 두고 있습니다. Period 객체를 사용하여 모집 기간을 캡슐화하고, 기존의 LocalDate 매개변수 대신 Period를 사용하도록 메서드 시그니처를 수정했습니다. 이로 인해 모집 라운드의 생성 및 업데이트 과정이 더 명확해지고, 관련 데이터가 보다 잘 관리됩니다.

Changes

파일 경로 변경 요약
src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java, src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRound.java RecruitmentRound 객체 생성 시 Period 객체 사용으로 변경, 메서드 시그니처 업데이트
src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RoundType.java Lombok의 @AllArgsConstructor@RequiredArgsConstructor로 변경
src/test/java/com/gdschongik/gdsc/domain/member/domain/MemberValidatorTest.java, src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipTest.java, src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipValidatorTest.java, src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java, src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRoundValidatorTest.java RecruitmentRound 생성 및 업데이트 테스트에서 Period 객체 사용으로 변경
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java, src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java Period 객체를 사용하는 새로운 상수 추가
src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java, src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java Period 객체 생성 방식 변경

Sequence Diagram(s)

sequenceDiagram
    participant AdminRecruitmentService
    participant RecruitmentRound
    participant Period

    AdminRecruitmentService->>RecruitmentRound: create(name, Period, recruitment, roundType)
    RecruitmentRound->>Period: createPeriod(startDate, endDate)
    Period-->>RecruitmentRound: return Period
    RecruitmentRound-->>AdminRecruitmentService: return RecruitmentRound
Loading

Assessment against linked issues

Objective Addressed Explanation
Period를 외부에서 생성된 상태로 받도록 수정 ( #792 )
테스트 상수에 PERIOD 추가 ( #792 )

Possibly related PRs

Suggested labels

♻️ refactor

Suggested reviewers

  • Sangwook02
  • seulgi99
  • AlmondBreez3

🐰 변화의 바람이 불어와
모집 라운드가 새로워져
기간을 담은 Period
더 명확하게 나아가네!
모두 함께 손잡고
더 나은 길을 걸어가자! 🌼


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

github-actions bot commented Oct 1, 2024

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

Period의 create메서드 리네이밍은 다른 이슈에서 하나요?

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: 3

🧹 Outside diff range and nitpick comments (12)
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (1)

16-16: 새로운 상수가 적절히 추가되었습니다.

START_TO_END_PERIOD 상수가 PR의 목적에 맞게 잘 추가되었습니다. 이는 Period를 외부에서 생성하여 사용하는 방식으로 변경하는 데 도움이 될 것입니다.

다만, 상수의 이름을 더 명확하게 만들기 위해 다음과 같이 변경하는 것을 고려해 보시기 바랍니다:

- public static final Period START_TO_END_PERIOD = Period.createPeriod(START_DATE, END_DATE);
+ public static final Period RECRUITMENT_PERIOD = Period.createPeriod(START_DATE, END_DATE);

이렇게 하면 이 기간이 모집 기간임을 더 명확히 나타낼 수 있습니다.

src/test/java/com/gdschongik/gdsc/domain/member/domain/MemberValidatorTest.java (1)

32-32: 테스트 기간 정의가 개선되었습니다.

RecruitmentRoundPeriod 생성을 현재 시간 기준 전후 1일로 변경한 것은 좋은 개선입니다. 이는 테스트 기간을 더 명확하게 정의하여 테스트의 신뢰성을 높입니다.

가독성을 더욱 높이기 위해 다음과 같이 상수를 사용하는 것을 고려해 보세요:

private static final int ONE_DAY = 1;
Period.createPeriod(now.minusDays(ONE_DAY), now.plusDays(ONE_DAY))

이렇게 하면 기간의 의미가 더 명확해질 것입니다.

src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (2)

30-30: 새로운 상수가 PR 목표에 맞게 추가되었습니다.

STUDY_ONGOING_PERIOD 상수가 PR의 목표에 맞게 적절히 추가되었습니다. 이는 Period를 미리 구성된 상태로 받는 방식으로 변경하는 데 도움이 됩니다.

가독성을 더욱 높이기 위해, 상수 선언 위에 간단한 주석을 추가하는 것이 어떨까요? 예를 들면:

// 8주 동안의 스터디 기간
public static final Period STUDY_ONGOING_PERIOD = Period.createPeriod(STUDY_START_DATETIME, STUDY_END_DATETIME);

이렇게 하면 상수의 의미와 용도를 더 명확히 이해할 수 있을 것 같습니다.


Line range hint 1-52: 전반적인 변경 사항이 PR 목표를 잘 달성했습니다.

이번 변경으로 Period를 외부에서 생성된 상태로 받을 수 있게 되었습니다. 이는 PR의 주요 목표를 달성하고 있으며, 코드의 책임 분리를 개선하는 데 도움이 됩니다.

주요 변경 사항:

  1. Period 클래스 import 추가
  2. STUDY_ONGOING_PERIOD 상수 추가

이러한 변경은 기존 코드 스타일과 일관성을 유지하면서도 목표한 리팩토링을 달성했습니다. 테스트 상수를 사용하여 Period를 생성함으로써, 테스트의 일관성과 유지보수성도 향상되었습니다.

향후 개선 사항으로, 이러한 방식을 다른 부분에도 적용하여 전체적인 코드 구조를 개선할 수 있을 것 같습니다. 예를 들어, 다른 도메인 객체들도 이와 같은 방식으로 외부에서 생성된 상태로 받도록 리팩토링을 고려해 볼 수 있습니다.

src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyAssignmentHistoryValidatorTest.java (2)

28-28: 변경 사항이 PR 목표와 일치합니다.

Period 객체를 외부에서 생성된 상태로 받도록 수정한 것은 좋은 변경입니다. 이는 도메인의 책임을 줄이고 관심사의 분리를 개선합니다.

가독성을 더욱 높이기 위해 STUDY_ONGOING_PERIOD의 정의를 이 테스트 클래스나 관련 상수 클래스에 추가하는 것이 어떨까요? 이렇게 하면 테스트의 맥락을 더 쉽게 이해할 수 있을 것 같습니다.


Line range hint 1-102: 추가 테스트 케이스 제안

현재 테스트 케이스들이 포괄적이고 변경 사항에 영향을 받지 않는 것 같습니다. 하지만 새로운 STUDY_ONGOING_PERIOD 상수의 동작을 명시적으로 검증하는 테스트 케이스를 추가하면 좋을 것 같습니다.

다음과 같은 테스트 케이스를 추가하는 것을 고려해보세요:

@Test
void STUDY_ONGOING_PERIOD_상수가_올바르게_적용되는지_확인한다() {
    // given
    Study study = createStudyWithMentor(1L);

    // when & then
    assertThat(study.getPeriod()).isEqualTo(STUDY_ONGOING_PERIOD);
}

이 테스트는 createStudyWithMentor 메서드가 STUDY_ONGOING_PERIOD를 올바르게 사용하고 있는지 확인합니다.

src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGraderTest.java (1)

33-33: STUDY_ONGOING_PERIOD 상수 사용으로 개선된 테스트 설정

이 변경은 PR의 목표와 잘 일치하며, Period 객체를 외부에서 생성된 상태로 받는 방식으로 리팩토링되었습니다. 이는 코드의 유지보수성을 향상시키고 관심사의 분리를 개선합니다.

일관성을 위해 applicationPeriod도 유사한 방식으로 상수화하는 것을 고려해 보시는 것은 어떨까요? 예를 들어:

private static final Period STUDY_APPLICATION_PERIOD = Period.createPeriod(STUDY_START_DATETIME.minusDays(7), STUDY_START_DATETIME.minusDays(1));

이렇게 하면 테스트 설정이 더욱 간결해지고 일관성이 향상될 것 같습니다.

src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipValidatorTest.java (1)

46-47: Period 객체 생성 방식 변경이 잘 이루어졌습니다.

RecruitmentRound.create() 메서드에서 Period 객체를 외부에서 생성하여 전달하도록 변경한 것은 PR의 목적에 잘 부합합니다. 이는 책임 분리와 캡슐화를 개선하는 좋은 변경사항입니다.

더 나아가, 가독성을 높이기 위해 Period 생성을 별도의 변수로 분리하는 것을 고려해 보시는 것은 어떨까요? 예를 들면:

Period period = Period.createPeriod(startDate, endDate);
return RecruitmentRound.create(RECRUITMENT_ROUND_NAME, period, recruitment, ROUND_TYPE);

이렇게 하면 각 매개변수의 역할이 더 명확해질 수 있습니다.

src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1)

78-81: Period 객체를 사용한 리팩토링이 잘 이루어졌습니다.

이 변경은 PR의 목표와 일치하며, Period를 외부에서 생성된 상태로 받도록 하는 리팩토링이 잘 이루어졌습니다. 코드의 가독성과 유지보수성이 향상되었습니다.

테스트의 명확성을 더욱 높이기 위해, Period 생성을 별도의 변수로 추출하는 것을 고려해 보시는 것은 어떨까요? 예를 들면:

Period period = Period.createPeriod(now.plusDays(1), now.plusDays(2));
RecruitmentRound recruitmentRound = RecruitmentRound.create(
    RECRUITMENT_ROUND_NAME,
    period,
    recruitment,
    ROUND_TYPE);

이렇게 하면 테스트에서 Period 객체의 생성과 사용이 더욱 명확해질 것 같습니다.

src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (2)

84-87: LGTM! Period 객체 사용으로 개선되었습니다.

Period 객체를 사용하여 날짜 관련 데이터를 캡슐화한 것은 좋은 개선입니다. 이는 PR의 목표와 일치하며 코드의 가독성과 유지보수성을 향상시킵니다.

더 나아가, Period.createPeriod(request.startDate(), request.endDate())를 메서드 호출 이전에 별도의 변수로 추출하는 것을 고려해 보세요. 이렇게 하면 코드의 가독성이 더욱 향상될 수 있습니다.

예를 들어:

Period period = Period.createPeriod(request.startDate(), request.endDate());
RecruitmentRound recruitmentRound = RecruitmentRound.create(
    request.name(),
    period,
    recruitment,
    request.roundType());

Line range hint 110-111: LGTM! Period 객체 사용이 일관되게 적용되었습니다.

updateRecruitmentRound 메서드에서도 Period 객체를 사용하여 날짜 관련 데이터를 캡슐화한 것은 좋은 개선입니다. 이는 createRecruitmentRound 메서드의 변경사항과 일관성이 있으며, PR의 목표를 잘 달성하고 있습니다.

createRecruitmentRound 메서드와의 일관성을 위해, 여기서도 Period 객체 생성을 별도의 변수로 추출하는 것을 고려해 보세요. 예를 들어:

Period period = Period.createPeriod(request.startDate(), request.endDate());
recruitmentRound.updateRecruitmentRound(request.name(), period, request.roundType());

이렇게 하면 코드의 가독성과 일관성이 더욱 향상될 수 있습니다.

src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyTest.java (1)

48-48: 상수 사용으로 일관성 개선, 추가 개선 제안

START_TO_END_PERIOD 상수를 사용하여 테스트의 일관성을 개선한 것은 좋은 변경입니다. 이는 PR의 목적인 외부에서 생성된 Period를 받는 방식과 일치합니다.

더 나아가, 이 상수의 정의를 테스트 클래스 내에 추가하는 것을 고려해보세요. 예를 들어:

private static final Period START_TO_END_PERIOD = Period.createPeriod(START_DATE, END_DATE);

이렇게 하면 상수의 의미와 구성을 명확히 할 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 76d2a19 and b676d6b.

📒 Files selected for processing (16)
  • src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRound.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RoundType.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/member/domain/MemberValidatorTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipValidatorTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRoundValidatorTest.java (9 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGraderTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyAssignmentHistoryValidatorTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (2 hunks)
  • src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (2 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (2 hunks)
🔇 Additional comments (20)
src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RoundType.java (2)

4-4: 어노테이션 변경이 적절합니다.

@AllArgsConstructor에서 @RequiredArgsConstructor로의 변경은 좋은 개선입니다. 이는 value 필드만을 위한 생성자를 생성하여 코드의 명확성을 높입니다.

Also applies to: 7-7


Line range hint 1-22: PR 목표와의 연관성 설명 필요

이 파일의 변경사항이 PR의 주요 목표인 "Period를 외부에서 생성된 상태로 받도록 수정"과 어떻게 연관되는지 명확하지 않습니다. 이 변경이 전체 리팩토링의 일부인지, 아니면 독립적인 코드 개선인지 설명해 주시면 좋겠습니다.

src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (2)

5-5: import 문이 적절히 추가되었습니다.

Period 클래스의 import 문이 올바르게 추가되었습니다. 이는 새로 추가된 상수에 필요한 것으로 보입니다.


Line range hint 1-30: 다른 코드에 미치는 영향을 확인해 주세요.

이 변경사항은 Period를 외부에서 생성하도록 하는 PR의 목적에 잘 부합합니다. 그러나 이전에 내부적으로 Period 객체를 생성하던 코드베이스의 다른 부분들에 영향을 줄 수 있습니다.

다음 스크립트를 실행하여 Period 객체를 생성하는 다른 부분들을 확인해 보시기 바랍니다:

이 결과를 바탕으로 필요한 경우 다른 부분들도 수정해 주시기 바랍니다.

✅ Verification successful

변경 사항 확인 완료

Period.createPeriod 메서드가 코드베이스의 여러 곳에서 일관되게 사용되고 있음을 확인했습니다. 따라서 이번 변경사항은 다른 부분에 영향을 미치지 않으며, 문제가 없음을 확인했습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: 코드베이스에서 Period 객체를 생성하는 부분을 검색합니다.

echo "Period 객체 생성 패턴 검색 결과:"
ast-grep --lang java --pattern 'new Period($_, $_)'

echo "\nPeriod.createPeriod 메서드 호출 패턴 검색 결과:"
ast-grep --lang java --pattern 'Period.createPeriod($_, $_)'

Length of output: 16823

src/test/java/com/gdschongik/gdsc/domain/membership/domain/MembershipTest.java (1)

39-39: 변경사항이 PR 목표와 일치합니다.

RecruitmentRound.create() 메서드 호출에서 START_DATEEND_DATE를 별도로 전달하는 대신 START_TO_END_PERIOD를 사용하도록 변경한 것은 좋은 접근입니다. 이는 Period를 외부에서 생성된 상태로 받는다는 PR의 목표와 일치합니다.

다음 스크립트를 실행하여 START_TO_END_PERIOD 상수가 적절히 정의되어 있는지 확인하세요:

✅ Verification successful

START_TO_END_PERIOD 상수가 적절히 정의되었습니다.

변경사항이 PR 목표를 충족하며, START_TO_END_PERIOD 상수가 올바르게 정의되어 테스트의 무결성이 유지됩니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of START_TO_END_PERIOD constant

# Test: Search for the START_TO_END_PERIOD constant definition
rg --type java -A 5 'START_TO_END_PERIOD'

Length of output: 15575

src/test/java/com/gdschongik/gdsc/domain/member/domain/MemberValidatorTest.java (2)

27-28: PR 목표와 일치하는 변경 사항입니다.

Recruitment 객체 생성 시 START_TO_END_PERIOD 상수를 사용하도록 변경한 것은 좋은 개선입니다. 이는 Period를 미리 구성된 상태로 받는다는 PR의 목표와 일치하며, 코드의 가독성과 유지보수성을 향상시킵니다.


Line range hint 1-58: 전반적으로 잘 구현된 변경사항입니다.

이 PR의 변경사항들은 Period를 외부에서 생성하여 받는다는 목표를 잘 달성하고 있습니다. 테스트 코드의 가독성과 유지보수성이 향상되었으며, 테스트 케이스의 신뢰성도 개선되었습니다. 제안된 작은 개선사항을 고려해 보시고, 그 외에는 현재 구현이 PR의 목표를 잘 달성하고 있다고 판단됩니다.

src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (1)

3-3: 새로운 import 문이 적절히 추가되었습니다.

Period 클래스의 import가 올바르게 추가되었습니다. 이는 새로 추가된 STUDY_ONGOING_PERIOD 상수에 필요한 것으로 보입니다.

src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRound.java (1)

Line range hint 62-70: LGTM! 변경 사항이 PR 목표와 일치합니다.

create 메서드의 변경 사항이 PR의 목표와 잘 일치합니다. Period를 외부에서 받도록 수정함으로써 다음과 같은 이점이 있습니다:

  1. 관심사의 분리: Period 생성 책임이 도메인 외부로 이동되었습니다.
  2. 캡슐화 개선: 날짜 관련 로직이 Period 객체 내부로 캡슐화되었습니다.
  3. 코드 가독성 향상: 메서드 시그니처가 더 간결해졌습니다.

이러한 변경으로 코드의 유지보수성과 확장성이 향상되었습니다.

src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1)

72-73: 변경 사항이 PR 목표와 일치하며 개선되었습니다.

이 변경은 Period를 외부에서 생성하여 전달하는 PR의 목표와 잘 부합합니다. 고정된 상수 대신 동적인 startDateendDate를 사용함으로써 다음과 같은 이점이 있습니다:

  1. 테스트의 유연성 향상: 다양한 날짜 시나리오를 쉽게 테스트할 수 있습니다.
  2. 관심사의 분리 개선: Period 생성 책임이 도메인 외부로 이동했습니다.
  3. createRecruitmentRound 메서드의 재사용성 증가: 다양한 테스트 케이스에 활용할 수 있습니다.

이러한 변경은 코드의 유지보수성과 테스트 용이성을 향상시킵니다.

src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (1)

Line range hint 1-114: 전반적인 리팩토링이 잘 이루어졌습니다.

이 PR의 변경사항은 Period 객체를 외부에서 생성하여 받는 목표를 잘 달성하고 있습니다. 이러한 변경으로 인해:

  1. 날짜 관련 데이터의 캡슐화가 개선되었습니다.
  2. RecruitmentRound 클래스의 책임이 명확해졌습니다.
  3. 코드의 일관성과 가독성이 향상되었습니다.

추가적인 개선사항으로 제안된 Period 객체 생성을 별도의 변수로 추출하는 방법을 고려해보시기 바랍니다. 이는 코드의 가독성을 더욱 향상시킬 수 있습니다.

전체적으로 이 리팩토링은 코드의 품질을 향상시키고 유지보수성을 개선하는 좋은 변경사항입니다.

src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryTest.java (2)

Line range hint 1-159: 전반적인 변경사항이 PR 목표와 일치합니다.

이 파일의 변경사항은 PR의 목표인 "Period를 외부에서 생성된 상태로 받도록 수정"을 잘 반영하고 있습니다. createStudyWithMentor 메서드의 수정 외에는 다른 테스트 케이스들이 그대로 유지되어 있어, 기존의 테스트 커버리지를 유지하고 있습니다.

추가 권장사항:

  1. 이 변경으로 인해 영향을 받을 수 있는 다른 테스트 클래스나 메서드가 있는지 확인해 주세요.
  2. STUDY_ONGOING_PERIOD의 값이 테스트 목적에 적합한지 검토해 주세요.
  3. 이 변경이 실제 프로덕션 코드에 미치는 영향을 평가하고, 필요하다면 관련 문서를 업데이트해 주세요.

25-25: LGTM! Period 생성 책임 이동이 잘 반영되었습니다.

이 변경은 PR의 목표인 "Period를 외부에서 생성된 상태로 받도록 수정"과 잘 일치합니다. STUDY_ONGOING_PERIOD 상수를 사용함으로써 Period 생성 책임이 이 메서드 외부로 이동되었습니다.

다음 사항들을 확인해 주시기 바랍니다:

  1. STUDY_ONGOING_PERIOD 상수가 적절히 정의되어 있고 접근 가능한지 확인해 주세요.
  2. 다른 테스트 메서드들이 특정 Period 값에 의존하고 있다면, 그에 맞게 업데이트가 필요할 수 있습니다.

이러한 변경사항들이 테스트 커버리지에 영향을 미치지 않는지 확인하기 위해 다음 스크립트를 실행해 주세요:

✅ Verification successful

STUDY_ONGOING_PERIOD의 정의 및 사용이 올바르게 반영되었습니다.

PR의 변경사항을 검증한 결과, STUDY_ONGOING_PERIOD 상수가 적절하게 정의되어 있으며, 기존의 테스트 메서드들에서 제대로 사용되고 있음을 확인했습니다. 추가적인 문제는 발견되지 않았습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of STUDY_ONGOING_PERIOD and potential impacts on other tests

# Test 1: Check if STUDY_ONGOING_PERIOD is properly defined
echo "Checking STUDY_ONGOING_PERIOD definition:"
rg --type java "STUDY_ONGOING_PERIOD\s*=" src/

# Test 2: Check for other occurrences of Period.createPeriod that might need updating
echo "\nChecking for other Period.createPeriod usages:"
rg --type java "Period\.createPeriod" src/test/

# Test 3: Check for test methods that might be affected by this change
echo "\nChecking for potentially affected test methods:"
rg --type java "createStudyWithMentor" src/test/

Length of output: 18503

src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRoundValidatorTest.java (6)

61-62: LGTM: START_TO_END_PERIOD 사용으로 일관성 개선

Period.createPeriod(START_DATE, END_DATE) 대신 START_TO_END_PERIOD를 사용하여 테스트 케이스 전반에 걸쳐 일관성이 개선되었습니다. 이는 PR의 목표인 사전 구성된 상태로 Period를 받는 것과 일치합니다.


74-75: LGTM: RecruitmentRecruitmentRound 생성에서 일관된 변경

RecruitmentRecruitmentRound 생성 모두에서 START_TO_END_PERIOD를 사용하도록 일관되게 변경되었습니다. 이는 이전 세그먼트의 변경사항과 일치하며, PR의 목표를 잘 반영하고 있습니다.

Also applies to: 78-78


94-95: LGTM: 일관된 START_TO_END_PERIOD 사용

이전 변경사항들과 동일하게 START_TO_END_PERIOD를 사용하여 일관성을 유지하고 있습니다. 이는 테스트 코드의 가독성과 유지보수성을 향상시킵니다.


107-108: LGTM: 테스트 클래스 전반에 걸친 일관성 유지

START_TO_END_PERIOD를 사용하는 변경사항이 테스트 클래스 전반에 걸쳐 일관되게 적용되었습니다. 이는 코드의 일관성과 가독성을 높이는 긍정적인 변화입니다.

Also applies to: 111-111


127-128: LGTM: 다양한 테스트 메서드에서의 일관성 유지

START_TO_END_PERIOD를 사용하는 변경사항이 다양한 테스트 메서드에서도 일관되게 적용되었습니다. 이는 테스트 코드의 일관성과 유지보수성을 높이는 좋은 접근 방식입니다.

Also applies to: 131-131


175-176: LGTM: 일관된 START_TO_END_PERIOD 사용으로 테스트 코드 개선

모든 변경 사항에서 START_TO_END_PERIOD를 일관되게 사용하고 있습니다. 이는 PR의 목표와 일치하며, 테스트 코드의 가독성과 유지보수성을 크게 향상시킵니다. 전반적으로 잘 수행된 리팩토링입니다.

Also applies to: 179-179, 192-193, 196-196, 209-210, 213-213

src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (1)

182-183: LGTM! Period 객체 사용으로 코드가 개선되었습니다.

START_TO_END_PERIOD를 사용하여 Period 객체를 직접 전달하는 방식으로 변경된 것이 좋습니다. 이 변경으로 인해:

  1. 코드의 가독성이 향상되었습니다.
  2. Period 생성 책임이 도메인 외부로 이동되어 관심사 분리가 개선되었습니다.
  3. PR의 목표와 일치하는 변경이 이루어졌습니다.

Comment on lines 135 to 138
RECRUITMENT_ROUND_NAME,
Period.createPeriod(ROUND_TWO_START_DATE, ROUND_TWO_START_DATE),
recruitment,
RoundType.SECOND);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Period 생성 방식의 불일치 및 잠재적 오타 확인 필요

이 부분에서 Period.createPeriod를 사용하는 것은 파일의 다른 부분에서 이루어진 변경사항과 일치하지 않습니다. 또한, 종료 날짜 매개변수에 ROUND_TWO_START_DATE를 사용하고 있어 오타일 가능성이 있습니다.

다음 사항을 확인해 주세요:

  1. Period.createPeriod 대신 START_TO_END_PERIOD를 사용할 수 있는지 검토
  2. 종료 날짜가 ROUND_TWO_START_DATE가 아닌 ROUND_TWO_END_DATE여야 하는지 확인

Comment on lines +151 to +152
Recruitment recruitment =
Recruitment.createRecruitment(ACADEMIC_YEAR, SEMESTER_TYPE, FEE, FEE_NAME, START_TO_END_PERIOD);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

일관된 변경 승인 및 불일치 사항 재확인 필요

151-152행과 155행의 변경사항은 이전 세그먼트들과 일관되게 START_TO_END_PERIOD를 사용하고 있어 적절합니다.

하지만 159-162행은 이전 세그먼트에서 지적한 것과 동일한 문제가 있습니다:

  1. Period.createPeriod 사용이 파일의 다른 부분과 일치하지 않습니다.
  2. 종료 날짜 매개변수에 ROUND_TWO_START_DATE를 사용하고 있어 오타일 가능성이 있습니다.

이 부분을 검토하고 필요한 경우 수정해 주시기 바랍니다.

Also applies to: 155-155, 159-162

Comment on lines +197 to +198
RecruitmentRound recruitmentRound =
RecruitmentRound.create(name, Period.createPeriod(startDate, endDate), recruitment, roundType);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Period 객체를 메서드 파라미터로 받도록 개선이 필요합니다.

현재 구현은 Period 객체를 사용하도록 개선되었지만, PR의 목표를 완전히 달성하지 못했습니다. 다음과 같은 개선을 제안합니다:

  1. startDateendDate 대신 Period 객체를 직접 메서드 파라미터로 받도록 변경하세요.
  2. 메서드 내에서 Period.createPeriod()를 호출하는 대신, 외부에서 생성된 Period를 사용하세요.

다음과 같이 메서드 시그니처를 변경하고 구현을 수정하는 것을 고려해보세요:

 protected RecruitmentRound createRecruitmentRound(
         String name,
-        LocalDateTime startDate,
-        LocalDateTime endDate,
+        Period period,
         Integer academicYear,
         SemesterType semesterType,
         RoundType roundType,
         Money fee) {
     Recruitment recruitment = createRecruitment(academicYear, semesterType, fee);
     RecruitmentRound recruitmentRound =
-            RecruitmentRound.create(name, Period.createPeriod(startDate, endDate), recruitment, roundType);
+            RecruitmentRound.create(name, period, recruitment, roundType);
     return recruitmentRoundRepository.save(recruitmentRound);
 }

이렇게 변경하면 PR의 목표인 "Period를 외부에서 생성된 상태로 받도록 수정"을 완전히 달성할 수 있습니다.

📝 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
RecruitmentRound recruitmentRound =
RecruitmentRound.create(name, Period.createPeriod(startDate, endDate), recruitment, roundType);
RecruitmentRound recruitmentRound =
RecruitmentRound.create(name, period, recruitment, roundType);

Copy link
Member

@kckc0608 kckc0608 left a comment

Choose a reason for hiding this comment

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

lgtm

@uwoobeat
Copy link
Member Author

uwoobeat commented Oct 9, 2024

Period의 create메서드 리네이밍은 다른 이슈에서 하나요?

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

🧹 Outside diff range and nitpick comments (1)
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (1)

16-16: LGTM: START_TO_END_PERIOD 상수 추가

START_TO_END_PERIOD 상수가 적절하게 추가되었습니다. 이는 PR의 목적에 부합하며, 시작일과 종료일을 Period 객체로 캡슐화하는 좋은 방법입니다.

상수의 이름을 더 명확하게 하기 위해 FIRST_ROUND_PERIOD로 변경하는 것을 고려해 보시는 것은 어떨까요? 이렇게 하면 두 번째 라운드의 상수와 더 일관성 있게 됩니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b676d6b and 6920268.

📒 Files selected for processing (2)
  • src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRoundValidatorTest.java (9 hunks)
  • src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (3 hunks)
🧰 Additional context used
🔇 Additional comments (11)
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (3)

5-5: LGTM: Period 클래스 import 추가

Period 클래스의 import 문이 올바르게 추가되었습니다. 이는 새로운 상수들에서 Period 객체를 사용하기 위해 필요한 변경사항입니다.


27-28: LGTM: ROUND_TWO_START_TO_END_PERIOD 상수 추가

ROUND_TWO_START_TO_END_PERIOD 상수가 적절하게 추가되었습니다. 이는 첫 번째 라운드에 대한 접근 방식과 일관성이 있으며, 두 번째 라운드의 기간을 명확하게 나타냅니다.

코드의 일관성과 가독성을 위해 상수 선언을 두 줄로 나눈 것이 좋습니다. 이러한 접근 방식을 계속 유지해 주세요.


Line range hint 5-28: 전반적인 변경 사항에 대한 요약

이번 변경으로 Period 객체를 사용하여 모집 라운드의 기간을 표현하는 방식이 도입되었습니다. 이는 PR의 목적에 부합하며, 코드의 가독성과 유지보수성을 향상시킵니다.

  1. Period 클래스의 import 추가
  2. 첫 번째 모집 라운드를 위한 START_TO_END_PERIOD 상수 추가
  3. 두 번째 모집 라운드를 위한 ROUND_TWO_START_TO_END_PERIOD 상수 추가

이러한 변경은 일관성 있게 구현되었으며, 기존 코드 스타일을 잘 따르고 있습니다. Period 객체를 사용함으로써 날짜 관리가 더욱 명확해졌고, 향후 기간과 관련된 로직을 다루는 데 있어 이점이 있을 것으로 보입니다.

src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentRoundValidatorTest.java (8)

61-62: 변경 사항이 일관성 있게 적용되었습니다.

START_TO_END_PERIOD를 사용하여 Recruitment 객체를 생성하는 방식으로 변경된 것이 확인됩니다. 이는 PR의 목적에 부합하며 코드의 가독성을 향상시킵니다.


74-75: 일관된 변경이 적용되었습니다.

이전 세그먼트와 마찬가지로 START_TO_END_PERIOD를 사용하여 Recruitment 객체를 생성하고 있습니다. 이는 PR의 목적에 일관되게 부합합니다.


78-78: 객체 생성 전반에 걸쳐 일관된 변경이 적용되었습니다.

RecruitmentRound.create 메서드에서도 START_TO_END_PERIOD를 사용하고 있어, 이전 변경사항들과 일관성을 유지하고 있습니다. 이는 PR의 목적을 잘 반영하고 있습니다.


94-95: 테스트 케이스 전반에 걸쳐 일관된 변경이 적용되었습니다.

여러 테스트 케이스에서 Recruitment.createRecruitment 메서드가 START_TO_END_PERIOD를 일관되게 사용하고 있습니다. 이는 PR의 목적을 충실히 이행하고 있으며, 코드의 일관성을 높이고 있습니다.


107-108: 다양한 객체 타입에 걸쳐 일관된 변경이 적용되었습니다.

RecruitmentRecruitmentRound 객체 생성 모두에서 START_TO_END_PERIOD를 일관되게 사용하고 있습니다. 이는 PR의 목적을 정확히 반영하며, 다양한 객체 타입에 걸쳐 일관성 있는 코드 스타일을 유지하고 있습니다.

Also applies to: 111-111


127-128: 일관된 변경과 함께 새로운 상수의 도입이 확인됩니다.

첫 번째 라운드에서는 이전과 동일하게 START_TO_END_PERIOD를 사용하고 있습니다. 두 번째 라운드에서는 새로운 상수인 ROUND_TWO_START_TO_END_PERIOD가 도입되었습니다. 이 변경은 PR의 목적에 부합하며, 각 라운드별로 명확한 구분을 제공합니다.

Also applies to: 131-131, 135-135


148-149: 여러 라운드에 걸쳐 일관된 변경이 적용되었습니다.

첫 번째 라운드와 두 번째 라운드에서 각각 START_TO_END_PERIODROUND_TWO_START_TO_END_PERIOD를 일관되게 사용하고 있습니다. 이는 이전 세그먼트의 변경사항과 일치하며, PR의 목적을 충실히 반영하고 있습니다. 각 라운드별로 명확한 구분을 제공하여 코드의 가독성과 유지보수성을 향상시키고 있습니다.

Also applies to: 152-152, 156-159


172-173: 전체 파일에 걸쳐 일관된 변경이 성공적으로 적용되었습니다.

모든 테스트 케이스에서 START_TO_END_PERIOD를 일관되게 사용하고 있습니다. 이는 PR의 목적을 완벽하게 달성하고 있으며, 코드의 일관성과 가독성을 크게 향상시켰습니다. Period 객체의 생성 책임을 외부로 이동시키는 리팩토링 목표가 성공적으로 구현되었습니다.

Also applies to: 176-176, 189-190, 193-193, 206-207, 210-210

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

@uwoobeat uwoobeat merged commit 382ac9a into develop Oct 12, 2024
1 check passed
@uwoobeat uwoobeat deleted the refactor/792-period branch October 12, 2024 07:36
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.

♻️ Period를 외부에서 생성된 상태로 받도록 수정
3 participants