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

AI 폴더 추천 고도화 및 요약 Coverage 개선 #106

Merged
merged 11 commits into from
Sep 29, 2024

Conversation

J-Hoplin
Copy link
Collaborator

@J-Hoplin J-Hoplin commented Sep 24, 2024

PR 내용

추가 및 변경 사항

API endpoint와 추가 혹은 변경된 사항을 적어주세요!

  • 마이그레이션(완료상태)

    • Folder Collection, visible 필드
  • URL Validator 수정

  • Puppeteer Pool 연결

    • Coverage 증가 목적
    • 우선 NCloud에 임시로 배포해둔 상태
    • 배포코드
  • 고도화 기획 사항

AI 폴더 추천 개선 사항

목표: 클라이언트에서 컴포넌트 변경 없이 프로세스 변경 진행

< DB 변경 사항 >
- Folder에 `visible` 필드 추가: 사용자가 볼 수 있는 필드인지 여부, Migration Script 추가, 기존 데이터들 visible = true로
- Folder 보여줄 때 Visible한 폴더들만 보여주도록 변경


< 프로세스 >
1. 사용자 폴더 + 기존 온보딩 폴더 합쳐서 분류
2. 만약 사용자 폴더가 아닌 경우에는 폴더 추가(visible=false)
3. AI Classification 줄때는 visible true, false 모두 보여준 상태로
4. 만약 사용자가 visible false인 폴더에 대해 모두 이동을 했을때 visible=true로 변경

PR 중점사항

리뷰어가 중점적으로 봐야하는 부분에 대해 적어주세요!

스크린샷

@github-actions github-actions bot added document 문서화 관련 작업 수정 및 생성 feature labels Sep 24, 2024
migrations/01_folder_visible.migration.ts Outdated Show resolved Hide resolved
src/modules/ai-classification/ai-classification.service.ts Outdated Show resolved Hide resolved
src/infrastructure/ai/ai.service.ts Outdated Show resolved Hide resolved
src/modules/ai-classification/ai-classification.service.ts Outdated Show resolved Hide resolved
src/modules/classification/classification.repository.ts Outdated Show resolved Hide resolved
@JonghunAn
Copy link
Member

저거 폴더 생성했을 때 폴더 개수랑 폴더목록 조회할때 쿼리 조건값에 visible 판단 쿼리 추가되야하지않아?
임의의 aiGenerated 하나 만들어 준다고 했는데 그거 포함될 것 같은데...

@J-Hoplin
Copy link
Collaborator Author

J-Hoplin commented Sep 24, 2024

@JonghunAn 음 우선 폴더 API에서 폴더 List 하는 API 하나밖에 영향범위가 없어서 해당 부분은 적용을 해둔 상태고(폴더 Count 하는 부분은 우선 없어), AiClassification 부분에서는 visible 여부 상관 없이 모두 보여줘야하는 상황이라 classification list API에서 visible 필터 안걸어서 가져오도록 해두었어
image

Copy link
Collaborator

@hye-on hye-on left a comment

Choose a reason for hiding this comment

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

LGTM!! 👍 넘 고생했어!!

@@ -32,13 +36,60 @@ export class AiClassificationService {

// NOTE: AI 요약 요청
const start = process.hrtime();
if (payload.postContent.length < CONTENT_LEAST_LIMIT) {
try {
const puppeteerURL = this.config.get<string>('PUPPETEER_POOL_URL');
Copy link
Collaborator

Choose a reason for hiding this comment

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

puppeteer 쪽 로직이 별도의 함수나 service로 있으면 어떨까?!
ai classification execute함수에 컨텍스트가 많아서 함수 분리 정도는 해두면 어떨까 하는 생각이 드넹

content정도만 return 해서 본 함수에서 사용하는 식으로?

Copy link
Collaborator Author

@J-Hoplin J-Hoplin Sep 27, 2024

Choose a reason for hiding this comment

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

아무래도 본래 컨텍스트 목적이랑 달라서 형 말대로 분리하는게 더 나을것같당 외부 인프라 통신이기 때문에 infrastructurepuppeteer-pool로 모듈 분리해뒀어! 적용완료!

@@ -76,4 +87,14 @@ export class FolderRepository {
});
return folders;
}

async makeFoldersVisible(folderId: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

makeFoldersVisible인데 folderId는 단수당

@@ -19,6 +20,7 @@ import { AiClassificationService } from './modules/ai-classification/ai-classifi
DatabaseModule,
DiscordModule,
AiModule,
FoldersModule,
Copy link
Collaborator

Choose a reason for hiding this comment

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

요건 다시 지우면 안되는 모듈이얌?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

삭제해도 괜찮아!

@J-Hoplin J-Hoplin merged commit 8f4c89e into develop Sep 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document 문서화 관련 작업 수정 및 생성 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants