Skip to content

Feat/14 user entity#30

Open
dongyoungs wants to merge 8 commits intodevfrom
feat/14-user_entity
Open

Feat/14 user entity#30
dongyoungs wants to merge 8 commits intodevfrom
feat/14-user_entity

Conversation

@dongyoungs
Copy link

@dongyoungs dongyoungs commented Sep 15, 2023

📝 PR Summary

유저 관련 엔터티 추가

🌲 Working Branch

feat/ user

🌲 TODOs

개발용 유저 생성 API 추가

Related Issues

@donsonioc2010
Copy link
Member

@devmelonlee @dongyoungs Conflicts문제 해결해주세요~

// 스웨거 유저일시 유저 아이디 0 반환
return 0L;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

굿굿 좋습니다

Comment on lines +21 to +32
@GetMapping("/create")
public void createDevUser(@RequestParam String email, @RequestParam String password) {
log.info("email, password : " + email + " " + password);

UserCreateDTO userCreateDTO = new UserCreateDTO(email, password);
devLoginService.execute(userCreateDTO);

// model.addAttribute("email", email);
// model.addAttribute("password",password);

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

RequestParam으로 값이 두개나 들어가는 것은 좋지 않습니다
password는 더더욱 uri에 표시되면 좋지 않겠죠
email, password를 하나의 DTO로 받는다면 좋겠죠?
dto 패키지에 UserCreateRequest와 같은 이름의 DTO를 만들어 받아보세요!
tify 프로젝트의 PostFavorAnswerRequest 클래스를 참고해보세요

Copy link
Contributor

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.

확인했습니다 추후 API 플리퀘스트시 반영해서 올리겠습니다

Comment on lines +7 to +16
public class UserCreateDTO {
private final String email;
private final String password;

// @QueryProjection
public UserCreateDTO(String email, String password) {
this.email = email;
this.password = password;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

좋습니다
위에서 말한 UserCreateRequest와의 통합을 고려해보세요

Comment on lines +18 to +26
public void execute(UserCreateDTO userCreateDTO) {
User user =
User.builder()
.email(userCreateDTO.getEmail())
.password(userCreateDTO.getPassword())
.role(AccountRole.ADMIN)
.build();
userAdaptor.save(user);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@builder 패턴으로 구현한 것 좋습니다

Comment on lines +10 to +21
public class UserUtils {

private final UserAdaptor userAdaptor;

public Long getUserId() {
return SecurityUtils.getCurrentUserId();
}

public User getUser() {
return userAdaptor.query(SecurityUtils.getCurrentUserId());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Utils class로 자주 써야하는 메소드를 따로 구현해놓은 것 좋네요

Comment on lines +14 to +34
@Getter
@MappedSuperclass
@EntityListeners(value = {AuditingEntityListener.class})
public abstract class AbstractTimeStamp {

@Column(
name = "created_at",
nullable = false,
columnDefinition = "TIMESTAMP DEFAULT CURRENT_TIMESTAMP")
@CreationTimestamp
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd hh:mm:ss")
private Timestamp createdAt;

@Column(
name = "updated_at",
nullable = false,
columnDefinition = "TIMESTAMP DEFAULT CURRENT_TIMESTAMP")
@UpdateTimestamp
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd hh:mm:ss")
private Timestamp updatedAt;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

AbstractTimeStamp 추상클래스를 이용해서
어떤 기능을 구현한 걸까요?


@Getter
@Entity
@Table(name = "user")
Copy link
Contributor

Choose a reason for hiding this comment

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

모든 Table 이름의 경우 mysql option과 겹칠 위험을 줄이고
통일성을 위해
tbl_user와 같이 수정해주세요

Copy link
Contributor

Choose a reason for hiding this comment

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

네 수정하겠습니다.

Comment on lines +28 to +29
@Enumerated(EnumType.ORDINAL)
@Column(nullable = false, name = "role", columnDefinition = "varchar(32) default 'USER'")
Copy link
Contributor

Choose a reason for hiding this comment

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

Enum 타입이 mysql에서 varchar로 변하지 않는 것을 이런식으로 오류를 잡는 것 좋습니다.

Comment on lines +24 to +40
boolean etc_param1;

boolean etc_param2;

boolean etc_param3;

boolean etc_param4;

boolean etc_param5;

boolean etc_param6;

boolean etc_param7;

boolean etc_param8;

boolean etc_param9;
Copy link
Contributor

Choose a reason for hiding this comment

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

음 이부분은 주석처리를 먼저 해놓는게....
어떨지?

Copy link
Contributor

Choose a reason for hiding this comment

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

네!

Comment on lines +42 to +48
@Column(
name = "updated_at",
nullable = false,
columnDefinition = "TIMESTAMP DEFAULT CURRENT_TIMESTAMP")
@UpdateTimestamp
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd hh:mm:ss")
private Timestamp createdAt;
Copy link
Contributor

Choose a reason for hiding this comment

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

이 코드가 위 리뷰들 중 어떤 하나의 답이 될 수 있겠네요

Copy link
Contributor

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.

아 급하게하느라 기껏 추상클래스 만든거 간과했네요 수정하겠습니다

private LoginType loginType;

@NotNull
@Column(name = "fail_cnt", nullable = false)
Copy link
Contributor

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.

확인했습니다

Copy link
Contributor

@devmelonlee devmelonlee left a comment

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.

4 participants