feat - 일별 챌린지 실패 처리 기능 구현 #45
Conversation
| App findByDailyChallengeIdAndAppCodeAndOs(Long dayChallengeId, String appCode, String os); | ||
| App findByDailyChallengeIdAndAppCodeAndOs(Long dailyChallengeId, String appCode, String os); |
| @RequestMapping("/api/v1/dailyChallenge") | ||
| @RequestMapping("/api/v1/dailychallenge") |
There was a problem hiding this comment.
일반적인 컨벤션과 맞지 않는게 다른 데도 있어서 한 이슈에서 다 수정하려고 했는데 수정하셨군요 url 수정됐다구 클라에게 따로 말해줘야 할 것 같아여
There was a problem hiding this comment.
아하 제가 그건 차마 생각을 못했네요.,..!! 노티 정말 감사합니다!!
|
|
||
| public void modifyDailyChallengeStatusFailure() { | ||
| this.status = Status.FAILURE; | ||
| } |
There was a problem hiding this comment.
Q. 나중에 다른 enum값으로 업데이트 하는 성공 로직이라던지 추가될 거 같은데 그 때마다 함수를 각각 만드는게 좋을까요 아니면 파라미터로 enum 받아서 인자값으로 업데이트 해주는 공통함수 하나가 좋을까요 후자가 함수가 하나만 만들어져서 깔끔해보이긴하는데, 전자를 선택한 이유는 string인자도 아니고 enum이라 4개밖에 없어서일까요?? 뭔가 이미 이거에 대해 고민해보셨을 것 같아서 질문드립니다!
There was a problem hiding this comment.
엇 아닙니다 말씀 주신 후자의 방식이 훨씬 더 확장성 있는 좋은 방식인 것 같아요~~ 좋은 방식 제시해주셔서 감사합니다~!
|
수정하였습니다~! |
| dailyChallengeService.getDailyChallenge(IdConverter.getUserId(principal)))); | ||
| } | ||
|
|
||
| @PatchMapping("/failure") | ||
| public ResponseEntity<ApiResponse<?>> orderChallengeDailyChallenge( | ||
| Principal principal, | ||
| @RequestHeader("OS") final String os | ||
| Principal principal | ||
| ) { | ||
| dailyChallengeService.modifyDailyChallengeStatus(IdConverter.getUserId(principal), os); | ||
| dailyChallengeService.modifyDailyChallengeStatus(IdConverter.getUserId(principal)); |
There was a problem hiding this comment.
P5. 파라미터에 os 정보 빼신거 보고 기존과 달리 일별챌린지 조회할 때 사용자가 하루에 두 운영체제로 들어올 가능성은 적겠지만 헤더의 '운영체제에 맞는 앱들'만 반환해주는게 맞는 것 같다고 생각이 들었습니다! 제가 올린 PR 브랜치에서 해당사항 수정하고 푸시하겠습니다! 확인해주세요
There was a problem hiding this comment.
아하! ide에서 os가 사용되지 않는 변수로 떠서 삭제했던 것 같습니다! 사용하시는 변수라면 수정해주셔서 감사합니다~
Related issue 🚀
Work Description 💚
PR 참고 사항