Skip to content

Comments

Kwon ju hwan#1

Open
KwonJuHwan wants to merge 33 commits intoboocam-project:KwonJuHwanfrom
KwonJuHwan:KwonJuHwan
Open

Kwon ju hwan#1
KwonJuHwan wants to merge 33 commits intoboocam-project:KwonJuHwanfrom
KwonJuHwan:KwonJuHwan

Conversation

@KwonJuHwan
Copy link

JPA 환경에서 User라는 이름으로 Entity 구현 시, 컴파일 에러 발생이 일어나기 때문에, user-> member 로 변경하였습니다.

@Nine-JH
Copy link
Contributor

Nine-JH commented Jul 27, 2023

좋은 피드백 감사합니다! 일단은 수정 부분을 제외한 일부 로직들이 들어갔기 때문에 네이밍 관련 부분만 수정하는 브랜치를 따로 파서 작업 할 것 같아요
팀 회의때 공지하고 변경하도록 하겠습니다 😀

@KwonJuHwan
Copy link
Author

넵 확인하고 변경하겠습니다

@JeongUijeong
Copy link

JeongUijeong commented Jul 30, 2023

클래스에 트랜잭션 애노테이션 사용하니 좋네요! 저도 참고하도록 하겠습니다~ 👍🏻 더불어 컨트롤러 테스트까지 해보시면 좋을 것 같습니다 😀

@ghrltjdtprbs
Copy link

좋습니다! 정말 수고하셨어요 ㅎㅎㅎ😊

@KwonJuHwan
Copy link
Author

컨트롤러 테스트까지 구현하였고 로그인 / 회원가입 테스트 구현하였습니다.
등급업과 일반/관리자 authentication 부분은 프로세스가 추가되면 구현하겠습니다

@KwonJuHwan
Copy link
Author

자현님 코드 기반으로 컨트롤러 테스트, 엔티티 자체 태스트, 서비스 테스트를 분리하여 진행하였습니다.

@Nine-JH Nine-JH changed the base branch from main to KwonJuHwan August 3, 2023 10:36
@Nine-JH
Copy link
Contributor

Nine-JH commented Aug 3, 2023

고생하셨습니다👍 우선 브랜치가 main으로 설정되어 있어서 fcstudy-project:KwonJuHwan 으로 변경하겠습니다!

Copy link
Contributor

@Nine-JH Nine-JH left a comment

Choose a reason for hiding this comment

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

추가적인 코드 리뷰입니다! 도움이 되었으면 좋겠네요

Copy link

@JeongUijeong JeongUijeong left a comment

Choose a reason for hiding this comment

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

코드가 많이 추가됐네요! 정말 수고 많으셨습니다! 😀👍🏻 이미 좋은 피드백 많이 받으신 것 같아서 저는 따로 얹을 말이 없네요. 앞으로도 파이팅입니다~

  • 제가 구현하고 나서 다시 보니 몇 가지 생각나서 코멘트 추가했습니다!

Copy link

@kjyyjk kjyyjk left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!!

Copy link

@JeongUijeong JeongUijeong left a comment

Choose a reason for hiding this comment

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

제가 구현하면서 고민했던 부분이나 신경 쓴 부분 위주로 코멘트를 몇 개 남겨봤습니다! 😊
코멘트에 이모티콘 달아주시면 확인해주신 걸로 알고 approve 하도록 하겠습니다~~!

}
public void sellProduct(){
this.amount -= 1;
}

Choose a reason for hiding this comment

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

상품을 2개 이상 구매하면 수량 차감 메서드를 반복 호출해야 할 수도 있겠네요..!
여러 개 구매 시 어떻게 처리하면 좋을 지 고민해보시면 좋을 것 같아요 😊

Copy link
Author

Choose a reason for hiding this comment

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

상황에 대한 메소드 잘못된 부분 지적 감사드립니다. 메소드 안에 추가 로직 구현하였습니다


@Getter
public class ProductRegisterDTO {
@NotBlank

Choose a reason for hiding this comment

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

상품 명에 빈칸이 필요할 수도 있지 않을까 생각해봤습니다! @NotNull로 변경하면 띄어쓰기를 허용할 수 있습니다 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

상품명에 빈칸이 필요한 경우가 구체적으로 어떤 것이 있을까요? 상품명이 빈칸 = " "인 경우는 상품명이 없다고 판단하여 묶어서 테스트를 진행하였는데 의견 부탁드립니다!

Choose a reason for hiding this comment

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

제가 생각했던 빈칸이 필요한 경우는 예를 들어, "탈모 방지 샴푸" 와 같이 상품명에 띄어쓰기가 들어있는 경우였습니다! 😊

Copy link
Author

Choose a reason for hiding this comment

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

@notblank는 글자 없이 " " 이렇게 띄어쓰기만 사용자가 입력하는 경우에만 예외를 터트리는 걸로 알고 있어요! 글자 사이에 있는 블랭크는 예외를 터트리지 않는 걸로 알고 있슴다

Choose a reason for hiding this comment

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

오 그렇군요! 제가 잘못 이해했나 보네요😅 알려주셔서 감사합니다! 제 코드를 수정해야겠네요 하하 👍🏻

ProductService productService;

@PostMapping("/productRegister")
public String productRegister(@Validated @RequestBody ProductRegisterDTO DTO, BindingResult br) throws BindException {

Choose a reason for hiding this comment

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

관리자만 상품 등록 및 재고 추가가 가능하도록 세션을 통해 확인해보는 건 어떨까요? 😀

Copy link
Author

Choose a reason for hiding this comment

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

첫번째 유저 질문을 까먹고 있었네요. 좋은 지적 감사드립니다!

@JeongUijeong JeongUijeong self-requested a review August 4, 2023 13:51
@Nine-JH Nine-JH self-requested a review August 7, 2023 07:41
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.

5 participants