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

[Steady] 스테디 탈퇴 기능 추가 및 리팩토링 #200

Merged
merged 8 commits into from
Feb 29, 2024
Merged

Conversation

weonest
Copy link
Contributor

@weonest weonest commented Feb 28, 2024

✅ PR 체크리스트

  • 테스트
  • 문서화 작업

💡 어떤 작업을 하셨나요?

Issue Number : #199

작업 내용

  • 스테디 참여자가 스테디를 탈퇴할 수 있는 기능을 추가하였습니다
  • 기존의 Pagination 과정에서 이전 커서를 잘못 계산하는 부분을 수정하였습니다.
  • 참여자 삭제 정책을 Soft Delete 에서 Hard Delete로 변경하였고, 이에 따라 스테디 참여자 인원 계산 방식을 변경하였습니다.

📝리뷰어에게

  • 기존의 참여자 삭제 정책을 Soft Delete로 책정하였었는데, 참여자에 대한 정보를 따로 보관할 이유가 없다고 판단하여 Hard Delete로 변경하였습니다.
  • 또한 논리적 삭제인 경우 DB에서 데이터를 지우는 것이 아니라는 근거로 PATCH 메서드를 사용하게끔 API를 작성하였었는데, 해당 부분에 대해 다시 공부해본 결과 논리적 삭제 물리적 삭제와 관계 없이 서비스 입장에서 바라봤을 때는 삭제라는 명확한 행위를 호출하는 것이기 때문에 DELETE 메서드로 변경하였습니다.

@weonest weonest self-assigned this Feb 28, 2024
Copy link

Test Results

 27 files   27 suites   8s ⏱️
129 tests 129 ✅ 0 💤 0 ❌
130 runs  130 ✅ 0 💤 0 ❌

Results for commit 3b7b42f.

Copy link
Contributor

@K-jun98 K-jun98 left a comment

Choose a reason for hiding this comment

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

굿 고생하셨습니다.

@@ -114,7 +114,14 @@ public ResponseEntity<Void> updateSteadyQuestions(@PathVariable Long steadyId,
return ResponseEntity.noContent().build();
}

@PatchMapping("/{steadyId}/{memberId}")
@DeleteMapping("{steadyId}/withdraw")
Copy link
Contributor

Choose a reason for hiding this comment

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

REST API 형식으로 한다면 Http DeleteMethod가 행위를 나타내고 있어서 withdraw는 제거해도 되지않을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

REST API에 대해서 좀 더 조사를 해보니 자원에 대한 컨트롤 자원에 대해서는 예외적으로 동사를 사용해도 괜찮다는 내용을 확인했습니다. [ 참고 링크 ]
그런데 여기서 말하는 컨트롤 자원이 정확히 어떤 것을 의미하는지는 잘 모르겠습니다. 위 참고 링크에서 설명하는 예시에서는 http://api.test.com/posts/duplicate 와 같은 사용은 허용해도 좋다라고 말하고 있는데, 단순한 CRUD를 넘어서는 동작을 표현할 때를 이야기하는 것 같기도 합니다.

만약, 행위를 나타내지 않기 위해 withdraw라는 표현을 없앤다면

  1. @DeleteMapping("/{steadyId}/members/{memberId}") -> 리더가 참여자를 추방하는 API
  2. @DeleteMapping("/{steadyId}") -> 스테디 자체를 삭제하는 API
    와 겹치지 않게 DeleteMethod를 사용해야 하는데 어떤 구조를 가지면 좋을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 DELETE /steady/{steadyId}/members/{memberId} 이런 형식은 어떤가요?

Copy link
Contributor Author

@weonest weonest Feb 29, 2024

Choose a reason for hiding this comment

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

이미 위에서 말씀드린 1번 api가 @RequestMapping("/api/v1/steadies) + @DeleteMapping("/{steadyId}/members/{memberId}") 구조로
DELETE /api/v1/steadies/{steadyId}/members/{memberId}를 사용하고 있습니다

@weonest weonest changed the title Feat/#199 [Steady] 스테디 탈퇴 기능 추가 및 리팩토링 Feb 29, 2024
@K-jun98 K-jun98 merged commit 8de3768 into dev Feb 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants