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

7주차 미션 / 서버 4조 조동현 #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mr8356
Copy link

@mr8356 mr8356 commented Nov 9, 2024

api 찾기위해서 jsp 파일 일일히 찾아보느라 힘드네요,,

Copy link
Contributor

@lyouxsun lyouxsun left a comment

Choose a reason for hiding this comment

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

7주차 미션도 수고하셨습니다!!
스프링의 기본 기능에 대해서 잘 이해하고 계신 것 같아요😊
이번 주까지 배운 스프링의 동작 원리를 잘 활용하셔서 남은 �기간 동안에도 많은 걸 배우셨으면 좋겠습니다. 화이팅!!! 🔥🔥

// public HomeController(QuestionRepository questionRepository) {
// this.questionRepository = questionRepository;
// }

/**
* TODO: showHome
Copy link
Contributor

Choose a reason for hiding this comment

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

기능을 모두 구현한 후, TODO 주석은 지우는게 좋습니다!

/**
* TODO: showQnA
*/
// <a href="qna/show?questionId=${question.questionId}">${question.title}</a>
@GetMapping("qna/show")
public String showQna(@RequestParam(required = true) int questionId, Model model) {
Copy link
Contributor

Choose a reason for hiding this comment

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

questionId가 없으면 예외가 발생하도록 처리해 주셨네요! 👍🏻👍🏻



// @PostMapping("/addAnswerV1")
public String addAnswerV1(@RequestParam int questionId,
Copy link
Contributor

Choose a reason for hiding this comment

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

addAnswerV1, 2, 3에도 Question의 Answer 개수를 증가시키는 로직이 있으면 더 좋을 것 같습니다

    Question question = questionRepository.findByQuestionId(answer.getQuestionId());
    question.increaseCountOfAnswer();
    questionRepository.updateCountOfAnswer(question);


@Slf4j
@Controller
@RequiredArgsConstructor
public class QuestionController {
Copy link
Contributor

Choose a reason for hiding this comment

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

클래스 단에서 @RequestMapping("/qna") 로 묶어주면 더 좋을 것 같아요:)

if (UserSessionUtils.isLoggedIn(session)) {
return "/qna/form";
}
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.

로그인 된 회원이 아닌 경우에는 로그인 화면으로 리다이렉트 시키는건 어떨까요?

Question question = new Question(writer, title, contents, 0);
questionRepository.insert(question);
model.addAttribute("question", question);
return "redirect:/qna/show";
Copy link
Contributor

Choose a reason for hiding this comment

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

리다이렉트는 서버가 클라이언트에게 새로운 요청을 보내도록 하기 때문에, model 속 데이터가 유지되지 않습니다. 따라서 이 경우에는 Model에 대한 로직이 없어도 괜찮을 것 같아요!

Copy link
Contributor

@lyouxsun lyouxsun Nov 18, 2024

Choose a reason for hiding this comment

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

동현님의 QnaController/qna/show API를 보면 파라미터가 없을 경우 예외가 발생하도록 구현하셔서, 생성된 질문이 저장된 후 리다이렉트하는 과정에서 예외가 발생합니다.

redirct url을 "redirect:/qna/show?questionId="+question.getId() 로 수정하는 게 좋을 것 같습니다!

* showUpdateQuestionFormV1 : @RequestParam, HttpServletRequest, Model
* showUpdateQuestionFormV2 : @RequestParam, @SessionAttribute, Model
*/
// 참고한 jsp tag /qna/form?questionId=${question.questionId}"
@GetMapping("/qna/form")
Copy link
Contributor

Choose a reason for hiding this comment

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

동일한 url 요청을 처리하는 메서드가 여러 개 있으면 오류가 발생할 수 있습니다.
하나를 제외한 @RequestMapping 는 모두 주석처리해 주세요☺️

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