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

Issue/168/update best review #169

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"@nestjs/jwt": "^10.2.0",
"@nestjs/passport": "^10.0.3",
"@nestjs/platform-express": "^10.4.15",
"@nestjs/schedule": "^4.1.2",
"@nestjs/swagger": "^8.1.0",
"@prisma/client": "^6.1.0",
"@slack/web-api": "^7.8.0",
Expand Down Expand Up @@ -142,5 +143,6 @@
},
"overrides": {
"glob": "^9"
}
},
"packageManager": "[email protected]+sha512.1acb565e6193efbebda772702950469150cf12bcc764262e7587e71d19dc98a423dff9536e57ea44c49bdf790ff694e83c27be5faa23d67e0c033b583be4bfcf"
}
11 changes: 11 additions & 0 deletions src/common/entities/EReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ export namespace EReview {
});
export type Details = Prisma.review_reviewGetPayload<typeof Details>;

export const WithLectures =
Prisma.validator<Prisma.review_reviewDefaultArgs>()({
include: {
lecture: ELecture.Basic,
},
});

export type WithLectures = Prisma.review_reviewGetPayload<
typeof WithLectures
>;

export namespace EReviewVote {
export const Basic =
Prisma.validator<Prisma.review_reviewvoteDefaultArgs>()({});

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

  1. 코드 가독성: 새로운 WithLectures 상수와 타입이 추가되었습니다. 전반적으로 코드가 잘 정리되어 있어 가독성이 좋습니다. 하지만 각 프로퍼티의 역할이나 사용 목적에 대한 주석을 추가하면 다른 개발자들이 이해하는 데 도움이 될 것입니다.

  2. 정의 및 사용 장소: WithLectures 상수와 타입이 EReview 네임스페이스에 잘 포함되어 있지만, 전체적인 구조에서 이 상수가 어디에서 사용될지 명확하지 않습니다. 만약 이 상수가 다른 모듈 또는 네임스페이스에서 사용될 가능성이 있다면, 예시나 문서를 추가해주는 것이 좋습니다.

  3. 버그 리스크: 현재 코드에는 명백한 버그가 있는 것 같지는 않습니다. 하지만 ELecture.Basic이 유효한지 확인해야 합니다. 만약 Basic이 정의되지 않은 상태에서 접근하게 되면 런타임 오류가 발생할 수 있습니다.

  4. 타입 안정성: WithLectures의 타입이 Prisma.review_reviewGetPayload를 기반으로 하고 있으므로 타입 안정성은 잘 유지되고 있는 것 같습니다. 그러나 언제든지 Prisma의 API가 변경될 수 있으니, 이러한 의존성에 유의해야 합니다.

  5. 테스트: 이 새로운 추가 기능에 대해 테스트가 필요한지 검토해야 합니다. WithLectures와 관련된 단위 테스트가 없다면, 나중에 버그가 발생할 가능성을 높일 수 있습니다.

