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

[FIX] 키워드 조회 API 로직 수정 #103

Merged
merged 8 commits into from
Aug 17, 2023

Conversation

yeseul106
Copy link
Member

📝 Summary

  • 키워드 목록을 조회하는 API 로직을 수정했습니다.

👩‍💻 Contents

  • 기획 측과 논의 결과, 키워드 목록을 조회할 때 업무 분류를 먼저 하고 해당 업무 아래의 모든 키워드 목록을 불러오는 것으로 로직을 수정했습니다.
  • 클라분들의 요청으로, 키워드 색상도 서버가 저장하고 내려주어야 해서 키워드 엔티티에 color 컬럼을 추가했습니다.
  • 업무 카드를 생성 시, request body로 받는 keywordList를 처리하는 로직이 누락되어 있어서 해당 로직도 추가했습니다.

📝 Review Note

📣 Related Issue

📬 Reference

@yeseul106 yeseul106 added 🐼 예슬 🐼 🐞Fix🐞 에러 발생 시, 버그 labels Aug 16, 2023
@yeseul106 yeseul106 self-assigned this Aug 16, 2023
Copy link
Member

@Seokyeong237 Seokyeong237 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
Contributor

@unanchoi unanchoi left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

리뷰 확인 부탁드립니다

Comment on lines +75 to +80
public void deleteTask(Long taskId) {
Task findTask = taskRepository.findByIdOrThrow(taskId);

findTask.deleted();
findTask.getSubTasks().forEach(this::deleteSubTaskAndCard);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p3;

@Transactional이 빠져있는 것 같습니다!

taskRepository.save(Task.builder()
.name(requestDto.getName())
.category(findCategory)
.isDeleted(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;

false는 Service에서 값으로 넣는 것 보다 entity 생성자에 들어가는 것이 조금 더 좋을 것 같네요!

private final SubTaskRepository subTaskRepository;

@Override
@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

p2;

readOnly = true option을 넣어주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

확인입니다 !

Comment on lines 38 to 39
@Column(name = "is_deleted", nullable = false)
private boolean isDeleted;
private Boolean isDeleted;
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;

Wrapper class로 바꾼 이유가 궁금합니다! 기본적으로 Entity boolean값은 원시 타입으로 해두는 것이 좋다고 생각하는데 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

어라,,,? 뭐지 바꾼 기억이 없는데,,,ㅎ 다시 바꿔 놓겠습니다 !

Comment on lines 13 to 17
List<SubTask> findByTaskId(Long taskId);

default SubTask findByIdOrThrow(Long subTaskId) {
return findById(subTaskId).orElseThrow(
() -> new NotFoundException(ErrorCode.NOT_FOUND_SUB_TASK));
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;

repository마다 목록 조회 method 이름이 findAllfind로 다른데 통일해도 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

확인용 ~!

}

@Override
@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

p2;

readOnly = true를 넣어주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 ! 반영해서 올려놓겠습니다 !

private final CategoryRepository categoryRepository;

@Override
@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

p2;

readOnly = true를 넣어주세요!

Comment on lines 12 to 22
//* 업무 전체 목록 조회
List<TaskGetResponseDto> getAllTask(Long memberId);

//* 특정 카테고리 내 업무 목록 조회
List<TaskGetResponseDto> getByCategoryId(Long categoryId);

//* 업무 수정
void updateTaskName(Long taskId, TaskUpdateRequestDto taskUpdateRequestDto);

void createTaskName(TaskCreateRequestDto taskCreateRequestDto);
void deleteTask(Long taskId);
Copy link
Contributor

Choose a reason for hiding this comment

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

p4;

주석은 삭제하거나 통일하는 것이 좋을 것 같습니다!

@JoinColumn(name = "folder_id")
private Folder folder;

@Column(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.

p3;

원시타입이므로 nullable=false가 없어도 괜찮을 것 같아요!

Copy link
Contributor

@unanchoi unanchoi left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

@yeseul106 yeseul106 merged commit ba6a669 into develop Aug 17, 2023
1 check passed
@unanchoi unanchoi deleted the fix/#102-keyword-list-get-api branch August 18, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞Fix🐞 에러 발생 시, 버그 🐼 예슬 🐼
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FIX] 키워드 목록 조회 API 로직 수정
3 participants