-
Notifications
You must be signed in to change notification settings - Fork 0
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
#13 Feature: 일정 관련 로직 구현 #14
Changes from 26 commits
0e6c242
b3987e9
0221646
22d7352
86f55b9
de5882a
13516d7
7fc76e9
b42e6bf
d6cf99e
c88f41b
11bc21e
780f0e0
beb3e22
88be70c
a16a8b9
701e41c
e1f57ec
d775ff8
9605432
ed56c78
bc41e21
83f771d
9e45d04
4205924
d9678bd
1128456
2a80e14
d13e44c
3803684
1f87e02
1827771
68b3016
2890e83
7913dd3
673e01e
d873daf
bca5166
999aafd
3386392
f057d03
66c8e2c
f852c6a
5d4da54
bc9f26a
a4c5411
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
- 제목 : #이슈번호 feat: 기능명 | ||
ex) #15 feat: pull request template 작성 | ||
(확인 후 지워주세요) | ||
|
||
## 1. 개발내용 | ||
|
||
## 2. 구현 세부사항 | ||
|
||
## 3. 메모 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
package leets.weeth.domain.event.controller; | ||
|
||
import leets.weeth.domain.event.dto.RequestEvent; | ||
import leets.weeth.domain.event.dto.ResponseEvent; | ||
import leets.weeth.domain.event.service.EventService; | ||
import leets.weeth.global.common.response.CommonResponse; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.format.annotation.DateTimeFormat; | ||
import org.springframework.web.bind.annotation.*; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.List; | ||
|
||
@RestController | ||
@RequiredArgsConstructor | ||
@RequestMapping("/events") | ||
public class EventController { | ||
private final EventService eventService; | ||
|
||
// 일정 생성 | ||
@PostMapping("/create") | ||
public CommonResponse<String> createEvent(@RequestBody RequestEvent requestEvent) { | ||
eventService.createEvent(requestEvent); | ||
return CommonResponse.createSuccess("일정 생성 성공."); | ||
} | ||
|
||
// 일정 상세 조회 | ||
@GetMapping("/{id}") | ||
public CommonResponse<ResponseEvent> getEvent(@PathVariable Long id) { | ||
ResponseEvent responseEvent = eventService.getEventById(id); | ||
return CommonResponse.createSuccess(responseEvent); | ||
} | ||
|
||
// 기간 별 일정 조회 | ||
@GetMapping("") | ||
public CommonResponse<List<ResponseEvent>> getEvents( | ||
@RequestParam(name = "start") @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime startDate, | ||
@RequestParam(name = "end") @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime endDate) { | ||
Comment on lines
+48
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ISO.DATA_TIME 으로 맞춘 점이 좋네요! 프론트와 데이터 타입이 맞지 않는 경우가 많은데, 신경쓰신거 같아서 좋은것 같습니다:) |
||
|
||
List<ResponseEvent> responseEvents = eventService.getEventsBetweenDate(startDate, endDate); | ||
return CommonResponse.createSuccess(responseEvents); | ||
} | ||
|
||
// 일정 수정 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 궁금한 점이 있는데, 혹시 일정 수정은 아무나 할 수 있는 건가요? 아니면 인증인가 부분을 아직 구현 전이라서 그런걸까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 관리자만 수정할 수 있게 구현할 예정입니다! 아직 인증인가를 구현하지 않았습니다! |
||
@PatchMapping("/update/{id}") | ||
public CommonResponse<String> updateEvent(@PathVariable Long id, @RequestBody RequestEvent requestEvent) { | ||
eventService.updateEvent(id, requestEvent); | ||
return CommonResponse.createSuccess("id: " + id + " 일정 수정 성공"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기도 마찬가지로 ENUM을 사용하면 더 좋을것 같습니다! |
||
} | ||
|
||
// 일정 삭제 | ||
@DeleteMapping("/{id}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. create와 update 요청과는 다르게 왜 delete는 주소에 delete가 없는건가요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 참고해서 수정해보겠습니다 |
||
public CommonResponse<String> deleteEvent(@PathVariable Long id) { | ||
eventService.deleteEvent(id); | ||
return CommonResponse.createSuccess("id: " + id + " 일정 삭제 성공"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위와 같아요! |
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package leets.weeth.domain.event.dto; | ||
|
||
import lombok.Getter; | ||
import lombok.Setter; | ||
|
||
import java.time.LocalDateTime; | ||
|
||
@Getter | ||
@Setter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 빌더 패턴을 사용하면 추후 dto를 수정하더라도 더 쉽게 코드를 변경할 수 있을것 같아요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 EventMapper 인터페이스를 사용해 dto와 객체 간의 변환을 하고 있어 직접적으로 dto를 생성하여 사용하는 로직이 없는 상황이라서 빌더 패턴을 사용하지 않고 있습니다! dto 수정시 mapper가 자동으로 잡아서 변환할 수 있게 처리 중인데 빌더 패턴을 넣어야하는지 잘 모르겠습니다! 더 고민해보겠습니다! 감사합니다!_ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. record를 통해 DTO 클래스를 구현해준다면 더욱 좋을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 공부해보고 팀원들과 얘기해서 사용해보겠습니다! 감사합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dto 에 setter는 아래의 이유로 쓰지 않는다고 하네요. 대신 이렇게 많이 대신 쓰이기도 한다고 합니다. 꼭 필요한 어노테이션인지 확인하시면 좋을것 같아요:) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 양 쪽 DTO 모두 사용하지 않아서 setter는 제거하였습니다! |
||
public class RequestEvent { | ||
private String title; | ||
|
||
private String content; | ||
|
||
private String location; | ||
|
||
private LocalDateTime startDateTime; | ||
|
||
private LocalDateTime endDateTime; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package leets.weeth.domain.event.dto; | ||
|
||
import lombok.Getter; | ||
import lombok.Setter; | ||
|
||
import java.time.LocalDateTime; | ||
|
||
@Getter | ||
@Setter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 마찬가지로 빌더 패턴 생각해보시면 더 좋을거 같아요 |
||
public class ResponseEvent { | ||
private Long id; | ||
|
||
private String title; | ||
|
||
private String content; | ||
|
||
private String location; | ||
|
||
private LocalDateTime startDateTime; | ||
|
||
private LocalDateTime endDateTime; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package leets.weeth.domain.event.entity; | ||
|
||
import jakarta.persistence.*; | ||
import leets.weeth.global.common.entity.BaseEntity; | ||
import lombok.*; | ||
|
||
import java.time.LocalDateTime; | ||
|
||
@Entity | ||
@Getter | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
@AllArgsConstructor | ||
@Builder | ||
@ToString | ||
public class Event extends BaseEntity { | ||
|
||
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
@Column(name = "event_id") | ||
private Long id; | ||
|
||
private String title; | ||
|
||
private String content; | ||
|
||
private String location; | ||
|
||
private LocalDateTime startDateTime; | ||
|
||
private LocalDateTime endDateTime; | ||
|
||
// 일정 수정을 위한 메소드 | ||
public void update(String title, String content, String location, LocalDateTime startDateTime, LocalDateTime endDateTime) { | ||
this.title = title; | ||
this.content = content; | ||
this.location = location; | ||
this.startDateTime = startDateTime; | ||
this.endDateTime = endDateTime; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 수정 메서드라면 인자를 변수 하나하나 받는것 보다 Event 객체를 서로 주고받게 하는건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 수정사항을 dto로 받기 때문에 event 객체를 넘기지 않고 dto 객체를 인자로 넘겨 수정하는 방식으로 바꿔봤는데 이 방식은 어떤가요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. null 값이 들어오면 수정하지 않게 하기 위해서 다른 방식으로 구현해봤는데 한 번 확인 부탁드립니다! |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package leets.weeth.domain.event.mapper; | ||
|
||
import leets.weeth.domain.event.dto.RequestEvent; | ||
import leets.weeth.domain.event.dto.ResponseEvent; | ||
import leets.weeth.domain.event.entity.Event; | ||
import org.mapstruct.Mapper; | ||
import org.mapstruct.factory.Mappers; | ||
|
||
@Mapper(componentModel = "spring") | ||
public interface EventMapper { | ||
EventMapper INSTANCE = Mappers.getMapper(EventMapper.class); | ||
|
||
Event fromDto(RequestEvent requestEvent); | ||
|
||
ResponseEvent toDto(Event event); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package leets.weeth.domain.event.repository; | ||
|
||
import leets.weeth.domain.event.entity.Event; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
public interface EventRepository extends JpaRepository<Event, Long> { | ||
|
||
List<Event> findByStartDateTimeBetween(LocalDateTime startDate, LocalDateTime endDate); | ||
|
||
Optional<Event> findById(Long id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 몰랐습니다... |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
package leets.weeth.domain.event.service; | ||
|
||
import jakarta.persistence.EntityNotFoundException; | ||
import leets.weeth.domain.event.dto.RequestEvent; | ||
import leets.weeth.domain.event.dto.ResponseEvent; | ||
import leets.weeth.domain.event.entity.Event; | ||
import leets.weeth.domain.event.mapper.EventMapper; | ||
import leets.weeth.domain.event.repository.EventRepository; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class EventService { | ||
private final EventRepository eventRepository; | ||
|
||
private final EventMapper eventMapper; | ||
|
||
// 일정 생성 | ||
@Transactional | ||
public void createEvent(RequestEvent requestEvent) { | ||
Event event = eventMapper.fromDto(requestEvent); | ||
eventRepository.save(event); | ||
} | ||
|
||
// 일정 상세 조회 | ||
@Transactional(readOnly = true) | ||
public ResponseEvent getEventById(Long id) { | ||
Event event = eventRepository.findById(id) | ||
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 레포지토리에 달았던 코멘트와 같이 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이것도 몰랐습니다.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @transactional(readOnly = true)는 명시적으로 표시하는 게 좋을 거 같아서 성능상 차이가 없다며 남겨두겠습니다! |
||
.orElseThrow(() -> new EntityNotFoundException("id에 해당하는 일정이 존재하지 않습니다.")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 에러 메시지를 enum으로 관리하거나 커스텀 exception 안에 Message를 고정해두는 방식이 관리하기 더 좋을것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 생각하기엔 커스텀 exception 안에 메시지를 고정하면 다른 도메인에서 사용시 에러 메시지가 명시적으로 전달되기 어려울 거 같아서 도메인 별로 에러 메시지를 enum으로 관리하는 것이 나을 거 같습니다! 해당 방식으로 수정하겠습니다! 다른 방법이 있다면 알려주시면 감사하겠습니다!! |
||
return eventMapper.toDto(event); | ||
} | ||
|
||
// 기간 별 일정 조회 | ||
@Transactional(readOnly = true) | ||
public List<ResponseEvent> getEventsBetweenDate(LocalDateTime startDate, LocalDateTime endDate) { | ||
List<Event> events = eventRepository.findByStartDateTimeBetween(startDate, endDate); | ||
return events.stream() | ||
.map(eventMapper::toDto) | ||
.collect(Collectors.toList()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 데이터의 불변성을 지키기 위해 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 불변성을 위해서 그렇게 하는게 나을 거 같습니다! 수정하겠습니다 |
||
} | ||
|
||
// 일정 수정 | ||
@Transactional | ||
public void updateEvent(Long id, RequestEvent requestEvent) { | ||
Event oldEvent = eventRepository.findById(id) | ||
.orElseThrow(() -> new EntityNotFoundException("id에 해당하는 일정이 존재하지 않습니다.")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위 코멘트와 마찬가지로 enum으로 관리하면 더 좋을거 같아요 |
||
|
||
oldEvent.update( | ||
requestEvent.getTitle(), | ||
requestEvent.getContent(), | ||
requestEvent.getLocation(), | ||
requestEvent.getStartDateTime(), | ||
requestEvent.getEndDateTime() | ||
); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분은 왜 MapStruct을 사용하지 않는건지 알수 있을까요? 그래서 저같으면, mapstruct을 이미 사용하고 계시기 때문에 request값이 null이 아닌 부분만 update를 할 수 있지 않을까 생각했습니다:) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아직 라이브러리에 대한 이해도가 없어서 수정 로직을 어떻게 바꿔야할지 고민 중입니다! 테스트 해보니 현재 방식으로는 null도 같이 업데이트 되고 있어서 더 연구해보고 수정하도록 하겠습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 찾아보니 MapStruct를 이용해 update 하는 방식은 Event Entity에 Setter를 모두 열어놔야하는 방식이라서 다른 방식으로 구현해봤는데 한 번 확인해주실 수 있으실까요?? |
||
|
||
// 일정 삭제 | ||
public void deleteEvent(Long id) { | ||
if (!eventRepository.existsById(id)) { | ||
throw new EntityNotFoundException("id에 해당하는 일정이 존재하지 않습니다."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기도요! |
||
} | ||
eventRepository.deleteById(id); | ||
} | ||
} |
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.
String 보다는 enum으로 상태관리를 하는건 어떨까요?
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.
문자열로 반환하는 부분은 전부 enum으로 변경해서 관리하겠습니다!