프로젝트 상태값 변경시 프로젝트 매니저만 가능하도록 수정 #325
Conversation
| return ApiResponse.success(projectMemberDeleteWebResponse); | ||
| } | ||
| @PutMapping("/manager") | ||
| public ApiResponse<ProjectManagerUpdateWebResponse> projectManagerUpdateWebResponseApiResponse( |
There was a problem hiding this comment.
[P4] 네이밍을 updadateProjectManager 정도면 괜찮지 않을까 싶습니다.
| final UUID memberId = UUID.fromString("019739ea-e7eb-76b7-b5e1-b9dc3ea1e9c2"); | ||
|
|
||
| // when | ||
| final ResultActions result = mockMvc.perform(delete("/api/project-member") |
There was a problem hiding this comment.
구현하신 내용에는 http method 를 PUT 으로 사용하고 계신데 테스트 코드에는 DELETE 네요.
해당 테스트코드 정상 동작한다면 API 문서화를 위해 수정이 필요해보입니다.
| public void changeManager(Boolean isManager){ | ||
| this.manager = !isManager; | ||
| } |
There was a problem hiding this comment.
[P3] isManager 에 null 이 유입되어 DB 에 저장하는 경우 예외가 발생할 수 있어 null 을 막는 방어코드가 있으면 좋을 것 같습니다.
| this.projectAmount = request.projectAmount(); | ||
| } | ||
| public void updateStatus(String status){ | ||
| this.step = status; |
There was a problem hiding this comment.
[P4] 이 부분은 이전에 네이밍이 명확하지 않아서 일치하지 않네요. 이후에 제가 status 로 변경할게요.
|
|
||
| project.updateStatus(status); | ||
|
|
||
| return project.getId(); |
There was a problem hiding this comment.
[P2] status 상태가 변경을 위한 API 여서 클라이언트 측에서 명확한 확인을 위해 Response 로 status 도 함께 포함되어 있으면 좋을 것 같습니다.
| @RequestParam UUID projectId, | ||
| @RequestParam(name = "status", required = false) |
There was a problem hiding this comment.
[P2] http method 로 POST 를 사용하고 있어서 RequestBody 로 입력받는 방법이 적절해 보입니다.
| .param("projectId", projectId.toString()) | ||
| .param("status","COMPLETED") |
There was a problem hiding this comment.
HTTP Method 가 POST 라면 request body 로 입력 값을 전달받는 방법이 적절해보입니다.
|
|
||
| return ApiResponse.success(new MyProjectListWebResponse(webList)); | ||
| } | ||
| @PostMapping("/project-status") |
There was a problem hiding this comment.
매니저 상태를 변경하는 로직에서 HTTP Method 를 PUT 으로 선언하셔서 일관성을 위해 POST 또는 PUT 을 일치시키는 방법이 적절해보입니다.
| } | ||
|
|
||
| if (!request.getMethod().equalsIgnoreCase("GET")) { | ||
| Optional<ProjectMember> projectManager = projectMemberService.findProjectManagerByMemberIdAndProjectId(memberId, projectId); |
There was a problem hiding this comment.
[P2] 해당 응답 값이 null 이 반환되면 응답 값으로 NPE 가 발생할 수 있어서 예외 핸들링이 필요해 보입니다.
| if (projectManager.isEmpty()) { | ||
| writeErrorResponse(response); | ||
| return false; |
There was a problem hiding this comment.
[P1] 현재 OSIV = false 로 선언되어 있어서 @transactional 범위 밖에서 entity 를 호출하면 사이드 이펙트가 발생할 수 있습니다. 이 부분은 DTO 로 조회하는 방식으로 변경해주세요.
pbg0205
left a comment
pbg0205
left a comment
There was a problem hiding this comment.
@PutMapping 에 URL 경로를 케밥 케이스로만 변경 바랍니다.
작업 반영해주시느라 수고 많으셨습니다 👍👍
update-project-manager Kernel360#323
2 participants
📌 개요
🛠️ 변경 사항
✅ 주요 체크 포인트
🔁 테스트 결과
-포스트맨 테스트
🔗 연관된 이슈
#323