-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feature/hyeyeon step1 #8
base: base/hyeyeon
Are you sure you want to change the base?
Conversation
- todo 도메인 - controller, entity, repository, service 패키지 추가 - user 도메인 - controller, entity, repository, service 패키지 추가
- 각 Entity간의 연관관계를 정의하고 연결 - Todo일정을 등록할 때, startDay와 endDay간의 시간을 비교하여 검증하도록 Validator 구현 - 각 Entity에 대해 기본적인 생성기능 구현
- target type에 따라 구분하여 날짜 선후관계 검증
- jwt, spring security를 활용한 사용자 인증
- 생성, 수정, 삭제기능 구현
- setTodoName -> editTodoName형식으로 api 의미를 분명히 하기 위해 수정
- @EnableJpaAudting 어노테이션 추가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
몸풀기적인 Step이라, 간단하게 리뷰해보았습니다~
고민은 다음 스텝부터 본격적으로 ㅎㅎ;;;;;
implementation 'org.springframework.boot:spring-boot-starter-security' | ||
|
||
// JWT | ||
implementation group: 'io.jsonwebtoken', name: 'jjwt-api', version: '0.11.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
형식을 위쪽이랑 똑같이 작성하면 좋겠네요.
implementation 'io.jsonwebtoken:jjwt-api:0.11.2'
@RequiredArgsConstructor | ||
public class CalendarEntityController { | ||
|
||
private final CalendarService calendarService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공백이 두칸이네요
|
||
public record CreateCalendarRequest(Long userId, String calendarName) { | ||
|
||
public static CreateCalendarRequest create (Long userId, String calendarName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이름 alias는 좋지만, 일일히 만들기엔 (특히 dto의 수가 많아진다면) 조금 불편할 것 같긴 하네요.
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
|
||
@AllArgsConstructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RequiredArgsConstructor
|
||
import java.io.IOException; | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
|
||
@Transactional | ||
public Todo getTodoEntity(CalendarEntity calendarEntity, Long todoId) { | ||
List<Todo> todos = todoRepository.findAllByCalendarEntity(calendarEntity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read만 하고 있다면, @Transactional(ReadOnly = true)
로 바꿔주세요.
* @param calendarId | ||
* @param todoId | ||
*/ | ||
public void deleteTodo(Long userId, Long calendarId, Long todoId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오히려 @Transactional
은 이쪽이 더 필요하겠네요.
todo.editEndTime(endTime); | ||
todo.editTodoMemo(todoMemo); | ||
todo.editAllDayFlag(allDayFlag); | ||
todo.editRepeatFlag(repeatFlag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit를 했는데 persist/save를 하진 않았네요?
@Column(name = "todo_id") | ||
private Long id; | ||
|
||
@Size(max=50) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 글자수 제한이 있지만, 입력값에 대한 Validation은 없던 것 같아요.
|
||
public static CreateTodoRequest create( | ||
Long userId, Long calendarId, | ||
String todoName, Date startDay, Time startTime, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- LocalDateTime 이던, OffsetDateTime 이던, 구체적인 시간 클래스를 받는게 더 나을 것 같습니다. (굳이 Day와 Time을 분리할 필요도 없어보여요.)
- userId, calendarId가 non-Null임을 보장해야 할 필요가 있어보입니다.
DB 테이블
사용자 관리
Validation 구현
알림설정
Todo Entity 수정
테스트 코드 작성 관련