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

ADD::회원가입 기능 #4

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

ADD::회원가입 기능 #4

wants to merge 1 commit into from

Conversation

hsem4717
Copy link
Collaborator

@hsem4717 hsem4717 commented Mar 5, 2023

📄 Summary


🔨 Tasks


🙋🏻 More

@hsem4717 hsem4717 requested a review from gimhanul March 5, 2023 23:19
Comment on lines +21 to +25
if (userService.signup(userSignUpRequest).equals("OK")) {
return new ResponseEntity(HttpStatus.CREATED);
}
return new ResponseEntity(HttpStatus.BAD_REQUEST);
}
Copy link
Member

Choose a reason for hiding this comment

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

service에서는 딱히 완료됐다는 OK를 보내줄 필요가 없습니다
어차피 에러가 나면 예외를 던지기 때문에 굳이 ok를 보내지 않아도 예외가 나지 않았다면 실행이 잘 된 것으로 볼 수 있습니다.

Copy link
Member

Choose a reason for hiding this comment

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

responseEntity도 저렇게 안 ㅆㅓ줘도 됩니다
@RestController 가 알아서 ResponseEntity에 담아서 보내줍니다
HttpStatus를 붙이고 싶으면 컨트롤러 메소드 위에 @HttpStatus annotation을 달면 해당 스테이터스로 보내줍니다

Copy link
Member

Choose a reason for hiding this comment

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

그래서 서비스 쪽에서 return "OK" 지우시고 그냥 여기서는 service 호출만 하십셔

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
Member

Choose a reason for hiding this comment

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

이건 진짜 말 그대로 찬찬히 읽어보셈...

Comment on lines +29 to 45
@Column(length = 50)
private String email;

@Column(nullable = false)
private int rank;
private int ranking;

@Column(nullable = false)
private int amountOfCommit;

@Column(nullable = false, length = 300)
@Column( length = 300)
private String profileImage;

@Column(nullable = false, length = 100)
@Column(length = 100)
private String githubId;

@Column(nullable = false, length = 100)
@Column( length = 100)
private String githubBio;
Copy link
Member

Choose a reason for hiding this comment

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

이건 왜 nullable을 허용했쥬?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

String 문자열들만
SQL Error: 1048, SQLState: 23000
Column 'email' cannot be null
이 오류가 계속 뜹니다

Copy link
Member

Choose a reason for hiding this comment

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

이건 나도 한 번 봐야할 거 같은데 객체 생성할 때 저 값들 잘 넣어주고 있는지 확인 해 바 not null은 무조건 있어야 댐 무결성을 위해서...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분 안고쳐져요...

src/main/resources/application.properties Show resolved Hide resolved
src/main/resources/templates/index.html Show resolved Hide resolved

@EnableJpaAuditing
Copy link
Member

Choose a reason for hiding this comment

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

configuration 파일 분리해주세용

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
Member

@gimhanul gimhanul Mar 9, 2023

Choose a reason for hiding this comment

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

@Configuration annotation 검색

@hsem4717 hsem4717 requested a review from gimhanul March 7, 2023 10:52
@gimhanul
Copy link
Member

gimhanul commented Mar 9, 2023

3학년 1반 찾아오렴 ^__^...

@hsem4717
Copy link
Collaborator Author

hsem4717 commented Mar 9, 2023

진짜로요...?

@gimhanul
Copy link
Member

gimhanul commented Mar 9, 2023

진짜로요...?

아니면 오늘밤커뮤싥ㄱ?

@hsem4717
Copy link
Collaborator Author

hsem4717 commented Mar 9, 2023

저 때문에 점호 후에 따로 시간 쓰실 필요 없이 코멘트로 알려주시면 인터넷 검색해서 오늘 밤 부터 주말동안 계속 고쳐보겠습니다 오늘도 못고치면 말씀드려 다음에 부탁드리겠습니다 신경 써주셔서 감사합니다

@gimhanul
Copy link
Member

gimhanul commented Mar 9, 2023

저 때문에 점호 후에 따로 시간 쓰실 필요 없이 코멘트로 알려주시면 인터넷 검색해서 오늘 밤 부터 주말동안 계속 고쳐보겠습니다 오늘도 못고치면 말씀드려 다음에 부탁드리겠습니다 신경 써주셔서 감사합니다

코멘트 달아놨음니다 확인 ㄱㄱ

@hsem4717
Copy link
Collaborator Author

hsem4717 commented Mar 9, 2023

감사합니다

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