  6. 추가적인 개선: 자주 사용하는 패턴에 대한 헬퍼 함수나 유틸리티를 도입하면 코드의 재사용성과 유지보수성이 향상될 수 있습니다.

전반적으로 코드는 명확하고 잘 작성되어 있지만, 위에 언급한 몇 가지 사항을 고려하면 코드의 품질과 안정성을 더 높일 수 있을 것입니다.

Expand Down
11 changes: 9 additions & 2 deletions src/modules/reviews/reviews.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import {
Query,
} from '@nestjs/common';
import { session_userprofile } from '@prisma/client';
import { ReviewProhibited } from '@src/common/decorators/prohibit-review.decorator';
import { GetUser } from 'src/common/decorators/get-user.decorator';
import { Public } from 'src/common/decorators/skip-auth.decorator';
import { IReview } from 'src/common/interfaces/IReview';
import { ReviewsService } from './reviews.service';
import { EReview } from '../../common/entities/EReview';
import { ReviewsService } from './reviews.service';
import EReviewVote = EReview.EReviewVote;
import { ReviewProhibited } from '@src/common/decorators/prohibit-review.decorator';

@Controller('api/reviews')
export class ReviewsController {
Expand Down Expand Up @@ -52,6 +52,13 @@ export class ReviewsController {
}
}

@Get('update-best-reviews')
async triggerUpdateBestReviews() {
console.log('Triggered manual update for Best Reviews');
await this.reviewsService.updateBestReviewsCron();
return { message: 'BestReviews update triggered successfully' };
}

@Public()
@Get(':reviewId')
async getReviewInstance(

Choose a reason for hiding this comment

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

코드 패치에 대한 리뷰는 다음과 같습니다:

  1. 가독성: 코드의 가독성이 좋습니다. import 문을 정리하고, 불필요한 중복을 제거하여 코드를 깔끔하게 유지했습니다.

  2. 중복 제거: ReviewProhibited 모듈의 import 순서가 변경된 것은 일관성을 유지하는 데 도움이 됩니다. 불필요한 중복을 제거했습니다.

  3. 메소드 추가: triggerUpdateBestReviews 메소드가 추가되었습니다. 이 메소드는 Best Reviews의 수동 업데이트를 트리거하며, 잘 작성된 것으로 보입니다. 하지만, 이 메소드에서 console.log를 남기는 것이 운영 환경에서는 로그가 남을 수 있으므로, 프로덕션 코드에는 로깅을 위한 적절한 방법(Promise와의 실패 처리 및 레벨링)을 사용하는 것이 좋습니다.

  4. 에러 처리: triggerUpdateBestReviews() 메소드에서 오류 발생 시 적절한 에러 핸들링을 구현하는 것이 좋습니다. 예를 들어, await this.reviewsService.updateBestReviewsCron(); 호출 시 실패할 경우 적절한 에러 메시지를 클라이언트에게 반환하도록 수정할 필요가 있습니다.

  5. 추가적인 유효성 검사: triggerUpdateBestReviews() 메소드에 접근을 제한하기 위해 권한 검사를 추가하는 것을 고려해볼 수 있습니다. 특정 사용자만 이 메소드를 호출할 수 있도록 설정하면 보안에 도움이 될 것입니다.

  6. 주석 추가: 새로운 메소드에 대한 주석을 추가하면 유지 보수에 도움이 될 것이며, 다른 개발자가 이해하는 데 수월할 것입니다. 메소드의 목적이나 사용 방법에 대한 간단한 설명을 덧붙이는 것이 좋습니다.

이와 같은 수정사항을 고려하면 코드의 품질과 안정성을 높일 수 있을 것입니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Gerbera3090 이렇다고 하네요. console.log는 지워주세용

Expand Down
69 changes: 67 additions & 2 deletions src/modules/reviews/reviews.service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Transactional } from '@nestjs-cls/transactional';
import { HttpException, HttpStatus, Injectable } from '@nestjs/common';
import { session_userprofile } from '@prisma/client';
import { Cron } from '@nestjs/schedule';
import { review_review, session_userprofile } from '@prisma/client';
import { IReview } from 'src/common/interfaces/IReview';
import { toJsonReview } from 'src/common/interfaces/serializer/review.serializer';
import { LectureRepository } from 'src/prisma/repositories/lecture.repository';
import { ReviewsRepository } from 'src/prisma/repositories/review.repository';
import { Transactional } from '@nestjs-cls/transactional';
import { EReview } from '../../common/entities/EReview';
import EReviewVote = EReview.EReviewVote;

Expand Down Expand Up @@ -174,4 +175,68 @@ export class ReviewsService {
): Promise<EReviewVote.Basic> {
return await this.reviewsRepository.upsertReviewVote(reviewId, user.id);
}

@Cron('0 0 * * *') // 매일 자정에 실행하는 Cron 작업 설정: 과거 update-best-reviews.py를 계승
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 이렇게 해놓으면 cron 도는거에요?

Copy link
Contributor

Choose a reason for hiding this comment

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

이거 서버에서 지금 실행되는거죠..? 저희가 서버가 여러개라서 이렇게 해놓으면 모든 서버에서 다 돌아갈거에요.
그래서 지금 secure-VM에서 크론을 돌고 있는거기도 하고요, 거기는 요청을 한 군데로만 보내니까요.

지금 상황은 그런데 저거 쓰는거는 긍정적인 방향인거 같긴 합니다.
저기 sync 모듈을 서버 안에서 처리하지 말고 아예

