Q&A 기능 추가 #526
Q&A 기능 추가 #526
Conversation
|
|
||
| import java.util.List; | ||
|
|
||
| public record ReadQuestionAndAnswersResponse(List<ReadQuestionAndAnswerResponse> questionAndAnswers) { |
There was a problem hiding this comment.
선택
해당 Response의 이름만 봐서는 하나의 Question에 여러 Answer가 달린 줄 알았습니다
QuestionAndAnswer의 복수형이라 QuestionAndAnswers가 된 것 같은데 QuestionsAndAnswers는 어떨까요
QNAs도 생각해봤는데 결국엔 똑같을 것 같아서..ㄱ렇습니다
There was a problem hiding this comment.
QuestionAndAnswer를 모두 qna로 바꾸기로 했기에 Qnas로 수정하도록 하겠습니다.
괜찮을까요?
There was a problem hiding this comment.
저는 괜찬을 것 같습니다 너무 늦게 확인했네요 죄송합니다
|
|
||
|
|
There was a problem hiding this comment.
이... 이게 무슨일이야..!
감사합니다 🙇♀️
| if (auction.isDeleted()) { | ||
| throw new InvalidAuctionToAskQuestionException("삭제된 경매입니다."); | ||
| } |
There was a problem hiding this comment.
질문
DB에서 조회할 때 처음부터 삭제되지 않은 경매만 조회할 수 있을 것 같은데, 조건 없이 조회해 서비스 단에서 검증해주신 이유가 있으실까요?
서비스 사용자가 봤을 때에는 삭제된 경매는 보이지 않을 것이기 때문에 바로 삭제되지 않은 Auction만 찾으면 될 것 같아서 질문드립니다
There was a problem hiding this comment.
다른 기능 구현 시 이와 동일하게 진행했기 때문입니다.
하지만, 해당 방법이 더 적절할 것 같아 현재 로직에서는 말씀해 주신 내용과 같이 수정한 후 따로 이슈를 만들어 다른 부분에서도 해당 로직으로 수정할 수 있도록 하겠습니다.
| } | ||
| } | ||
|
|
||
| // TODO: 2023-10-04 [고민] 내가 참여한 경매와 같이 user에 경매 조회가 있는 것처럼, 경매 쪽에 Q&A 조회 메서드가 있는 게 적절할까요? |
There was a problem hiding this comment.
선택
저라면 Auction 패키지에서 별도의 클래스(Controller - Service)를 만들어서 처리해줄 것 같기는 합니다
다만 어떤 로직은 분리되어있고 어떤 로직은 하나로 합쳐져 있는 상황이라 전체적으로 회의를 하고 리펙토링을 해야 할 것 같습니다
그래서 지금 당장은 선택에 맡기도록 하겠습니다
There was a problem hiding this comment.
현재 Auctin 패키지에 별도의 클래스만 만들어 작업한 상태입니다.
해당 부분은 일단 유지하도록 하겠습니다.
| .orElseThrow(() -> new UserNotFoundException("해당 사용자를 찾을 수 없습니다.")); | ||
| final Auction auction = auctionRepository.findById(questionDto.auctionId()) | ||
| .orElseThrow(() -> new AuctionNotFoundException("해당 경매를 찾을 수 없습니다.")); | ||
| checkInvalidAuction(auction); |
| // then | ||
| em.flush(); | ||
| em.clear(); |
There was a problem hiding this comment.
필수
@Id 전략을 Identity로 사용하고 있기 때문에 해당 구문은 필요없을 것 같습니다
em.flush()랑 em.clear()를 호출하지 않아도 insert문이 나가게 되므로 삭제하면 될 것 같습니다
There was a problem hiding this comment.
지난 pr 코멘트와 의견이 상반되어 헷갈리네요...
해당 내용과 어떤 점이 다르기에 의견이 바뀌신건지 궁금합니다.
There was a problem hiding this comment.
지난 pr 코멘트의 경우 Fixture를 적용하기 전의 코드로 인지하고 코멘트를 달았었습니다
근데 지금보니까 아니네요? 이 부분은 제가 착각한 것 같습니다
죄송합니다!!!!!!!!!!!!!!!!!!!!
변명을 하자면 지난 pr 코멘트에서 실제 em.flush()와 em.clear()를 호출하는 부분이 github가 그...접는 페이지 표시로 생략해서 헷갈린 것 같네요
조금 더 설명을 하자면
지난 pr 코멘트의 경우 given절에서 테스트하고자 하는 주체가 아닌 다른 값들(신고 테스트하는데 필요한 auction이랑 user 등등)을 저장해주고 em.flush()와 em.clear()를 호출하는 과정으로 인지했었습니다
그래서 테스트하고자 하는 대상(신고 Repository)의 정확한 동작을 확인하기 위해 given절에서 세팅한 em.flush()로 메모리 DB에 쿼리를 실행시키고, em.clear()로 1차 캐시에 캐싱된 값을 모두 제거를 한 뒤에 테스트를 해야한다는 의미였습니다
지금 테스트의 경우 Fixture에 em.flush()와 em.clear()를 호출하기 때문에 given절에서의 처리는 모두 끝난 상태입니다
해당 부분은 위의 이유로 반드시 호출되어야 하는 상태라고 생각합니다
다른 점은 이제부터인데
save() 호출 시 id 전략이 Identity이므로 save() 호출 즉시 insert를 날리고 그 결과를 save()의 반환값으로 반환합니다
그러므로 em.flush()는 이미 save() 호출 시에 호출이 됩니다 (save() 메서드 호출 시 em.flush()가 내부적으로 호출)
또한 save()의 결과를 이미 반환하므로 캐싱된 값을 제거하는 em.clear()를 호출하더라도 이미 반환 받은 값에는 영향을 주지 않습니다
그러므로 필요가 없는 코드라고 볼 수 있을 것 같습니다
There was a problem hiding this comment.
이해했습니다!
그래서 저는 해당 부분이 필요는 없지만 명시적으로 작성해 주는 코드라고 생각했네요.
이번 pr에서는 해당 부분 코드를 제거하는 방향으로 가고, 이후에 이슈를 만들어 다른 테스트에서도 해당 코드를 제거할 수 있도록 하겠습니다.
| em.flush(); | ||
| em.clear(); |
There was a problem hiding this comment.
필수
JpaQuestionRepositoryTest와 동일합니다
삭제 부탁드립니다
|
|
||
|
|
| given(answerService.create(any(CreateAnswerDto.class))).willReturn(생성된_답변_아이디); | ||
|
|
||
| // when & then | ||
| final ResultActions resultActions = mockMvc.perform(RestDocumentationRequestBuilders.post("/questions/{questionId}/answers", 질문_아이디) |
There was a problem hiding this comment.
선택
140글자인데 = 앞에서 개행 한 번 어떠신가요
|
|
||
| @ParameterizedTest | ||
| @MethodSource("provideAnswerRequestWithEmptyContent") | ||
| void 답변을_입력하지_않은_경우_답변시_400을_반환한다(final CreateAnswerRequest 답변_내용이_빈값인_답변_등록_request) throws Exception { |
There was a problem hiding this comment.
칭찬
칭찬해요!!!!!!!!!!!!!!
@MethodSource로 명확하게 값을 전달할 수 있네요
| @@ -0,0 +1,53 @@ | |||
| package com.ddang.ddang.questionandanswer.presentation; | |||
There was a problem hiding this comment.
저도 패키지명 qna로 바꾸는 것이 좋다고 생각합니다
| @RestController | ||
| @RequestMapping("/questions") | ||
| @RequiredArgsConstructor | ||
| public class QuestionAndAnswerController { |
There was a problem hiding this comment.
패키지명이 qna로 변경된다면 컨트롤러 이름도 QnaController로 변경해도 좋을 것 같아요
| .build(); | ||
| } | ||
|
|
||
| // TODO: 2023/08/18 [고민] auctionId를 클라이언트에게서 받기 vs create 후 생성된 객체 아이디가 아닌 경매 아이디도 받아오기 (일단 전자) |
There was a problem hiding this comment.
질문
실질적으로 answerSerivce.create()의 반환값을 사용하지 않고 있는데, 그 반환값을 Answer의 id로 하지 않고 aucrionId를 반환하도록 하는 것은 이상한가요? 이상하다면 전 지금이 좋습니다.
There was a problem hiding this comment.
선택
저도 지금이 좋은 것 같습니다.
answerService에서 AuctionId를 반환하는 것이 서비스에서 컨트롤러를 알고 필요한 값을 골라서 주는 느낌이라 presentation 레이어 변경에 따라 서비스에도 영향이 있을 것 같아요.
There was a problem hiding this comment.
개인적으로 Answer를 save()하는 메서드에서 auctionId가 넘겨지기를 기대하지는 않을 것 같고, 동일하게 Id라 헷갈릴 것 같아 해당 방법은 적절하지 않다고 생각합니다.
그래서 auctionId를 반환하는 방식으로 수정할 것이라면 dto 등을 통해 묶어줘야 하지 않을까 생각했습니다.
그런데 저도 지금 상태가 현재 후보들 중에선 가장 적절하다고 생각해 유지하도록 하겠습니다!
| final Question question = questionRepository.findById(answerDto.questionId()) | ||
| .orElseThrow(() -> | ||
| new AuctionNotFoundException("해당 질문을 찾을 수 없습니다.") | ||
| ); |
There was a problem hiding this comment.
필수
질문을 찾을 수 없는 것인데 AuctionNotFoundException이 터지네요
| if (auction.isDeleted()) { | ||
| throw new InvalidAuctionToAskQuestionException("삭제된 경매입니다."); | ||
| } |
| } | ||
| } | ||
|
|
||
| // TODO: 2023-10-04 [고민] 내가 참여한 경매와 같이 user에 경매 조회가 있는 것처럼, 경매 쪽에 Q&A 조회 메서드가 있는 게 적절할까요? |
There was a problem hiding this comment.
내가 참여한 경매도 컨트롤러만 user 패키지에 있고 서비스는 auction 서비스에 있었기 때문에 이대로 가도 무방하다고 생각합니다.
| @ToString(of = {"id", "content"}) | ||
| public class Answer extends BaseCreateTimeEntity { | ||
|
|
||
| // TODO: 2023-10-04 [고민] answer에 writer를 만들어 바로 가져오는 방식이 좋을까요? |
There was a problem hiding this comment.
채팅방에서도 참여자 두 명을 다 필드에 두지 않고 한 명은 경매의 판매자를 통하도록 되어있어서 이대로 가도 될 것 같아요. 만약 수정한다면 채팅방 쪽도 동일하게 수정이 필요할 것 같습니다.
There was a problem hiding this comment.
이번 pr에서는 현재 구조를 유지한 후 도메인 범위 축소 회의를 진행할 때 다시 이야기해 보도록 했기에 유지하도록 하겠습니다.
| JpaAuctionRepository auctionRepository; | ||
|
|
||
| @Autowired | ||
| JpaQuestionRepository questionRepository; |
There was a problem hiding this comment.
필수
서비스 fixture에서 사용되는 접근제어자는 private으로 통일하기로 했습니다!
There was a problem hiding this comment.
중간 커밋에서 해당 피드백이 달린 것 같습니다.
최종 요청 파일에서는 private이 붙어 있습니다!
| @RestController | ||
| @RequestMapping("/questions") | ||
| @RequiredArgsConstructor | ||
| public class QuestionAndAnswerController { |
| .build(); | ||
| } | ||
|
|
||
| // TODO: 2023/08/18 [고민] auctionId를 클라이언트에게서 받기 vs create 후 생성된 객체 아이디가 아닌 경매 아이디도 받아오기 (일단 전자) |
There was a problem hiding this comment.
선택
저도 지금이 좋은 것 같습니다.
answerService에서 AuctionId를 반환하는 것이 서비스에서 컨트롤러를 알고 필요한 값을 골라서 주는 느낌이라 presentation 레이어 변경에 따라 서비스에도 영향이 있을 것 같아요.
| // given | ||
| given(tokenDecoder.decode(eq(TokenType.ACCESS), anyString())).willReturn(Optional.of(사용자_ID_클레임)); | ||
| given(answerService.create(any(CreateAnswerDto.class))) | ||
| .willThrow(new AuctionNotFoundException("해당 질문을 찾을 수 없습니다.")); |
There was a problem hiding this comment.
필수
리마인드 차원에서 작성합니다.
엔초가 남겨준 날카로운 리뷰 반영 완료되면 여기도 잊지 않고 바꿔주세요!!
다른 곳은 테스트에서 깨질 것 같은데 여기는 isNotFound()인지만 확인하고 있어 혹시나 하는 마음에 코멘트 달아둡니다!
There was a problem hiding this comment.
오..!
메리가 말씀 안 해주셨으면 놓칠 뻔했네요.
감사합니다 메리!
| } | ||
| } | ||
|
|
||
| // TODO: 2023-10-04 [고민] 내가 참여한 경매와 같이 user에 경매 조회가 있는 것처럼, 경매 쪽에 Q&A 조회 메서드가 있는 게 적절할까요? |
There was a problem hiding this comment.
선택
지토의 방식이 좋은 것 같긴한데 백엔드 회의에서 이야기 했던 것처럼 현재는 유지하고 패키지 구조에 대해 논의해본 뒤 결정해도 될 것 같습니다.
There was a problem hiding this comment.
이번 pr에서는 현재 구조를 유지한 후 도메인 범위 축소 회의를 진행할 때 다시 이야기해 보도록 했기에 유지하도록 하겠습니다.
| final ReadQuestionAndAnswerDto 두번째_질문 = questionAndAnswerDtos.get(1); | ||
| softAssertions.assertThat(두번째_질문.readQuestionDto()).isEqualTo(질문_정보_dto2); | ||
| softAssertions.assertThat(두번째_질문.readAnswerDto()).isEqualTo(답변_정보_dto2); | ||
|
|
| @RestController | ||
| @RequestMapping("/auctions") | ||
| @RequiredArgsConstructor | ||
| public class AuctionQuestionController { |
There was a problem hiding this comment.
선택
패키지명과 클래스명을 변경하게 된다면 여기도 AuctionQnaController로 통일해주시면 좋을 것 같습니다.
| @ToString(of = {"id", "content"}) | ||
| public class Answer extends BaseCreateTimeEntity { | ||
|
|
||
| // TODO: 2023-10-04 [고민] answer에 writer를 만들어 바로 가져오는 방식이 좋을까요? |
There was a problem hiding this comment.
선택
엔초가 말씀해주신 것처럼 채팅방에서 경매를 통해 판매자를 가져오게 되어있습니다.
저는 남겨주신 것처럼 answer에 writer를 바로 가져오는 방식이 좋을 것 같다고 생각했습니다. (엔초 말처럼 변경된다면 이에 맞춰 채팅방도 수정이 필요할 것 같아요.)
이유는 채팅방을 테스트 할 때 판매자 정보를 얻기 위해 항상 경매에 의존적이게 되어 비효율적인 테스트가 발생한다고 생각했기 때문입니다.
| private final QuestionService questionService; | ||
|
|
||
| @GetMapping("/{auctionId}/questions") | ||
| public ResponseEntity<ReadQuestionAndAnswersResponse> readAllByAuctionId(@PathVariable final Long auctionId) { |
There was a problem hiding this comment.
질문 & 선택
해당 로직을 auction.presentation 하위에 위치시키신 이유가 궁금합니다!
경매글을 통해 qna에 접근하긴 하지만 qna(questionandanswer) 하위에 있는 서비스를 의존하고 있다는 점에서 해당 컨트롤러는 qna 패키지 하위에 위치하는 것도 충분히 자연스러울 것 같다고 생각했습니다.
채팅방도 경매글을 통해 조회하는 로직이 있지만 /chattings uri를 통해 조회하는 것처럼 qna도 비슷하게 가져가면 어떤지 건의드려봅니다!
There was a problem hiding this comment.
저는 채팅보다는 내가 참여한 경매 목록에 더 가깝다고 생각했습니다.
채팅은 경매에 종속적이기도 하지만, 채팅 목록을 통해 접근 가능하기에 메리가 말씀해 주신 방법이 적절하다고 생각합니다.
하지만, qna의 경우는 경매 내에만 존재하며, 해당 경매에 대한 qna이기에 내가 참여한 경매 목록과 같은 상황에 더 가깝다고 생각해 현재의 구조가 되었습니다.
There was a problem hiding this comment.
이해했습니다~! 이대로 진행하시면 좋을 것 같습니당
# Conflicts: # backend/ddang/src/main/java/com/ddang/ddang/exception/GlobalExceptionHandler.java # backend/ddang/src/main/resources/static/docs/docs.html # backend/ddang/src/test/java/com/ddang/ddang/configuration/CommonControllerSliceTest.java
| @Transactional | ||
| public Long create(final CreateQuestionDto questionDto) { |
There was a problem hiding this comment.
칭찬
헉 이럴수가.. 리뷰 빼먹었었네요.. 칭찬해요!
| public User getWriter() { | ||
| return question.getWriter(); | ||
| } |
There was a problem hiding this comment.
필수
Answer의 writer는 auction의 seller가 되지 않을까요?
| public User getWriter() { | |
| return question.getWriter(); | |
| } | |
| public User getWriter() { | |
| return question.getAuction().getSeller(); | |
| } |
There was a problem hiding this comment.
헉... 정말 큰일 날 뻔 헀네요.
고마워요 엔초!
| public void delete() { | ||
| deleted = DELETED_STATUS; | ||
| } |
There was a problem hiding this comment.
질문
Question을 삭제해도 Answer는 그대로 남아있는 건가요?
There was a problem hiding this comment.
저도 해당 부분을 고민하다 Anwer의 경우 가져오는 방법이 Question을 통해서밖에 없어 굳이 삭제처리를 않았습니다.
그런데 갑자기 드는 생각으로는 Question은 삭제되었음으로 표현되고 Answer는 보여야 하지 않을까 싶기도 하네요!
해당 부분에 대해 처리해두도록 하겠습니다..!
| public boolean isWriter(final User user) { | ||
| return question.getAuction().isOwner(user); | ||
| } | ||
|
|
||
| public void delete() { | ||
| deleted = DELETED_STATUS; | ||
| } | ||
|
|
||
| public User getWriter() { | ||
| return question.getWriter(); | ||
| } |
There was a problem hiding this comment.
필수
추가된 메서드들에 대한 테스트코드가 없는 것 같아요!
| @Test | ||
| void 신고_전인_질문이라면_거짓을_반환한다() { | ||
| // when | ||
| final boolean actual = questionReportRepository.existsByIdAndReporterId(질문.getId(), 신고자.getId()); | ||
|
|
||
| // then | ||
| assertThat(actual).isFalse(); | ||
| } |
There was a problem hiding this comment.
필수
전체 테스트 돌렸을 때 이 테스트코드 실패하네요.. 근데 자코코 돌렸을 땐 왜 build successful이 나올까요?
There was a problem hiding this comment.
제 쪽에서는 실패하는 테스트가 없는데 혹시 어떤 문제인지 알 수 있을까요?
그리고 혹시 모르니 다시 한번 확인해 주시면 감사하겠습니다..!
There was a problem hiding this comment.
필수
저는 계속 실패했다고 나오네요..
왜 제이미는 성공하고 저는 실패하는지는 모르겠지만, jpa 메서드가 잘못되어서 그런 것 같아요.
JpaQuestionReportRepository의 메서드가 existsByQuestionIdAndReporterId()여야 할 것 같습니다. 관련 리뷰 그 쪽에도 달아놓을게요!
There was a problem hiding this comment.
헉 그렇네요..!
제거엔 대체 어떻게 성공이 된걸까요?
알 수 없는 자코코와 인텔리제이...
엔초 덕분에 문제를 찾을 수 있었네요!
꼼꼼한 리뷰 고마워요 엔초!
| public boolean isWriter(final User user) { | ||
| return writer.equals(user); | ||
| } | ||
|
|
||
| public void delete() { | ||
| deleted = DELETED_STATUS; | ||
| } |
There was a problem hiding this comment.
필수
여기도 새로 추가된 메서드들에 대한 테스트코드가 없습니다!
There was a problem hiding this comment.
자코코.. 생각보다 관대하군요..?ㅎㅎ
delete()관련 테스트는 이미 존재했기에 isWriter() 테스트만 추가해 두었습니다.
감사합니다!
There was a problem hiding this comment.
필수
isDelete() 테스트만 있고. delete() 테스트는 없는 것 같아요!
| import com.ddang.ddang.configuration.QuerydslConfiguration; | ||
| import com.ddang.ddang.report.domain.AnswerReport; | ||
| import com.ddang.ddang.report.infrastructure.persistence.fixture.JpaAnswerReportRepositoryFixture; | ||
| import org.assertj.core.api.*; |
| import com.ddang.ddang.configuration.QuerydslConfiguration; | ||
| import com.ddang.ddang.report.domain.QuestionReport; | ||
| import com.ddang.ddang.report.infrastructure.persistence.fixture.JpaQuestionReportRepositoryFixture; | ||
| import org.assertj.core.api.*; |
| import com.ddang.ddang.configuration.IsolateDatabase; | ||
| import com.ddang.ddang.qna.application.dto.ReadQnaDto; | ||
| import com.ddang.ddang.qna.application.dto.ReadQnasDto; | ||
| import com.ddang.ddang.qna.application.exception.InvalidAuctionToAskQuestionException; | ||
| import com.ddang.ddang.qna.application.exception.InvalidQuestionerException; | ||
| import com.ddang.ddang.qna.application.exception.QuestionNotFoundException; |
There was a problem hiding this comment.
안 돼~~~
감사합니다...
아래 내용 모두 수정했습니다!
| import com.ddang.ddang.report.application.exception.InvalidAnswererReportException; | ||
| import com.ddang.ddang.report.application.fixture.AnswerReportServiceFixture; | ||
| import com.ddang.ddang.user.application.exception.UserNotFoundException; | ||
| import org.assertj.core.api.*; |
| import com.ddang.ddang.report.application.exception.InvalidQuestionReportException; | ||
| import com.ddang.ddang.report.application.fixture.QuestionReportServiceFixture; | ||
| import com.ddang.ddang.user.application.exception.UserNotFoundException; | ||
| import org.assertj.core.api.*; |
| public interface JpaAnswerRepository extends JpaRepository<Answer, Long> { | ||
|
|
||
| boolean existsByQuestionId(Long questionId); | ||
|
|
||
| @EntityGraph(attributePaths = {"question", "question.auction", "question.auction.seller"}) | ||
| Optional<Answer> findByIdAndDeletedIsFalse(Long answerId); |
There was a problem hiding this comment.
필수
바로 아래 JpaQuestionRepository에는 파라미터에 final이 붙어있는데 여기에는 누락된 것 같아요!
| ReadAnswerInReportDto answerDto, | ||
| String description | ||
| ) { | ||
| public static ReadAnswerReportDto from(final AnswerReport answerReport) { |
|
|
||
| public interface JpaAnswerReportRepository extends JpaRepository<AnswerReport, Long> { | ||
|
|
||
| boolean existsByIdAndReporterId(Long id, Long ReporterId); |
There was a problem hiding this comment.
필수
여기에도 final 붙이는걸로 통일해주시면 감사하겠습니다
|
|
||
| public interface JpaQuestionReportRepository extends JpaRepository<QuestionReport, Long> { | ||
|
|
||
| boolean existsByIdAndReporterId(Long id, Long reporterId); |
There was a problem hiding this comment.
필수
여기에도 final 붙이는걸로 통일해주시면 감사하겠습니다아
| public boolean isWriter(final User user) { | ||
| return question.getAuction().isOwner(user); | ||
| } |
There was a problem hiding this comment.
필수
isWriter() 테스트코드가 없네요
| @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) | ||
| @SuppressWarnings("NonAsciiCharacters") | ||
| class QuestionTest extends QuestionFixture { | ||
|
|
There was a problem hiding this comment.
필수
isDeleted() 테스트만 있고 delete() 테스트는 없는 것 같습니다.
| @Test | ||
| void 신고한_질문이라면_참을_반환한다() { | ||
| // when | ||
| final boolean actual = questionReportRepository.existsByIdAndReporterId(이미_신고한_질문.getId(), 신고자.getId()); | ||
|
|
||
| // then | ||
| assertThat(actual).isFalse(); | ||
| } | ||
|
|
There was a problem hiding this comment.
필수
"참을 반환한다"인데 false를 검증하고 있어요
| 질문_신고1 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | ||
| 질문_신고2 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | ||
| 질문_신고3 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | ||
| 질문_신고4 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); |
There was a problem hiding this comment.
필수
질문 신고 1,2,3,4의 신고자가 각각 달라야 할 것 같아요
| 질문_신고1 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고2 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고3 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고4 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고1 = new QuestionReport(신고자, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고2 = new QuestionReport(신고자2, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고3 = new QuestionReport(신고자3, 이미_신고한_질문, "신고합니다"); | |
| 질문_신고4 = new QuestionReport(신고자4, 이미_신고한_질문, "신고합니다"); |
There was a problem hiding this comment.
허거덩 만들어두고 사용조차 하지 않았군요..!
수정했습니다!
| @Test | ||
| void 신고_전인_질문이라면_거짓을_반환한다() { | ||
| // when | ||
| final boolean actual = questionReportRepository.existsByIdAndReporterId(질문.getId(), 신고자.getId()); | ||
|
|
||
| // then | ||
| assertThat(actual).isFalse(); | ||
| } |
There was a problem hiding this comment.
필수
저는 계속 실패했다고 나오네요..
왜 제이미는 성공하고 저는 실패하는지는 모르겠지만, jpa 메서드가 잘못되어서 그런 것 같아요.
JpaQuestionReportRepository의 메서드가 existsByQuestionIdAndReporterId()여야 할 것 같습니다. 관련 리뷰 그 쪽에도 달아놓을게요!
|
|
||
| public interface JpaQuestionReportRepository extends JpaRepository<QuestionReport, Long> { | ||
|
|
||
| boolean existsByIdAndReporterId(final Long id, final Long reporterId); |
There was a problem hiding this comment.
필수
메서드가 의도와 다르게 동작할 것 같네요.
| boolean existsByIdAndReporterId(final Long id, final Long reporterId); | |
| boolean existsByQuestionIdAndReporterId(final Long questionId, final Long reporterId); |
There was a problem hiding this comment.
필수
JpaAnswerReportRepository도 boolean existsByAnswerIdAndReporterId(final Long answerId, final Long ReporterId);로 수정해주시면 감사하겠습니다.
| @Test | ||
| void 답변의_작성자가_아니라면_참을_반환한다() { | ||
| // when | ||
| final boolean actual = 답변.isWriter(답변_작성자가_아닌_사용자); | ||
|
|
||
| // then | ||
| assertThat(actual).isFalse(); | ||
| } |
There was a problem hiding this comment.
필수
| @Test | |
| void 답변의_작성자가_아니라면_참을_반환한다() { | |
| // when | |
| final boolean actual = 답변.isWriter(답변_작성자가_아닌_사용자); | |
| // then | |
| assertThat(actual).isFalse(); | |
| } | |
| @Test | |
| void 답변의_작성자가_아니라면_거짓을_반환한다() { | |
| // when | |
| final boolean actual = 답변.isWriter(답변_작성자가_아닌_사용자); | |
| // then | |
| assertThat(actual).isFalse(); | |
| } |
* feat: 질문과 답변에 대한 엔티티 추가 * feat: 질문 레포지토리 추가 * feat: 질문 등록 서비스 추가 * feat: 질문 등록 api 추가 * test: QuestionService 테스트 추가 * feat: 답변 레포지토리 추가 * feat: 답변 등록 서비스 추가 * refactor: 컨트롤러 클래스 명 수정 * feat: 답변 등록 api 추가 * feat: 질문 및 답변 조회 레파지토리 추가 * feat: 질문 및 답변 조회 서비스 추가 * rename: 하위 패키지 생성 * feat: 질문과 답변 목록 전체 조회 api 추가 * test: 테스트 픽스처 접근 제어자 설정 * docs: 고민 todo 추가 * feat: 질문과 답변관련 flyway 스크립트 추가 * docs: api 문서 최신화 * refactor: 불필요한 개행 제거 * feat: 경매 조회시 삭제된 경매를 제외해주는 메서드 추가 * refactor: 경매 조회시 삭제된 경매는 존재하지 않는 경매로 처리되도록 수정 * refactor: 개행 추가 * test: 불필요한 코드 제거 * test: 컨벤션에 맞춰 개행 * style: todo 제거 * refactor: 예외처리를 적절한 커스텀 예외로 변경 * rename: QuestionAndAnswer를 Qna로 축약해 사용 * ci: 브랜치 최신화 충돌 문제 해결 * feat: 질문 삭제 기능 추가 * feat: 질문 삭제 기능 서비스 추가 * feat: 질문 삭제 기능 api 추가 * refactor: 삭제된 질문은 조회되지 않도록 수정 * test: 메서드 네이밍 수정 * feat: 답변 삭제 기능 서비스 추가 * feat: 답변 삭제 api 추가 * feat: 질문과 답변에 삭제 여부 필드 추가 * feat: 질문 신고 엔티티, 레파지토리 추가 * feat: 트랜잭션 어노테이션 추가 * feat: 질문 신고 등록 서비스 추가 * feat: 질문 신고 등록 api 추가 * feat: 질문 신고 조회 레포지토리 추가 * feat: 질문 신고 조회 서비스 추가 * feat: 질문 신고 조회 api 추가 * feat: 답변 신고 엔티티, 레포지토리 추가 * feat: 답변 신고 등록 서비스 추가 * feat: 답변 신고 등록 api 추가 * feat: 답변 신고 조회 레포지토리 추가 * feat: 답변 신고 조회 서비스 추가 및 테스트 수정 * feat: 답변 신고 조회 api 추가 * feat: flyway 스크립트 report 테이블 생성 쿼리 추가 * ci: 충돌문제 해결 * fix: 답변의 작성자 조회 시 질문된 경매의 판매자를 전달하도록 수정 * test: 누락된 테스트 추가 * test: 누락된 테스트 추가 * refactor: import 와일드카드 제거 * refactor: 누락된 final 추가 * refactor: 컨벤션에 따른 개행 추가 * docs: 누락된 api 문서화 추가 * test: 누락된 테스트 추가 * fix: 신고 존재여부 조회 시 의도와 다른 로직 문제 해결 * test: 테스트 픽스처 수정 * test: 메서드 명 수정
📄 작업 내용 요약
Q&A 기능 추가
🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
+) 두 번째 pr 요청 커밋 범위
복잡한 내용을 없을 것 같습니다.
고민인 내용들은 todo를 통해 작성해 두었습니다.
api 한번 확인해주시면 감사하겠습니다.
Q&A API 명세
질문 등록하기
request
POST/questionsresponse
답변 등록하기
request
POST/questions/{quesionId}/answersresponse
질문과 답변 모두 조회하기 (경매 ID를 통해)
request
GET/auctions/{auctionId}/questionsresponse
📎 Issue 번호