Skip to content

Conversation

@parkjaehak
Copy link
Collaborator

@parkjaehak parkjaehak commented Jan 26, 2025

📄 요약(Summary)

작업 승인 처리

✍🏼 상세(More)

PR Desciption

변경 사항 설명

  • reviewer인 경우에 대해 작업 승인 가능하도록 처리
  • approveTask시 검토자가 수정한 속성반영을 위하여 reviewer, processor, dueDate, category, mainCategory, label에 대한 연관관계 설정

Requirements for Reviewer

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

  • @starboxxxx ApplicationEventListener의 경우 CreateTaskService에 대한 부분 참고해서 수정하면 될까요?
  • 추가로 CreateTaskResponse로 변경하였으니 reabse할때 참고해주시면 좋을 것 같습니다.

PR Log

PR 작업하면서 고민했던 내용, 해결한 내용, 고민 중인 내용 등

새롭게 배운 것

고민 중인 사항

첨부 자료

Requirements for Reviewer

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

✅ 체크리스트(Checklist)

  • PR 양식에 맞게 작성했습니다
  • 모든 테스트가 통과했습니다
  • 프로그램이 정상적으로 작동합니다
  • 적절한 PR 라벨을 설정했습니다
  • 불필요한 코드를 제거했습니다

🚪 이슈 번호(Issue numbers)

Closes #85

@parkjaehak parkjaehak added ✨ feature 구현·개선 사항에 관련된 내용입니다 HIGH 우선순위 상 labels Jan 26, 2025
@parkjaehak parkjaehak self-assigned this Jan 26, 2025
@starboxxxx
Copy link
Collaborator

ApplicationListener의 경우는 작업 승인, 담당자 변경 등의 메소드만 만들어놔주시면 제가 별도로 설정할 예정입니다!! 상관안쓰셔두 돼요!

Copy link
Contributor

@joowojr joowojr 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

Choose a reason for hiding this comment

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

taskid는 dto 선언이 아닌 path variable로 변경 부탁드립니다!

@NotNull
Long mainCategoryId,
@NotNull
Long categoryId,
Copy link
Contributor

Choose a reason for hiding this comment

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

카테고리 id(2차)만 받아도 충분할 거 같은데 어떻게 생각하시나요?

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
Contributor

Choose a reason for hiding this comment

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

string이 아니라 데이터 객체로 선언하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

정렬하는 속성과 그 속성을 오름차순 혹은 내림차순 정렬을 할지에 대한 방향은 항상 함께 전달되기 때문에 객체로 묶었습니다.

@Transactional
public UpdateTaskResponse updateTask(Long requesterId, UpdateTaskRequest updateTaskRequest) {
memberService.findActiveMember(requesterId);
Category category = categoryService.findById(updateTaskRequest.categoryId());
Copy link
Contributor

Choose a reason for hiding this comment

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

승인 권한이 있는 담당자만 가능하도록 해야해서, SPEL을 사용하여 메서드 단위로 승인을 하거나 MEMBER의 리뷰어 권한을 조회해서 예외 처리 추가하시면 될거같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 이해한게 맞다면 말씀하신 부분은 approvalTaskByReviewer()메서드에 추가되어있는데 확인한번 부탁드립니다.

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

@joowojr joowojr Jan 26, 2025

Choose a reason for hiding this comment

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

수정시에도 요청자나 담당자와 일치하는지 검증하는거 필요할거같아요!

@parkjaehak parkjaehak force-pushed the CLAP-119 branch 2 times, most recently from 66b96dd to 582ca7d Compare January 26, 2025 06:50
@parkjaehak parkjaehak merged commit be511ef into develop Jan 26, 2025
1 check passed
@joowojr joowojr deleted the CLAP-119 branch January 26, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feature 구현·개선 사항에 관련된 내용입니다 HIGH 우선순위 상

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLAP-119 작업 요청 승인

4 participants