  1. 모노 레포 형식으로 만들기
  2. 필요한 prisma 타입 및 interface 공유하기
  3. cron 붙여서 secureVM에서 바로 DB로 밀어넣기

로 바꾸는게 좋을 거 같네요. 이 작업까지 한 번에 해줄래요 아니면 잘라서 진행할래요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아마 그러면 secure-vm에 cron에 열어뒀던 get요청 보내는 코드를 추가하면 될 것 같습니다. 지금 cron은 주석 해두고요.

async updateBestReviewsCron() {
function calculateKey(review: EReview.WithLectures): number {
const baseYear = new Date().getFullYear();
const lectureYear = review.lecture.year;
const yearDiff = baseYear - lectureYear > 0 ? baseYear - lectureYear : 0;
return Math.floor(
(review.like / (review.lecture.audience + 1)) *
Math.pow(0.85, yearDiff),
);
}

function getBestReviews(
reviews: EReview.WithLectures[],
minLikedCount: number,
maxResultCount: number,
): review_review[] {
const likedCount = Math.max(
minLikedCount,
Math.floor(reviews.length / 10),
);
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

Choose a reason for hiding this comment

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

저 calculateKey 함수는 어떤건가요...? 약간 시간이 흐름에 따라 감쇠하는 factor가 있는건가..?

Copy link
Contributor

Choose a reason for hiding this comment

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

이거 혹시 하고 싶다면...? 그냥 좋아요 개수 / 청중 수를 감쇠로하지 말고, 시간 흐름별 좋아요 개수를 moving average로 해보는건 어때요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아마 시간이 지날수록 해당 과목이 현재랑은 좀 달라져서 효과는 떨어지니까 그걸 노리고 만들었던 것 같아요


const mostLikedReviews = reviews
.sort((a, b) => calculateKey(b) - calculateKey(a))
.slice(0, likedCount);

const latestDateStart = new Date();
latestDateStart.setDate(latestDateStart.getDate() - 7);

const latestReviews = reviews.filter(
(review) =>
review.written_datetime &&
new Date(review.written_datetime) >= latestDateStart,
);

const bestCandidateReviews = [...mostLikedReviews, ...latestReviews];
return bestCandidateReviews.length > maxResultCount
? bestCandidateReviews.slice(0, maxResultCount)
: bestCandidateReviews;
}

console.log('Running scheduled job: Update Best Reviews');

// Process humanity reviews
const humanityReviews = await this.reviewsRepository.getHumanityReviews();
const humanityBestReviews = getBestReviews(humanityReviews, 50, 20);

await this.reviewsRepository.clearHumanityBestReviews();
await this.reviewsRepository.addHumanityBestReviews(
humanityBestReviews.map((r) => ({ reviewId: r.id })),
);

// Process major reviews
const majorReviews = await this.reviewsRepository.getMajorReviews();
const majorBestReviews = getBestReviews(majorReviews, 2000, 1000);

await this.reviewsRepository.clearMajorBestReviews();
await this.reviewsRepository.addMajorBestReviews(
majorBestReviews.map((r) => ({ reviewId: r.id })),
);

console.log('BestReview updated by scheduled job');
}
}

Choose a reason for hiding this comment

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

