Skip to content

Conversation

@Eseas
Copy link
Collaborator

@Eseas Eseas commented Feb 20, 2025

바로 MERGE하지 마세요! 검토 후 MERGE해야합니다!

✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)

  • 기능 추가
  • 리뷰 반영 수정
  • 리팩토링
  • 버그 수정
  • 컨벤션 수정
  • 브랜치 최신화

🏆 구현 목표

  • 체크리스트 요청 반려 시 더이상 요청을 보낼 수 없던 문제 해결

📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)

  • 체크리스트 요청 반려 시 더이상 요청을 보낼 수 없던 문제 해결

🔍 테스트 방법 (변경 사항을 확인하기 위한 테스트 방법을 기술합니다.)

  • 스웨거 테스트

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

if(checklistStatus == ChecklistStatus.APPLICATION_WAITING || checklistStatus == ChecklistStatus.REJECTED) {
checklistStatus = ChecklistStatus.APPROVE_WAITING;
return;
}

Choose a reason for hiding this comment

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

코드 패치에 대한 간략한 리뷰를 제공하겠습니다.

  1. 버그 위험:

    • setChecklistApplication 메서드는 checklistStatusAPPLICATION_WAITING 또는 REJECTED일 경우에만 상태를 APPROVE_WAITING으로 변경합니다. 이러면 오히려 REJECTED 상태에서도 APPROVE_WAITING으로 상태가 변경될 수 있다는 점이 문제입니다. 만약 REJECTED 상태에서 승인 대기 상태로 넘어가는 것이 비즈니스 로직에 맞지 않다면, 이는 치명적인 오류를 유발할 수 있습니다.
  2. 개선 제안:

    • REJECTED 상태에서의 처리 로직을 명확히 해야 합니다. 예를 들어, REJECTED 상태일 때는 로그를 남기거나 예외를 던지는 등의 조치를 고려할 수 있습니다.
    • 메서드의 명칭이나 주석을 추가하여 이 메서드의 의도를 더욱 명확히 할 수 있습니다. 현재 메서드 이름만으로는 어떤 조건에서 상태를 변경하는지 명확하지 않습니다.
    • 나중에 유지보수나 기능 확장성을 고려해 상태 전이가 가능한 세부 규칙을 분리하여 관리하는 방법도 고려해볼 수 있습니다.
  3. 스타일:

    • 전반적으로 코드 스타일은 깔끔하지만, 조건문의 가독성을 높이기 위해 괄호를 추가하는 것도 좋은 습관입니다.

결론적으로, 코드 변경이 비즈니스 로직에 어떤 영향을 미치는지 충분히 검토한 후, 추가 테스트를 통해 예상되는 문제를 방지하는 것이 중요합니다.

@Eseas Eseas closed this Feb 20, 2025
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.

3 participants