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

[BE] 비회원 댓글 비밀번호만 체크하는 API 만들기 / 관리자 댓글 삭제 기능 (#217) #230

Merged
merged 7 commits into from
Jul 21, 2021
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.idea/
/db
db/
5 changes: 4 additions & 1 deletion backend/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ out/

## RestDocs 파일
/src/main/resources/static/docs/

## BOOT-INF
/BOOT-INF
BOOT-INF/

## 로그 파일
data/

## 기타
/2021-darass/
db/
db/
12 changes: 12 additions & 0 deletions backend/src/docs/asciidoc/api/v1/comments.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,23 @@ include::{snippets}/api/v1/comments/delete/success-login-user/http-response.adoc

include::{snippets}/api/v1/comments/delete/success-guest-user/http-request.adoc[]
include::{snippets}/api/v1/comments/delete/success-guest-user/path-parameters.adoc[]
include::{snippets}/api/v1/comments/delete/success-guest-user/request-parameters.adoc[]

==== Response

include::{snippets}/api/v1/comments/delete/success-guest-user/http-response.adoc[]

==== (성공) 관리자가 남의 댓글 삭제

==== Request

include::{snippets}/api/v1/comments/delete/success-admin-user/http-request.adoc[]
include::{snippets}/api/v1/comments/delete/success-admin-user/request-headers.adoc[]

==== Response

include::{snippets}/api/v1/comments/delete/success-admin-user/http-response.adoc[]

==== (실패) 남의 댓글을 삭제하는 경우

==== Response
Expand Down
18 changes: 18 additions & 0 deletions backend/src/docs/asciidoc/api/v1/users.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,21 @@ include::{snippets}/api/v1/users/delete/success/http-response.adoc[]

include::{snippets}/api/v1/users/delete/fail/http-response.adoc[]
include::{snippets}/api/v1/users/delete/fail/response-fields.adoc[]

=== 비로그인 유저 비밀번호 일치여부 조회 (GET /api/v1/users/check-password)

==== Request

include::{snippets}/api/v1/users/get/password-check-correct/http-request.adoc[]
include::{snippets}/api/v1/users/get/password-check-correct/request-parameters.adoc[]

==== Response (성공 - 비밀번호 일치하는 경우)

include::{snippets}/api/v1/users/get/password-check-correct/http-response.adoc[]
include::{snippets}/api/v1/users/get/password-check-correct/response-fields.adoc[]


==== Response (성공 - 비밀번호 일치하지 않는 경우)
Copy link
Collaborator

Choose a reason for hiding this comment

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

실패가 맞지 않나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

비밀번호가 일치하지 않더라도 조회 자체는 실패한 것이 아니고 성공이라고 생각했었습니다😊

Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 꼼꼼하게 보지 못했네요 ㅠㅠ 데일리때 설명해준대로 이해했어요 😊


include::{snippets}/api/v1/users/get/password-check-incorrect/http-response.adoc[]
include::{snippets}/api/v1/users/get/password-check-incorrect/response-fields.adoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,13 @@
import com.darass.darass.comment.controller.dto.CommentUpdateRequest;
import com.darass.darass.comment.service.CommentService;
import com.darass.darass.user.domain.User;
import java.util.List;
import javax.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.ModelAttribute;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;

import javax.validation.Valid;
import java.util.List;

@RestController
@RequestMapping("/api/v1/comments")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,41 @@ public List<CommentResponse> findAllCommentsByUrlAndProjectKey(String url, Strin

public void updateContent(Long id, User user, CommentUpdateRequest request) {
user = findRegisteredUser(user, request.getGuestUserId(), request.getGuestUserPassword());
Comment comment = returnValidatedComment(id, user);
Comment comment = findCommentById(id);

validateCommentUpdatableByUser(user, comment);

comment.changeContent(request.getContent());
commentRepository.save(comment);
}

public void delete(Long id, User user, CommentDeleteRequest request) {
user = findRegisteredUser(user, request.getGuestUserId(), request.getGuestUserPassword());
returnValidatedComment(id, user);
Comment comment = findCommentById(id);
User adminUser = comment.getProject().getUser();

validateCommentDeletableByUser(user, adminUser, comment);

commentRepository.deleteById(id);
}

private Comment returnValidatedComment(Long id, User user) {
Comment comment = commentRepository.findById(id)
private void validateCommentUpdatableByUser(User user, Comment comment) {
if (comment.isCommentWriter(user)) {
return;
}
throw ExceptionWithMessageAndCode.UNAUTHORIZED_FOR_COMMENT.getException();
}

private void validateCommentDeletableByUser(User user, User adminUser, Comment comment) {
if (user.isSameUser(adminUser) || comment.isCommentWriter(user)) {
return;
}
throw ExceptionWithMessageAndCode.UNAUTHORIZED_FOR_COMMENT.getException();
}

private Comment findCommentById(Long id) {
return commentRepository.findById(id)
.orElseThrow(ExceptionWithMessageAndCode.NOT_FOUND_COMMENT::getException);
matchUserWithComment(user, comment);
return comment;
}

private User findRegisteredUser(User user, Long guestUserId, String guestUserPassword) {
Expand All @@ -109,12 +128,6 @@ private User findRegisteredUser(User user, Long guestUserId, String guestUserPas
return user;
}

private void matchUserWithComment(User user, Comment comment) {
if (!comment.isCommentWriter(user)) {
throw ExceptionWithMessageAndCode.UNAUTHORIZED_FOR_COMMENT.getException();
}
}

private void validateGuestUser(User user, String password) {
if (!user.isValidGuestPassword(password)) {
throw ExceptionWithMessageAndCode.INVALID_GUEST_PASSWORD.getException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,16 @@

import com.darass.darass.auth.oauth.domain.RequiredLogin;
import com.darass.darass.comment.controller.dto.UserResponse;
import com.darass.darass.user.controller.dto.PasswordCheckRequest;
import com.darass.darass.user.controller.dto.PasswordCheckResponse;
import com.darass.darass.user.controller.dto.UserUpdateRequest;
import com.darass.darass.user.domain.User;
import com.darass.darass.user.service.UserService;
import javax.validation.Valid;
import lombok.AllArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;

@RestController
@RequestMapping("/api/v1/users")
Expand Down Expand Up @@ -42,4 +39,9 @@ public ResponseEntity<Void> delete(@RequiredLogin User user) {
return ResponseEntity.noContent().build();
}

@GetMapping("/check-password")
Copy link
Collaborator

Choose a reason for hiding this comment

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

api 이름 괜찮다고 생각합니다! 👍 👍

public ResponseEntity<PasswordCheckResponse> checkGuestUserPassword(@ModelAttribute PasswordCheckRequest passwordCheckRequest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get요청일 때 @RequestParam이랑 @PathParam만 사용해봐서 그런데, @ModelAttribute의 역할이 어떻게 되나요~?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

modelattribute를 사용하면 여러 쿼리파람들이 들어올때 그것을 하나의 dto 객체로 바인딩하여 받을 수 있습니다~
또 이 어노테이션은 생략 가능한데 가독성과 이해를 위하여 일부러 생략하지 않았습니다 ^^

PasswordCheckResponse passwordCheckResponse = userService.checkGuestUserPassword(passwordCheckRequest);
return ResponseEntity.ok(passwordCheckResponse);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.darass.darass.user.controller.dto;

import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;

import javax.validation.constraints.NotNull;

@Getter
@AllArgsConstructor
public class PasswordCheckRequest {

@NotNull
private Long guestUserId;

@NotNull
private String guestUserPassword;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.darass.darass.user.controller.dto;

import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
@AllArgsConstructor
public class PasswordCheckResponse {

private Boolean isCorrectPassword;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.darass.darass.comment.controller.dto.UserResponse;
import com.darass.darass.exception.ExceptionWithMessageAndCode;
import com.darass.darass.user.controller.dto.PasswordCheckRequest;
import com.darass.darass.user.controller.dto.PasswordCheckResponse;
import com.darass.darass.user.controller.dto.UserUpdateRequest;
import com.darass.darass.user.domain.User;
import com.darass.darass.user.repository.UserRepository;
Expand All @@ -18,8 +20,8 @@ public class UserService {
private final UserRepository users;

public UserResponse findById(Long id) {
Optional<User> expectedUser = users.findById(id);
User user = expectedUser.orElseThrow(ExceptionWithMessageAndCode.NOT_FOUND_USER::getException);
Optional<User> possibleUser = users.findById(id);
User user = possibleUser.orElseThrow(ExceptionWithMessageAndCode.NOT_FOUND_USER::getException);

return UserResponse.of(user, user.getUserType(), user.getProfileImageUrl());
}
Expand All @@ -36,4 +38,13 @@ public void deleteById(Long id) {
users.deleteById(id);
}

public PasswordCheckResponse checkGuestUserPassword(PasswordCheckRequest passwordCheckRequest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 부분에 대해 통합 테스트 코드 작성해주셔야 할 것 같아요!

Optional<User> expectedUser = users.findById(passwordCheckRequest.getGuestUserId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Optional<User> expectedUser = users.findById(passwordCheckRequest.getGuestUserId());
Optional<User> possibleUser = users.findById(passwordCheckRequest.getGuestUserId());

우기랑 같은 피드백을 남겼는데, Optional 변수명은 "possible + Type" 방식으로 쓰는 것이 어떨지 이야기하는 포스팅이 있었어요. 저는 더 직관적이라고 생각이 드는데 아론은 어떤가요?
https://betterprogramming.pub/useful-tips-for-naming-your-variables-8139cc8d44b5

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 지적 감사합니다~ 반영했습니다!

User user = expectedUser.orElseThrow(ExceptionWithMessageAndCode.NOT_FOUND_USER::getException);

if (user.isValidGuestPassword(passwordCheckRequest.getGuestUserPassword())) {
return new PasswordCheckResponse(true);
}
return new PasswordCheckResponse(false);
}
}
Loading