코드 리뷰를 진행하겠습니다. 전체적으로 동작 로직은 명확하게 작성되어 있습니다. 그러나 다음과 같은 개선 사항 및 버그 위험 요소들이 있습니다:

  1. 명확한 타입 정의:

    • calculateKey, getBestReviews 함수에서 매개변수와 반환 값에 대해 타입을 명시하는 것이 좋습니다. 특히 getBestReviews에서 likedCount, mostLikedReviews 등의 값이 어떻게 사용되는지 명확히 하는 것이 가독성 향상에 기여합니다.
  2. 상수화:

    • 0.85와 같은 상수 값은 의미를 명시할 수 있는 이름을 가진 상수로 추출하여 사용하는 것이 좋습니다. 이렇게 하면 코드의 가독성이 향상되고, 이후에 수정할 일이 생겼을 때 유지보수가 용이해집니다.
  3. 에러 처리:

    • await를 사용하는 부분에서 에러가 발생할 경우, 적절한 예외 처리가 이루어지지 않고 있습니다. 예를 들어, 데이터베이스 소스에서 리뷰를 가져오지 못하는 상황을 고려하여 적절한 예외 처리를 추가하는 것이 좋습니다.
  4. 정렬 및 필터링 로직:

    • mostLikedReviews를 생성하기 위한 sort()slice()는 비효율적일 수 있습니다. 특히 reviews 배열이 클 경우 성능 이슈가 발생할 수 있으므로, 필터링 및 정렬 방식을 검토해 보는 것이 좋습니다.
  5. 로그 개선:

    • 로그 메시지가 기본적인 상태를 출력하고 있으나, 현재 처리 중인 리뷰의 수나 작업 진행 상태 등을 추가하여 더욱 유용한 로그 정보를 제공할 수 있습니다.
  6. Cron 표현식의 설명:

    • Cron 스케줄링 작업의 표현식에 대해 주석으로 설명을 추가하였으나, 다른 개발자가 이 코드를 읽을 때 어떤 주기로 실행되는지를 명확히 이해할 수 있도록 보다 구체적인 설명을 추가하는 것이 좋습니다.
  7. 성능 최적화:

    • getBestReviews에서 두 개의 배열 (mostLikedReviews, latestReviews)를 결합할 때, 이 배열들의 길이에 따라 불필요한 스플라이스가 발생할 수 있습니다. 결과적으로 항상 최대 길이로 생성되고, 필요에 따라 자르기보다는 조건에 따라 생성하는 방식으로 수정할 수 있습니다.

이러한 지점들을 고려하면 코드의 품질과 유지보수성이 높아질 것으로 생각됩니다. 코드를 잘 작성하셨습니다. 앞으로도 계속 개선하는 방향으로 나아가시길 바랍니다!

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-- CreateIndex
CREATE INDEX `review_humanitybestreview_review_id_idx` ON `review_humanitybestreview`(`review_id`);

-- CreateIndex
CREATE INDEX `review_majorbestreview_review_id_idx` ON `review_majorbestreview`(`review_id`);

-- AddForeignKey
ALTER TABLE `review_humanitybestreview` ADD CONSTRAINT `review_humanitybestreview_review_id_fkey` FOREIGN KEY (`review_id`) REFERENCES `review_review`(`id`) ON DELETE CASCADE ON UPDATE CASCADE;

-- AddForeignKey
ALTER TABLE `review_majorbestreview` ADD CONSTRAINT `review_majorbestreview_review_id_fkey` FOREIGN KEY (`review_id`) REFERENCES `review_review`(`id`) ON DELETE CASCADE ON UPDATE CASCADE;

Choose a reason for hiding this comment

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

이 코드 패치는 두 개의 인덱스를 생성하고, 두 개의 외래 키 제약 조건을 추가하는 SQL 스크립트입니다. 다음은 코드 리뷰 및 개선 사항입니다:

  1. 인덱스 생성:

    • 인덱스 이름이 명확하고, 인덱스가 적용될 컬럼도 명확하게 지정되어 있으므로 좋습니다.
    • 인덱스가 필요한 이유나 목적이 주석으로 추가되면 더 이해하기 쉬울 것입니다. 예를 들어, 성능 최적화나 쿼리 개선을 위한 것인지 명시하면 좋습니다.
  2. 외래 키 제약 조건:

    • 외래 키의 참조 테이블인 review_review의 컬럼(id)이 적절히 설정되어야 합니다. 만약 해당 컬럼이 기본 키가 아니라면 오류가 발생할 수 있습니다.
    • ON DELETE CASCADEON UPDATE CASCADE 설정이 비즈니스 로직에 맞는지 확인해야 합니다. 이러한 설정은 관련된 데이터가 삭제/수정될 때 전파되어 영향을 미칠 수 있으므로 신중한 검토가 필요합니다.
  3. 에러 및 예외 처리:

    • 독립적으 로 실행할 수 있도록 제약 조건 추가와 인덱스 생성이 순차적으로 성공할 것이라는 가정 하에 작성되어 있습니다. 실제로는 각 쿼리 실행 후 에러 처리 로직이 필요할 수 있습니다.
    • 이러한 제약 조건 추가가 실패할 경우, 전체 작업이 롤백되도록 트랜잭션을 사용하는 것을 고려해 보세요.
  4. 코드 유지 관리:

    • 인덱스와 외래 키 제약 조건의 이름이 매우 길어져 있습니다. 이는 코드 유지보수 시 불편할 수 있습니다. 더 간결한 이름을 고려해보는 것이 좋습니다.
  5. 주석 사용:

    • 각 SQL 명령어 앞에 주석이 기본적으로 제공되지만, 구체적인 설명이 포함되면 좋습니다. 예를 들어, "이 인덱스는 리뷰 검색 성능을 높이기 위해 생성되었습니다."와 같은 설명을 추가하면 코드의 의도가 분명해질 것입니다.

