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: 종료미션 DurationStatus 추가 및 스케줄러 구현 #181

Merged
merged 8 commits into from
Jan 19, 2024

Conversation

char-yb
Copy link
Member

@char-yb char-yb commented Jan 18, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • 종료미션에 대한 durationStatus 컬럼 추가
  • 종료미션이 현 시간보다 낮은 일자이면 자정마다 durationStatus를 FINISHED로 갱신

📝 참고사항

📚 기타

@char-yb char-yb added 🥇 P1 급하고 꼭 필요한 이슈 ✨ feature 새로운 기능 추가 및 수정 labels Jan 18, 2024
@char-yb char-yb added this to the 3차 스프린트 milestone Jan 18, 2024
@char-yb char-yb requested review from kdomo and uwoobeat January 18, 2024 15:14
@char-yb char-yb self-assigned this Jan 18, 2024
@char-yb char-yb linked an issue Jan 18, 2024 that may be closed by this pull request
Copy link
Member

@kdomo kdomo left a comment

Choose a reason for hiding this comment

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

패키지 domain쪽보단 따로 scheduler로 빼서 거기에 넣는게 좋아보입니당
그리구 메소드명 complete보다 DurationStatus에 맞게 finish로 사용하면 어떨까용?

Comment on lines 7 to 15
@Component
@RequiredArgsConstructor
public class MissionBatchJob {
private final MissionService missionService;

public void updateCompleteDurationStatus() {
missionService.updateCompleteDurationStatus();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

MissionBatchJob 객체 없이 MissionBatchScheduler 에서 바로 service 호출해서 사용하는게 좋아보여용

Copy link
Member Author

Choose a reason for hiding this comment

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

배치 영역과 스케줄러 영역 분리가 필요할까봐 했었는데 지금은 없이하는게 나을 듯 하네용👍

Comment on lines 12 to 14
@Modifying
@Query(value = "update Mission m set m.durationStatus='FINISHED' where m.finishedAt <= NOW()")
void updateMissionDurationStatusComplete();
Copy link
Member

Choose a reason for hiding this comment

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

now() 했을 때 시간이 정확히 00시 00분이 아닐 경우도 생길 것 같아서
어플리케이션 단에서 시간을 맞춰서 쿼리하는게 좋을 것 같은데 어떻게 생각하세용?

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 저도 좋다고 생각해용


@EnableScheduling
Copy link
Member

Choose a reason for hiding this comment

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

Config로 따로 빼도 좋을 것 같아용

Comment on lines 6 to 8
@EnableScheduling
@Configuration
public class BatchConfig {}
Copy link
Member

Choose a reason for hiding this comment

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

스케줄링을 통해 정해진 시간에 행위를 할 뿐 배치처리라고 보긴 어려워서
Batch보다는 Scheduler가 맞는 표현인 것 같아용

Comment on lines 14 to 18
@Modifying
@Query(
value =
"update Mission m set m.durationStatus='FINISHED' where m.finishedAt <= :today and m.durationStatus != 'FINISHED'")
void updateFinishedDurationStatus(@Param("today") LocalDateTime today);
Copy link
Member

Choose a reason for hiding this comment

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

durationStatus enum의 FINISHED의 값이 변경되었을 때, 버그가 발생할 수 있는 포인트라고 생각되는데,
QueryDsl을 사용하면 컴파일단계에서 에러를 잡을 수 있는 이점이 있다고 생각하는데 어떻게 생각하시나용??

Copy link
Member Author

Choose a reason for hiding this comment

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

좋아용 반영해서 push 했습니다!

Copy link
Member

@kdomo kdomo left a comment

Choose a reason for hiding this comment

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

LGTM~
고생하셨습니당

Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

37 New issues
0 Security Hotspots
53.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@char-yb char-yb merged commit 91616bb into develop Jan 19, 2024
2 checks passed
@github-actions github-actions bot added the merged 머지된 PR label Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature 새로운 기능 추가 및 수정 merged 머지된 PR 🥇 P1 급하고 꼭 필요한 이슈
Projects
Status: 완료
Development

Successfully merging this pull request may close these issues.

✨ 종료미션 로직과 스케줄러 구현
2 participants