Skip to content

add remove post method#32

Open
JinHoooooou wants to merge 11 commits intomainfrom
jinho-step2-remove-post
Open

add remove post method#32
JinHoooooou wants to merge 11 commits intomainfrom
jinho-step2-remove-post

Conversation

@JinHoooooou
Copy link
Contributor

  1. 테스트 생성

    1. 유효한 post_id값과 post의 author와 request_body의 author이 같을 때
      * 삭제 하고, 응답으로 삭제한 post의 제목과 내용을 리턴한다.
    2. post_id가 유효하지 않을 때
      * 삭제를 하지 않고, post를 찾을 수 없습니다.라는 메세지를 리턴한다.
    3. post_id는 유효하지만, post의 author와 request_body의 author이 다를 때
      * 삭제를 하지 않고, 권한이 없습니다.라는 메세지를 리턴한다.
  2. 구현 코드 작성

  3. 테스트

    • image
      통과

@JinHoooooou JinHoooooou changed the title Jinho step2 remove post Jinho step2 remove post method Jul 6, 2021
@JinHoooooou JinHoooooou changed the title Jinho step2 remove post method add remove post method Jul 6, 2021
Comment on lines 99 to 100
Copy link
Contributor

Choose a reason for hiding this comment

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

이때는 인증이 아니라 권한과 관련된 부분이라 401 보다는 403이 맞을 것 같애 403 FORBIDDEN

Comment on lines 103 to 110
Copy link
Contributor

Choose a reason for hiding this comment

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

200으로 줘도 무방하지만, 대개는 204 NO_CONTENT로 주고, data는 따로 보내지 않음

Suggested change
return JsonResponse(
{
"post":
{
"title": to_delete_post.title,
"text": to_delete_post.text,
}
}, status=OK)
return JsonResponse(status=204)

Comment on lines 195 to 198
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 요구사항이 title, text를 리턴해달라고 했다면, 추가되는게 맞지만 대개의 경우에는 204에 data는 따로 응답으로 보내진 않음 - 참고만

Copy link
Contributor

Choose a reason for hiding this comment

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

위에 적은 것 처럼, 테스트 코드에서 given 설정해줄 때, author id도 invalid 하다는 것을 확실히 명시해주면 좋을 것 같애

invalid_author_id = 12314

Comment on lines 226 to 229
Copy link
Contributor

Choose a reason for hiding this comment

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

위에서 말한대로 403으로 수정~

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.

2 participants

Comments