결론적으로 코드 구조나 명명 규칙은 적절하나, 비즈니스 로직과 성능 고려를 위해 추가적인 검토가 필요합니다. 에러 처리 및 유지 보수성을 높이기 위한 개선이 필요합니다.

58 changes: 58 additions & 0 deletions src/prisma/repositories/review.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,4 +401,62 @@ export class ReviewsRepository {
},
});
}

// Fetch humanity reviews (HSS department)
async getHumanityReviews(): Promise<EReview.WithLectures[]> {
return this.prisma.review_review.findMany({
where: {
course: {
subject_department: {
code: 'HSS',
},
},
},
include: {
lecture: true,
},
});
}

// Fetch major reviews (Non-HSS department)
async getMajorReviews(): Promise<EReview.WithLectures[]> {
return this.prisma.review_review.findMany({
where: {
NOT: {
course: {
subject_department: {
code: 'HSS',
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 기필도 다 포함되어서 subject_department 긁어와서 in으로 넣는게 낫지 않을까 싶긴합니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

그리고 EReview.withLectures 만드셨으니까 include에 저렇게 lecture:true를 쓰시는게 아니라 include: EReview.withLectures.include 이렇게 하시면 됩니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

subject_department 테이블 보니까 전공만 따로 뽑는게 없는 거 같더라구요... 또 그냥 학과들만 넣자니 부전공 복수전공도 띄우는 것 같아서 일단은 visible 정도만 추가하겠습니다

},
},
},
},
include: {
lecture: true,
},
});
}

// Clear all humanity best reviews
async clearHumanityBestReviews() {
return this.prisma.review_humanitybestreview.deleteMany();
}

// Clear all major best reviews
async clearMajorBestReviews() {
return this.prisma.review_majorbestreview.deleteMany();
}

// Add new humanity best reviews
async addHumanityBestReviews(reviews: { reviewId: number }[]) {
return this.prisma.review_humanitybestreview.createMany({
data: reviews.map((review) => ({ review_id: review.reviewId })),
});
}

// Add new major best reviews
async addMajorBestReviews(reviews: { reviewId: number }[]) {
return this.prisma.review_majorbestreview.createMany({
data: reviews.map((review) => ({ review_id: review.reviewId })),
});
}
}

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

코드 리뷰

  1. 명확한 함수 이름: 각 함수의 이름은 기능을 잘 설명합니다. getHumanityReviewsgetMajorReviews는 각각 인문학 및 전공 리뷰를 가져오는 기능에 대해 명확합니다.

  2. 검증 추가: addHumanityBestReviewsaddMajorBestReviews 메서드는 리뷰 배열을 받아 데이터베이스에 추가합니다. 이 배열이 비어있거나 유효하지 않은 경우 예외 처리가 필요할 수 있습니다. 빈 배열을 추가하려고 할 때 데이터베이스 호출은 비효율적일 수 있습니다.

  3. 에러 핸들링: 현재 코드에는 에러 핸들링 로직이 없습니다. 데이터베이스 작업에서 예외가 발생할 수 있으므로, try-catch 문을 통해 에러를 처리하고 사용자에게 적절한 피드백을 제공해야 합니다.

  4. 주석 추가: 코드 각 섹션에 대한 주석이 잘 작성되어 있으며, 기능에 대한 개요를 제공합니다. 추가적으로 각 함수의 반환 값이나 예외가 발생할 수 있는 경우에 대해 더 상세한 주석을 추가하는 것도 좋습니다.

  5. 성능 고려: getMajorReviews에서 HSS 과목이 아닌 모든 리뷰를 검색하는 방식이 성능에 영향 미칠 수 있습니다. 대량의 데이터에 대해 쿼리가 느려질 수 있으므로, 인덱스를 추가하거나 쿼리를 최적화하는 것을 고려해야 할 수 있습니다.

  6. 타입 정의 강화: addHumanityBestReviewsaddMajorBestReviews에서 reviews 매개변수의 타입을 좀 더 구체화할 수 있습니다. 현재 reviewId가 숫자 형식임을 명시했지만, 이 외에 필요한 속성이 있는 경우 포함할 수 있습니다.

