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

[Feat] 태스크 위치 이동 API 구현 #53

Merged
merged 8 commits into from
Nov 11, 2024
Merged

[Feat] 태스크 위치 이동 API 구현 #53

merged 8 commits into from
Nov 11, 2024

Conversation

yangchef1
Copy link
Collaborator

@yangchef1 yangchef1 commented Nov 11, 2024

관련 이슈 번호

close #18

작업 내용

  • LexoRank 알고리즘에 맞춰 기존 생성 API 수정
  • 태스크 위치 이동 API 구현

고민과 학습내용

  1. 기본 Section 값 (No Status, To Do, In Progress, Done)
  • 현재는 프로젝트 생성 시, 기본 Section 로직이 없어 Section 관련 오류가 발생합니다.
  • 아래 스크린샷은 Section을 주석처리하고 실행한 결과입니다.
  • 프로젝트 부분의 기본 Section 생성 로직이 완성되면 추가 반영할 예정입니다.
  1. LexoRank
    LexoRank는 라이브러리를 사용하여 구현했습니다.
    라이브러리의 기능에 대해서는 아래 자료를 참고했습니다.

Reference

https://www.npmjs.com/package/lexorank

스크린샷

image

@yangchef1 yangchef1 added Feature 새로운 기능을 추가 BE 서버 작업 labels Nov 11, 2024
@yangchef1 yangchef1 self-assigned this Nov 11, 2024
@PMtHk
Copy link
Collaborator

PMtHk commented Nov 11, 2024


혹시 브랜치 생성에 어떤 문제가 있으신가요?
어제와 똑같이 커밋이 중복되는 문제가 있어보입니다.

Copy link
Collaborator

@PMtHk PMtHk left a comment

Choose a reason for hiding this comment

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

LexoRank 까지 빠르게 도입하셨군요!

return new UpdateTaskResponse(task);
}

async move(id: number, moveTaskRequest: MoveTaskRequest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡
updatemove 사이에는 많은 공통 로직이 있어보입니다.
update 를 더 확장성을 고려해서 만들고 그 로직을 활용하는 방향으로 move 를 구현하는 것이 좋을 것 같아요!

그동안 마스터님들의 말슴은
보통 세번 반복되면 리팩토링한다는 것이긴 합니다.

한번 더 겹치면 고려해주세용~

Copy link
Collaborator Author

@yangchef1 yangchef1 Nov 11, 2024

Choose a reason for hiding this comment

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

updatemove는 별도의 스토리로 구성한 만큼, 서로 의존성을 갖지 않게 하고 싶어서 update를 move에서 활용하는 설계는 고려하지 않았던 것 같습니다.

대신에 ,, 사실 같은 코드가 반복되다 보니, 아래 2가지 부분을 별도 메서드로 분리할까 고민했었습니다.
아래 코드를 분리하면 update, move외에 다른 메서드에서도 재사용할 수 있을 것 같아서용 ,,

작성할 때는 분리할 정도의 코드는 아니라고 생각했었는데, 말씀 듣고 다시 한번 생각해보니 다른 생성, 조회, 삭제 메서드에서도 활용될 수 있는 재사용성 높은 코드라서 분리하는게 여러모로 좋아보이네요 ...!

const task = await this.taskRepository.findOneBy({ id });
if (!task) {
	throw new NotFoundException('Task not found');
}
const section = await this.sectionRepository.findOneBy({ id: updateTaskRequest.sectionId });
if (!section) {
	throw new NotFoundException('Section not found');
}

Copy link
Collaborator

@iam454 iam454 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 +65 to +67
const beforePostion = LexoRank.parse(moveTaskRequest.beforePosition);
const afterPosition = LexoRank.parse(moveTaskRequest.afterPosition);
task.position = beforePostion.between(afterPosition).toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

lexo rank 사용법이 굉장히 간단하고 좋네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞습니다 ㅋㅋㅋ
비슷한 기능의 메서드가 많아서 공부할 땐 좀 헷갈렸는데, 사용법 자체는 굉장히 간단하더라구요 !

@yangchef1
Copy link
Collaborator Author

오 그렇네요 같은 코드가 보이네
나중에 왜 이랬는지 경험 공유해주세요 헤헤

위에 주엽님 코멘트에 답글로 남겨놨습니당 힛

시간 나면 렉소랭크 직접 구현하죠

조씁니다 ! 재밌겠다 ...

@yangchef1 yangchef1 merged commit ea4f1e0 into dev Nov 11, 2024
2 checks passed
@PMtHk
Copy link
Collaborator

PMtHk commented Nov 11, 2024

진짜 재밌는 거 맞죠? ㅎㅎ

@yangchef1 yangchef1 deleted the feat/#18 branch December 3, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 서버 작업 Feature 새로운 기능을 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

태스크 위치 수정 API를 구현한다.
4 participants