[✨ Feature] S-05 회고 작성 - DB 스키마 구현 - #24
Conversation
summaryStatus enum(ANALYZING/COMPLETED/FAILED), userId+retrospectDate 유니크 인덱스 반영. User와 1:N 관계 연결, 스키마 생성 시 FK 비활성화 설정 추가 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Retrospect와 N:1 관계 연결(retrospectId), 각 필드 설명을 @Property comment로 반영. 데이터 스펙 문서도 변경된 필드(activityAt, retrospectId)에 맞춰 갱신 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Retrospect와 N:1 관계 연결, 각 필드 설명을 @Property comment로 반영 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
51f2b15 to
253d8d8
Compare
BaseEnum 값 객체 패턴 + MikroORM Type을 만들어주는 createCustomType 헬퍼로 enum-DB 컬럼 매핑을 통일. RetrospectSummaryStatusEnum, GitActivityTypeEnum 을 이 구조로 정리.
GitActivity/RetrospectQuestion에서 관계와 같은 컬럼을 가리키는 별도 스칼라 FK 프로퍼티(userId/retrospectId)를 제거 — 부트스트랩 시 MetadataError(중복 fieldName) 발생하던 문제. createCustomType의 convertToDatabaseValue/convertToJSValue에 null 가드 추가 — nullable enum 컬럼 flush 시 크래시하던 문제. mikro-orm.config.ts에 namingStrategy 명시, FK 비활성화 사유 주석 추가.
private 생성자에서 summaryStatus를 항상 ANALYZING으로 세팅해두니 MikroORM discovery가 이를 컬럼 DEFAULT로 오추론해 스키마 생성이 깨짐(quote 안 된 값). 생성자 대입 제거로 추론 트리거 자체를 없앰. GitActivity의 user/retrospect 관계에 lazy 옵션 추가.
GitActivity → Retrospect 방향 ManyToOne에 걸려있던 lazy를 제거하고, 컬렉션을 들고 있는 Retrospect.gitActivities(OneToMany)로 옮김.
juice119
left a comment
There was a problem hiding this comment.
S-05 회고 작성 작업 전 DB 작업 입니다. 리뷰 부탁드려요
| @@ -0,0 +1,30 @@ | |||
| export abstract class BaseEnum<_T extends BaseEnum<_T>> { | |||
There was a problem hiding this comment.
TS Enum에 메타데이터 확장 불가, 빌드 시 크기등에 문제를 해결하기 위해서, java enum을 소스코드를 참조해서 enum class를 직접 구현했습니다.
관련 내용은 아래 블로그 글로 정리했습니다.
TS enum을 분석하고 Java Enum 방식을 직접 구현하기
There was a problem hiding this comment.
일단 좋은 분석 자료이고, 좋은 해결 방안이라고 생각해요.
그러나 저는 조금 다른 생각을 가지고 있는데요.. (취향 차이라고 생각하고, 그냥 제 생각을 공유 드리는 comment 입니다. juice119 님의 생각도 정답이라고 생각합니다.)
데이터만 담을 수 있지 isPaid() 같은 도메인 메서드를 심기는 어려웠다.
Java 는 개발해보면 정말 객체 지향적으로 코드를 작성할 수 있고, 많은 오픈 소스와 같이 협업한 많은 개발자들이 객체 지향적으로 코드를 작성했어요. 그러나 Javascript 는 그렇지 않더라구요. 예를 들어 PrismaORM 을 사용해보면 단순히 object 형태로 database 의 entity 를 던져 줍니다. 해당 entity 는 클래스가 아닌 그냥 object 이기 때문에 말씀 하신 부분과 동일하게 isPaid 함수를 심을 수 없어요. 그런데 해당 PrismaORM 은 매우 많이 쓰이는 라이브러리 입니다. 제가 개인적으로 느낀바는 Javascript 생태계는 객체 지향에 너무 집착하지 않았어요. 뭔가 프리스타일대로 작성되는게 많았어요. isPaid 같은 것을 사용하는 상황이 있다면 저는 그것을 Enum class 에 넣지 않고 비즈니스 서비스 로직에 포함시켜서 작성하고 있어요.
| @Property({ | ||
| type: 'varchar', | ||
| length: 255, | ||
| fieldName: 'git_id', |
There was a problem hiding this comment.
변수들이 camel case (e.g. repoName) snake case (e.g. git_id) 로 섞여 있는데, 이렇게 따로 관리하는건 어떤 이유인가요? naming 일관성 측면에서는 좋지 않아 보여서요.
| length: 255, | ||
| fieldName: 'git_id', | ||
| unique: true, | ||
| comment: 'git 기준 id', |
There was a problem hiding this comment.
(중요하지 않아요) 좀 더 직관적으로 하면 이해가 잘 될 것 같아요. githubNodeId 라고 하면 더 직관적일 것 같아요. 참고로 조사해 보니 gitlab 에서는 id, iid 라는 용어들을 사용하고 있고, bitbucket 은 id, uuid 등을 사용합니다.
git_id 라는 변수는 이러한 git service id 인가 라는 생각을 하게 만들었어요.
| @OneToMany( | ||
| () => GitActivity, | ||
| (gitActivity) => gitActivity.retrospect, | ||
| { lazy: true }, |
There was a problem hiding this comment.
(또한번 제가 micro orm 은 잘 모르지만..) lazy 를 명시한 이유가 있나요? default 인 populate 형태로 사용해도 될 것 같아서요. 다른 entity 의 many 관계에서는 대부분 populate 형태로 선언하셨는데, 이 경우만 lazy 를 적용한 이유가 궁금합니다.
| @Property({ type: 'int', comment: '질문 순서 (1~3)' }) | ||
| order: number; |
There was a problem hiding this comment.
실무적인 관점에서 order 변수는 피해야 합니다. query 작성시 ORDER BY order 이런식으로 쓰면 읽기, 쓰기에 불편함이 있거든요. 추가적으로 data 관점에서 order 값이 중복되지 않게 unique 로 묶으면 좋을 것 같아요.
| @@ -0,0 +1,30 @@ | |||
| export abstract class BaseEnum<_T extends BaseEnum<_T>> { | |||
There was a problem hiding this comment.
일단 좋은 분석 자료이고, 좋은 해결 방안이라고 생각해요.
그러나 저는 조금 다른 생각을 가지고 있는데요.. (취향 차이라고 생각하고, 그냥 제 생각을 공유 드리는 comment 입니다. juice119 님의 생각도 정답이라고 생각합니다.)
데이터만 담을 수 있지 isPaid() 같은 도메인 메서드를 심기는 어려웠다.
Java 는 개발해보면 정말 객체 지향적으로 코드를 작성할 수 있고, 많은 오픈 소스와 같이 협업한 많은 개발자들이 객체 지향적으로 코드를 작성했어요. 그러나 Javascript 는 그렇지 않더라구요. 예를 들어 PrismaORM 을 사용해보면 단순히 object 형태로 database 의 entity 를 던져 줍니다. 해당 entity 는 클래스가 아닌 그냥 object 이기 때문에 말씀 하신 부분과 동일하게 isPaid 함수를 심을 수 없어요. 그런데 해당 PrismaORM 은 매우 많이 쓰이는 라이브러리 입니다. 제가 개인적으로 느낀바는 Javascript 생태계는 객체 지향에 너무 집착하지 않았어요. 뭔가 프리스타일대로 작성되는게 많았어요. isPaid 같은 것을 사용하는 상황이 있다면 저는 그것을 Enum class 에 넣지 않고 비즈니스 서비스 로직에 포함시켜서 작성하고 있어요.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
juice119
left a comment
There was a problem hiding this comment.
리뷰 내용 반영했습니다. 확인 부탁드려요!
| answerText: string; | ||
|
|
||
| @Property({ type: 'int', comment: '질문 순서 (1~3)' }) | ||
| sortOrder: number; |
There was a problem hiding this comment.
order -> sortOrder로 변경했습니다.
There was a problem hiding this comment.
sortOrder 는 조금 덜 직관적인거 같아요. comment 로 남겨주신 설명 또한 정렬 순서가 아니라 질문 순서 입니다.
questionOrder 변수명이 적절한 것 같습니다.
코드를 작성하실때 comment 없이 코드 그 자체에서 설명이 되도록 작성되는게 좋아요.
| @Entity({ tableName: 'retrospect_questions' }) | ||
| @Unique({ | ||
| name: 'uk_retrospect_question_sort_order', | ||
| properties: ['sortOrder', 'retrospect'], |
There was a problem hiding this comment.
말씀하신대로 sortOrder, retrospect 로 유니크 조건 추가했어
There was a problem hiding this comment.
AI 가 쓴걸까요? ㅎㅎ;
향후 쓰이는 쿼리를 예상했을때 properties 순서가 반대로 (retrospect, sortOrder) 가는게 적절할 것 같습니다.
| comment: | ||
| '회고 요약 상태 (ANALYZING: 분석중, COMPLETED: 분석완료, FAILED: 분석실패)', | ||
| }) | ||
| summaryStatus: RetrospectSummaryStatusEnum; |
There was a problem hiding this comment.
nullabe을 제거하고 DRAFT사용자가 작성중인 상태를 추가했습니다.
There was a problem hiding this comment.
summaryStatus: RetrospectSummaryStatusEnum = RetrospectSummaryStatusEnum.DRAFT;
이렇게 해야 할것 같아요~
| length: 255, | ||
| fieldName: 'git_id', | ||
| unique: true, | ||
| comment: 'git 기준 id', |
| length: 255, | ||
| fieldName: 'git_id', | ||
| unique: true, | ||
| comment: 'git 기준 id', |
There was a problem hiding this comment.
(중요하지 않아요) 좀 더 직관적으로 하면 이해가 잘 될 것 같아요. githubNodeId 라고 하면 더 직관적일 것 같아요. 참고로 조사해 보니 gitlab 에서는 id, iid 라는 용어들을 사용하고 있고, bitbucket 은 id, uuid 등을 사용합니다.
git_id 라는 변수는 이러한 git service id 인가 라는 생각을 하게 만들었어요.
| comment: | ||
| '회고 요약 상태 (ANALYZING: 분석중, COMPLETED: 분석완료, FAILED: 분석실패)', | ||
| }) | ||
| summaryStatus: RetrospectSummaryStatusEnum; |
There was a problem hiding this comment.
summaryStatus: RetrospectSummaryStatusEnum = RetrospectSummaryStatusEnum.DRAFT;
이렇게 해야 할것 같아요~
| length: 100, | ||
| comment: 'AI 생성 제목 (분석 완료 시, FT-08)', | ||
| }) | ||
| title: string; |
There was a problem hiding this comment.
이런 부분들도 nullable 이 빠지면 아래 처럼 되어야 하지 않아요?
title: string = '';
| @Entity({ tableName: 'retrospect_questions' }) | ||
| @Unique({ | ||
| name: 'uk_retrospect_question_sort_order', | ||
| properties: ['sortOrder', 'retrospect'], |
There was a problem hiding this comment.
AI 가 쓴걸까요? ㅎㅎ;
향후 쓰이는 쿼리를 예상했을때 properties 순서가 반대로 (retrospect, sortOrder) 가는게 적절할 것 같습니다.
| answerText: string; | ||
|
|
||
| @Property({ type: 'int', comment: '질문 순서 (1~3)' }) | ||
| sortOrder: number; |
There was a problem hiding this comment.
sortOrder 는 조금 덜 직관적인거 같아요. comment 로 남겨주신 설명 또한 정렬 순서가 아니라 질문 순서 입니다.
questionOrder 변수명이 적절한 것 같습니다.
코드를 작성하실때 comment 없이 코드 그 자체에서 설명이 되도록 작성되는게 좋아요.




배경 및 목적
연관 이슈
Closes #23
작업 내용
Retrospect엔티티 추가 (summaryStatus enum, userId+retrospectDate 유니크 인덱스)GitActivity엔티티 추가 (Retrospect와 N:1 관계)RetrospectQuestion엔티티 추가 (Retrospect와 N:1 관계)@Propertycomment로 반영,docs/planning/05-data-api-spec.md와 동기화ERD
테스트 결과
nest build api성공 확인리뷰어 참고사항
java enum을 참조해서 직접 구현했습니다.