버그 위험

  • clearHumanityBestReviewsclearMajorBestReviews 함수는 데이터 삭제 작업을 수행하므로, 이러한 함수가 호출될 때 의도치 않은 데이터 삭제를 방지하기 위해 추가적인 확인을 요구할 수 있습니다.
  • createMany 메서드에 전달된 데이터에 대해 올바른 형식인지 검증하지 않으면 데이터베이스 오류를 일으킬 수 있습니다.

이와 같은 피드백을 바탕으로 코드를 보강하고 안정성을 높일 수 있을 것입니다.

11 changes: 10 additions & 1 deletion src/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,16 @@ model main_reviewwritedailyuserfeed {

model review_humanitybestreview {
review_id Int @id
review review_review @relation(fields: [review_id], references: [id], onDelete: Cascade)

@@index([review_id])
}

model review_majorbestreview {
review_id Int @id
review_id Int @id
review review_review @relation(fields: [review_id], references: [id], onDelete: Cascade)

@@index([review_id])
}

model review_review {
Expand All @@ -242,6 +248,9 @@ model review_review {
lecture subject_lecture @relation(fields: [lecture_id], references: [id])
writer session_userprofile? @relation(fields: [writer_id], references: [id])
review_reviewvote review_reviewvote[]
review_majorbestreview review_majorbestreview? @relation
review_humanitybestreview review_humanitybestreview? @relation


@@unique([writer_id, lecture_id], map: "review_comment_writer_id_af700a5d_uniq")
@@index([written_datetime], map: "review_comment_e5e30a4a")

Choose a reason for hiding this comment

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

코드 패치에 대한 리뷰를 진행하겠습니다.

  1. 관계 설정:

    • review_humanitybestreviewreview_majorbestreview 모델에서 review 필드가 제대로 생성되었습니다. 그러나, 이 관계에 대한 충돌 가능성을 고려해야 합니다. 두 모델이 동일한 review_id를 공유할 경우, 외래 키 제약 조건이 위반될 수 있습니다. 각각의 모델에 대해 어떤 상황에서 onDelete: Cascade가 필요한지 명확히 정리하는 것이 좋습니다.
  2. 인덱스 추가:

    • 인덱스를 추가한 것은 성능 측면에서 긍정적입니다. 그러나 review_id가 이미 기본 키로 설정되어 있는 상태에서 중복 인덱스를 생성한 것은 불필요할 수 있습니다. 기본 키 자체가 인덱스를 제공하므로, 추가적인 인덱스는 성능 향상에 크게 기여하지 않을 수 있습니다.
  3. 모델 명명 규칙:

    • 모델명을 통해 어떤 기능이나 목적을 갖는지 알 수 있도록 더 직관적인 명명 규칙을 사용하는 것이 좋습니다. 예를 들어, review_majorbestreviewreview_humanitybestreview를 더 명확한 이름으로 변경하면 코드 가독성이 향상될 것입니다.
  4. 주석 및 문서화:

    • 각 모델과 필드에 대한 적절한 주석을 추가하는 것이 좋습니다. 관계 설정이나 각 필드의 용도에 대한 설명이 추가되면 코드 이해도가 높아질 것입니다.
  5. 테스트 케이스:

    • 코드 변경 후 영향을 받는 부분에 대한 테스트 케이스를 작성하여, 변경 사항이 기존 기능에 악영향을 미치지 않는지 확인해야 합니다.
  6. 성능 고려사항:

    • 많은 수의 관계를 설정하면 성능에 영향을 줄 수 있으므로, 이러한 관계가 필요한지 다시 검토해 볼 필요가 있습니다.

종합적으로 보았을 때, 전반적으로 잘 작성된 코드이나, 몇 가지 개선 사항과 주의해야 할 점이 있습니다. 이러한 사항들을 고려하여 더 안정적이고 효율적인 코드를 작성하길 바랍니다.

Expand Down
Loading