Skip to content

Conversation

@decaffeine
Copy link

Step2 리뷰 요청드립니다.

김혜진(Hyejin Kim)/Escrow개발팀/11ST and others added 2 commits April 3, 2019 01:38
Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

도메인 객체에 로직 구현 및 Acceptance Test 구현 잘 했네요. 💯
도메인 객체에 로직 구현했으면 이에 대한 단위 테스트도 구현하면 어떨까?

this.contents = contents;
}

public Question update(User loginUser, Question updatedQuestion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Iterable<Question> questionsIterable = qnaService.findAll();
List<Question> questions = new ArrayList<>();
questionsIterable.forEach(questions::add);
model.addAttribute("questions",questions);
Copy link
Contributor

Choose a reason for hiding this comment

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

굳이 새로운 List에 담기보다 questionsIterable을 그냥 전달해도 되지 않을까?

Iterable<Question> questionsIterable = qnaService.findAll();
List<Question> questions = new ArrayList<>();
questionsIterable.forEach(questions::add);
model.addAttribute("questions",questions);
Copy link
Contributor

Choose a reason for hiding this comment

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

space convention 위반

public String login(String userId, String password, HttpSession session) throws UnAuthenticationException {
User loginUser = userService.login(userId, password);
session.setAttribute(HttpSessionUtils.USER_SESSION_KEY, loginUser);
return "redirect:/";
Copy link
Contributor

Choose a reason for hiding this comment

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

로그인 실패하는 경우에 대한 예외처리도 하면 좋지 않을까?

softly.assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FOUND);
softly.assertThat((response.getHeaders().getLocation()).getPath().startsWith("/questions"));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

도메인 객체에 로직을 구현했는데 이에 대한 단위 테스트도 구현하면 좋지 않을까?

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