Skip to content

Issue/225/kevin add v2 apis#229

Open
LarryKwon wants to merge 24 commits intodevfrom
issue/225/kevin-add-v2-apis
Open

Issue/225/kevin add v2 apis#229
LarryKwon wants to merge 24 commits intodevfrom
issue/225/kevin-add-v2-apis

Conversation

@LarryKwon
Copy link
Copy Markdown
Contributor

No description provided.

@LarryKwon LarryKwon requested a review from Copilot June 16, 2025 08:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces V2 APIs for timetables, lectures, and reviews along with associated serializers and interfaces. Key changes include adding new v2 endpoints in controllers and services, updated serializer functions for lecture, timetable, and review data, and new DTOs and interface modifications to support the V2 APIs.

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/server/src/modules/timetables/timetables.service.ts Added v2UpdateTimetable method with a comment indicating that the 'name' property is not present in the database.
apps/server/src/modules/timetables/timetables.module.ts Updated module to register the new v2TimetablesController.
apps/server/src/modules/timetables/timetables.controller.ts Added v2TimetablesController endpoints for GET, POST, PATCH, and DELETE operations.
apps/server/src/modules/reviews/reviews.controller.ts Introduced a commented out v2ReviewsController, likely intended for future use.
apps/server/src/modules/lectures/lectures.service.ts New methods for v2 lecture filtering and course enrichment have been added.
apps/server/src/modules/lectures/lectures.controller.ts Added v2LecturesController with new endpoints and dependency injections.
Apps in /common/serializer and /common/interfaces Updated serializers and interfaces to support V2 API responses and request DTOs.
apps/server/docs/openapi-generator.ts Minor update to support Windows by converting file paths to file URLs.
Comments suppressed due to low confidence (1)

apps/server/src/modules/lectures/lectures.controller.ts:68

  • [nitpick] Consider renaming the injected dependency from 'LectureService' to 'lecturesService' following the camelCase convention for instance variables.
private readonly LectureService: LecturesService,

await this.prismaService.timetable_timetable.update({
where: { id },
data: {
// name: dto.timeTableName, 문제: timetable 데이터베이스에 name 속성이 없음
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

Consider removing the commented-out code for the 'name' property if it is no longer needed to keep the code clean.

Suggested change
// name: dto.timeTableName, 문제: timetable 데이터베이스에 name 속성이 없음
// (Line removed to clean up the code)

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +121
/*
@Controller('api/v2/reviews')
export class v2ReviewsController {
constructor(private readonly reviewsService: ReviewsService) {}

@Public()
@Get()
async getReviews(
@Query() reviewsParam: IReview.v2QueryDto,
@GetUser() user: session_userprofile,
): Promise<IReview.v2Response> {
if (reviewsParam.response_type === 'count') {
const reviewsCount = await this.reviewsService.getReviewsCount(
reviewsParam.year,
reviewsParam.semester,
)
return reviewsCount
}
const result = await this.reviewsService.getReviews(reviewsParam, user)
return result
}
}
*/
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

If the commented-out v2ReviewsController code is not being used, please remove it to avoid confusion and reduce clutter.

Suggested change
/*
@Controller('api/v2/reviews')
export class v2ReviewsController {
constructor(private readonly reviewsService: ReviewsService) {}
@Public()
@Get()
async getReviews(
@Query() reviewsParam: IReview.v2QueryDto,
@GetUser() user: session_userprofile,
): Promise<IReview.v2Response> {
if (reviewsParam.response_type === 'count') {
const reviewsCount = await this.reviewsService.getReviewsCount(
reviewsParam.year,
reviewsParam.semester,
)
return reviewsCount
}
const result = await this.reviewsService.getReviews(reviewsParam, user)
return result
}
}
*/
// Removed the commented-out v2ReviewsController code block to reduce clutter.

Copilot uses AI. Check for mistakes.

export const v2toJsonProfessors = (professors: subject_professor[]): IProfessor.v2Basic[] => professors.map((professor) => ({
professorName: professor.professor_name,
professerId: professor.professor_id,
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

The property name 'professerId' appears to be misspelled; consider renaming it to 'professorId' for clarity and consistency.

Suggested change
professerId: professor.professor_id,
professorId: professor.professor_id,

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

const mod = await import(fileUrl)
for (const classDec of classes) {
const className = classDec.getName()
if (!className) continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 코드 패치에 대한 몇 가지 우려 사항이 있습니다:

  1. 주석 내용 불명확: // windows를 위해 약간 수정(?) 주석은 구체적이지 않습니다. 어떤 문제가 있었고 어떻게 수정했는지 명확히 설명해 주는 것이 좋습니다.

  2. pathToFileURL 사용의 의도: pathToFileURL로 파일 경로를 URI로 변환하는 이유가 설명되어 있지 않습니다. 이렇게 변경했을 때의 이점이나 필요성을 명시해야 이해하기 쉬울 것입니다. 기존 코드에서는 모듈 가져오기가 직접적으로 작동했는데, 이 변경으로 인해 성능이나 안정성에 문제가 발생할 수 있습니다.

  3. 에러 처리: import 문이 실패했을 때를 처리하는 로직이 없습니다. 파일 경로가 잘못되었거나 해당 파일이 존재하지 않는 경우를 처리하는 방식이 필요합니다. 적절한 에러 메시지를 출력하거나 예외를 던지는 방식으로 에러를 처리해야 합니다.

  4. 비동기 처리 고려사항: import 문은 비동기 처리를 고려해야 합니다. 상대적으로 긴 처리 시간을 가질 수 있으므로, 사용자에게 로딩 상태 등을 보여주는 방법이 필요할 수 있습니다.

  5. 테스트 필요: 특히 Windows와 관련된 변경 사항이므로, 이 변경이 다양한 환경에서 잘 작동하는지를 확인하기 위한 충분한 테스트가 필요합니다. 특히 macOS나 Linux 환경에서 기존 코드가 계속 잘 작동하는지를 검증해야 합니다.

이러한 점들을 수정하고 좀 더 명확한 설명을 추가하는 것이 좋습니다.


export interface Detail extends Basic {
related_courses_prior: Basic[]
related_courses_posterior: Basic[]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

  1. 인터페이스 추가: v2BasicForLecture 인터페이스가 새로 추가되었는데, 적절한 타입이 정의되어 있음을 확인했습니다. 그러나, 문서화가 부족합니다. 해당 인터페이스에 대한 사용 예시 또는 설명 주석을 추가하는 것이 좋습니다.

  2. 속성 유효성 검증: codetype 속성이 문자열로 정의되어 있지만, 이러한 문자열 값들이 어떤 범위나 리스트에 해당하는지 명시되어 있지 않습니다. 예를 들어, type이 'lecture', 'seminar'와 같은 특정 값만 허용된다면, 이를 명시하는 것이 필요할 수 있습니다.

  3. 타입 안전성: completedCourse는 boolean 타입입니다. 일반적으로 더 복잡한 논리가 요구될 수 있으므로, 이 필드를 확장할 여지가 있는지 고려해 볼 필요가 있습니다.

  4. 정렬: 코드가 시각적으로 더 보기 좋게 정렬될 수 있도록, 인터페이스 내 속성들을 정리하는 것이 좋습니다. 이는 가독성을 향상시킵니다.

  5. 기타 제안: 버전 관리의 일환으로, 이 인터페이스가 v2임을 명시하는 것 자체도 좋은 방침입니다. 그러나 나중에 관리를 쉽게 하려면, 버전관리를 더욱 체계적으로 처리할 수 있는 방법을 고안하는 게 좋습니다.

이러한 점들을 해결하면 코드의 품질과 유지보수성을 크게 향상시킬 수 있을 것입니다.


export class AutocompleteQueryDto {
@IsInt()
@Type(() => Number)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에 다음과 같은 몇 가지 문제가 있습니다.

  1. Null 가능 속성: 여러 인터페이스에서 numPeople과 같은 속성을 null로 설정하는 것은 좋지만, 이러한 속성이 실제로 그 경우에 적절한지 확인이 필요합니다. 해당 속성을 null로 두는 것이 코드의 나머지 부분에서 발생할 수 있는 잠재적인 버그를 일으킬 수 있습니다.

  2. 중복 코드: v2Detailv2DetailwithWishlited는 많은 필드를 공유합니다. 이 두 인터페이스를 합친 하나의 기본 인터페이스를 만들고, isWishlited 필드와 같은 추가 필드를 상속하는 방식으로 중복을 줄일 수 있습니다.

  3. TypeScript 변환: @Transform 데코레이터가 여러 곳에서 사용되고 있는데, 값이 undefined일 경우 parseInt가 NaN을 반환할 수 있습니다. 이 경우 @IsInt()@IsNumber() 검증이 기대한 대로 작동하지 않을 수 있습니다. 이러한 문제를 방지할 수 있도록 변환 함수를 수정할 필요가 있습니다.

  4. 문서화: 코드에 대한 문서가 부족합니다. 각 인터페이스와 클래스에 대한 설명을 추가하여 다른 개발자들이 코드를 이해하는 데 도움이 될 수 있습니다.

  5. 명명 규칙: v2Classtime, v2ExamTime 등의 명명이 적절하지만, 나중에 다른 버전의 클래스가 추가되면 혼란을 초래할 수 있습니다. 각 버전의 표기를 명확히 하여 서로 구별할 수 있도록 해야 합니다.

이러한 잠재적인 문제들을 해결한 후, 코드는 더 안전하고 유지 보수성이 높아질 것입니다.

professorName: string
professerId: number
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에서 몇 가지 문제와 개선 사항이 발견되었습니다.

  1. 타이포: professerId가 잘못된 철자입니다. 올바른 철자는 professorId입니다. 이는 코드의 일관성과 가독성을 해칠 수 있습니다.

  2. 인터페이스 명명 규칙: 새로 추가된 v2Basic 인터페이스 이름은 부적절할 수 있습니다. 더 명확하게 ProfessorBasic와 같은 이름으로 변경하는 것이 좋습니다. 이로 인해 코드의 의도가 더 명확해질 것입니다.

  3. 주석 추가: 타입이나 인터페이스가 무엇을 의미하는지에 대한 주석을 추가하는 것을 고려하십시오. 사용자가 코드의 목적을 이해하는 데 도움이 될 수 있습니다.

  4. 가능한 버그 위험: professorNameprofessorId의 데이터 타입이 올바른지 확인해야 합니다. 데이터베이스나 API에서 받아오는 값을 검증하지 않으면 나중에 런타임 오류가 발생할 수 있습니다.

결론적으로, 몇 가지 피드백을 고려한 후에 다시 검토하는 것이 좋습니다.


export class CreateDto {
@ApiProperty()
@IsString()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치를 검토한 결과, 몇 가지 우려사항과 개선 제안을 발견하였습니다:

  1. IsEnum 사용 문제: @IsEnum(['default', 'recent', 'hall-of-fame'])는 올바르지 않은 사용법입니다. @IsEnum에는 해당 Enum 객체가 필요하며 문자열 배열을 인자로 전달할 수 없습니다. Enum을 정의하신 후 그 Enum 타입을 사용해야 합니다. 예를 들면, enum Mode { DEFAULT = 'default', RECENT = 'recent', HALL_OF_FAME = 'hall-of-fame' }와 같은 형식입니다.

  2. Transform 데코레이터의 사용: @Transform(({ value }) => (typeof value === 'string' ? [value] : value))mode의 타입을 배열로 강제 변환하려는 의도가 보이나, 실제로 mode가 여러 값을 가질 수 있도록 설계되어 있는지 확인이 필요합니다. 현재 mode가 스트링 리터럴로 정의되어 있기 때문에 배열로 변환할 필요가 없어 보입니다. 이는 불필요한 복잡성을 추가할 수 있습니다.

  3. 유효성 검사 추가: yearsemester는 과목이나 학기 정보와 관련된 중요한 필드입니다. 하지만 이들에 대한 추가 유효성 검사(예: 특정 범위나 값 체크)는 필요하지 않은지 검토해보기를 권장합니다.

  4. 클래스 구조의 일관성: v2Basic과 같은 인터페이스가 이후에 필요할 경우, 해당 인터페이스를 활용하는 곳에서도 일관된 방식으로 접근할 수 있는지 확인할 필요가 있습니다. 이로 인해 유지보수성을 높일 수 있습니다.

전체적으로 이러한 문제점을 수정하고 리뷰 후에 머지를 진행하는 것이 좋습니다.


public termFilter(term?: string[]): object | null {
if (!term) {
return null
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치는 여러 잠재적인 문제와 개선 가능성을 지니고 있습니다.

  1. Duplicated Import Statement: 코드 상단에 applyOffsetapplyOrder의 import 문이 두 번 존재합니다. 하나는 주석처리되어 있으며, 이 중 하나는 제거해야 합니다.

  2. Return Types: v2DepartmentFilter, v2TypeFilter, 및 v2LevelFilter 메서드는 object | null을 반환합니다. 타입을 모호하게 만들 수 있으므로, 어떤 구조의 객체가 리턴되는지를 명확히 정의하는 것이 좋습니다. 이는 후속 개발자가 이해하기 쉽게 합니다.

  3. Error Handling: this.TYPE_ACRONYMS[type as keyof typeof this.TYPE_ACRONYMS]의 접근이 실패할 경우(예: 잘못된 type 값) 런타임 오류가 발생할 수 있습니다. 이러한 상황을 처리하는 로직을 추가해야 합니다.

  4. Magic Strings and Numbers: 함수 내부에서 'ALL'이나 0과 같은 값을 하드코딩하는 대신, 상수 변수로 이를 정의하여 코드 가독성을 높이고 유지보수성을 향상시킬 수 있습니다.

  5. Redundant Checks: if (!department)if (department === 0)의 체크는 함께 결합할 수 있습니다. 예를 들어, if (department == null || department === 0)처럼 작성하여 코드를 간단하게 만들 수 있습니다.

이 외에도, 각 메서드에 대한 테스트가 포함되어 있는지 확인하여 코드의 안정성을 더욱 개선하는 것도 중요합니다.


async getMajors(user: session_userprofile): Promise<subject_department[]> {
const majors = (
await this.prisma.session_userprofile_majors.findMany({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

확인한 코드 패치에 대해 몇 가지 잠재적인 문제와 개선 사항을 제시하겠습니다:

  1. 트랜잭션 사용에 대한 주의: this.prisma.$transaction 내에서 여러 쿼리를 사용하고 있습니다. 이 경우, 트랜잭션 롤백이 필요할 수 있습니다. 그러나 deleteManycreateMany 작업 간에 에러가 발생할 경우, 전체 트랜잭션이 롤백되지 않으면 일관성이 깨질 수 있습니다. 트랜잭션이 올바르게 커밋되고 오류가 발생한 경우 예외 처리가 필요합니다.

  2. performance issue: newIds.map(...)에서 department_id를 할당하는 구조는 만약 newIds 배열이 클 경우 성능 문제를 일으킬 수 있습니다. 데이터가 크면 이 작업이 느려질 수 있습니다. 이를 개선하기 위해 createMany로 들어가는 데이터의 구성 방식을 최적화 고려해야겠습니다.

  3. 입력 검증 강화: 지금은 유효하지 않은 ID의 검증 후 예외를 던지지만, 함수 호출 전에 더 많은 검증을 수행할 수 있습니다. 예를 들어, newIds가 비어 있는지 또는 중복 값이 있는지 미리 확인하는 것이 좋습니다.

  4. 타입 검증: newIds에 대해 숫자 배열이라는 것을 보장할 방법이 없으므로, 데이터 타입 검사를 추가하는 것이 안전합니다. 이로 인해 API 사용자가 잘못된 형식으로 데이터를 전송할 경우의 문제를 예방할 수 있습니다.

  5. 코드 가독성: const existingIds = new Set(existing.map((d) => d.id)) 구문은 일반적인 사용 패턴입니다. 하지만 existing.map 대신 existing.map(({ id }) => id)와 같이 구조 분해 할당을 사용하면 가독성이 향상됩니다.

요약하자면, 몇 가지 개선 사항을 통해 더 높은 안정성과 성능을 얻을 수 있습니다. 따라서 이 코드는 즉시 병합하기보다는 검토가 더 필요합니다.

)
return applyOffset<ELecture.DetailsWithCourse>(orderedQuery, 0)
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

  1. 변수의 타입 명시: filters 변수는 object[]로 선언되어 있습니다. filter의 형식이 정확히 무엇인지 명확히 할 수 있도록 타입을 더 구체적으로 정의하는 것이 좋습니다. 예를 들면, filters: Array<Prisma.YourFilterType> 형식으로 표현할 수 있습니다.

  2. 예외 처리: findMany 메서드 호출에 대한 예외 처리가 없습니다. 데이터베이스 쿼리에서 문제가 발생할 경우를 대비하여 try-catch 블록을 사용하여 오류를 처리하는 것이 좋습니다.

  3. 정렬 및 오프셋: applyOrderapplyOffset 메서드의 호출 결과에 대한 예외 처리가 없습니다. 이 메서드들이 안전하게 작동하지 않을 경우에 대한 대처가 필요합니다.

  4. 필터링 구성: AND 조건과 필터가 전적으로 결합되는 방식에 대해 주의해야 합니다. 쿼리 내의 필터가 예상치 않은 동작을 발생시킬 수 있습니다. 각 필터에 대한 디버깅 또는 로깅 메커니즘을 추가하는 것을 고려해볼 수 있습니다.

  5. Magic Number 사용: DEFAULT_LIMIT을 300으로 설정했는데, 이 값이 어떤 의미를 가지는지 주석이 필요합니다. 매직 넘버를 사용하기보다는 의미 있는 이름이 있는 상수를 사용하는 것이 좋은 코드 작성 원칙입니다.

  6. 입력 유효성 검사: query의 각 필드에 대한 유효성 검사가 없습니다. 무시할 수 있는 값, 범위 검사 및 유형 검사를 수행하는 것이 좋습니다.

  7. 코드 일관성: 메서드 및 변수 명명 규칙이 일관되지 않은 경우, 코드의 가독성에 영향을 줄 수 있습니다. 전체 코드베이스에서 일관된 스타일을 유지하는 것이 좋습니다.

이 외에도 코드의 맥락과 전체 구조에 따라 추가적인 개선 점이 있을 수 있습니다. 전체 코드의 안정성과 가독성을 높이기 위해서는 모든 경로를 고려한 철저한 검토가 필요합니다.

day?: number
begin?: number
end?: number
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

새로 추가된 v2LectureQuery 인터페이스는 다음과 같은 몇 가지 문제가 있습니다.

  1. 명명 규칙: v2LectureQuery라는 이름은 어떤 명세를 따르는지 명확하지 않습니다. 인터페이스의 목적이나 관련성을 명확히 하기 위해 좀 더 구체적인 이름을 고려해 보세요.

  2. 타입 안전성: department, level, year, semester, day, limit, begin, end와 같은 속성들은 모두 number 타입으로 정의되어 있습니다. 이러한 속성들은 상황에 따라 string 또는 다른 구조체로 정의될 수 있습니다. 이러한 경우를 처리할 수 있는 타입을 사용하면 더 유연한 코드를 할 수 있습니다.

  3. 주석 및 문서화 부족: 각 속성이 무엇을 의미하는지에 대한 주석이 없습니다. 이 인터페이스를 사용하는 개발자가 쉽게 이해할 수 있도록 각 속성에 대한 설명을 추가하는 것이 좋습니다.

  4. 확장성: 데이터의 미래 확장을 고려하여 인터페이스를 설계하는 것도 중요합니다. 더 많은 쿼리 파라미터가 필요한 경우, 새로운 쿼리 파라미터를 추가할 수 있는 방법을 고려해야 합니다.

이러한 문제들을 개선하면 코드의 품질이 향상될 것입니다.

Comment thread package.json
"tsx": "^4.19.4",
"typescript-eslint": "^8.30.1",
"uuid": "^11.1.0",
"uuidv4": "^6.2.13",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

  1. 스크립트 경로 변경: generate:server:api-docs의 경로에서 apps/server/docs/openapi-generator.ts./apps/server/docs/openapi-generator.ts로 변경한 것은 상대 경로를 명확하게 해줍니다. 하지만 이 변경에 대한 코드가 현재 코드베이스에서 잘 작동하는지 충분한 테스트가 필요합니다.

  2. 빌드 스크립트 변경: build:server에서 yarn sentry:sourcemaps 명령어가 삭제되었습니다. 이로 인해 Sentry와 관련된 소스맵 업로드가 누락될 수 있습니다. 이 변경이 의도된 것인지 확인이 필요합니다.

  3. 의존성 업데이트: @types/node@sentry/nestjs의 버전이 업데이트되었습니다. 새로운 버전이 기존 코드와 호환되는지 확인해야 하며, 특히 major 버전 업데이트가 있다면 더 주의가 필요합니다.

  4. 의존성 추가: @sentry/node가 새로 추가되었습니다. 이 추가가 자연스럽게 이루어진 것인지, 혹은 기존의 기능에 영향을 줄 수 있는지 확인해야 합니다.

  5. 버전 관리: 의존성 버전을 증가시키는 것이 좋은 방법이지만, 호환성 문제를 줄이기 위해 패키지를 업그레이드한 경우 변경 로그를 검토하고 필요한 경우 코드를 수정해야 합니다.

최종적으로 코드가 실제로 잘 작동하는지 확인하기 위해 충분한 테스트를 진행하기를 권장합니다.

}

export const v2toJsonExamtime = (examtime: subject_examtime) => {
const DAY_STR = ['월요일', '화요일', '수요일', '목요일', '금요일', '토요일', '일요일']
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

enum으로 써야하는 것들은 libs/common/src/enum.ts에서 관리해줄래요? @kjh0209

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

넵 수정했습니다

}
}

export async function v2toJsonLectureWithCourseDetail2(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

얘는 위에 있는 v2toJsonLectureWithCourseDetail 이 친구랑 다른게 뭐에요..? 다른게 명확하면 이름을 다르게 짓고, 그렇지 않으면 적절히 하나로 쓸 수 있게 통합해야할 거 같아요

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

아 위에 거는 wishlist 관련 항목이 출력되는 거고 아래 거는 출력 안되는 응답이에요 api 목록에 response 형식이 wishlist 관련 항목을 출력해야하는 거도 있고 아닌 거도 있고 그랬어서 이렇게 나눴었습니다..

@Type(() => Number)
professorId?: number

@IsEnum(['default', 'recent', 'hall-of-fame'])
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

이것도 다른 Enum들처럼 비슷하게 해주세용

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

넵 수정했습니다

order?: string[]

@IsOptional()
@Type(() => Number)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

이거 왜 바꾼거야..?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

엇 그러게요..? 아무래도 제가 다른 코드들 수정하면서 실수로 탭이 눌러져서 바뀐 것 같아요 수정 취소해도 될 것 같습니다

Comment on lines +98 to +121

/*
@Controller('api/v2/reviews')
export class v2ReviewsController {
constructor(private readonly reviewsService: ReviewsService) {}

@Public()
@Get()
async getReviews(
@Query() reviewsParam: IReview.v2QueryDto,
@GetUser() user: session_userprofile,
): Promise<IReview.v2Response> {
if (reviewsParam.response_type === 'count') {
const reviewsCount = await this.reviewsService.getReviewsCount(
reviewsParam.year,
reviewsParam.semester,
)
return reviewsCount
}
const result = await this.reviewsService.getReviews(reviewsParam, user)
return result
}
}
*/
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

이거는 왜 주석처리 해놨어여??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

아 제가 reviews에 대한 api도 만드려다가 시간이 없어서 중단했었는데 그래서 주석처리 해놨어요

}
return {
new_code: {
contains: `__${level.toString()[0]}`,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

이거 혹시 이렇게 해야 동작하는건가?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 부분도 원래 lecture filter api를 만드려다가 필터가 제대로 동작을 안해서 다른 api부터 개발하기로 넘어갔던 부분이라 새로 코드를 수정해야할 것 같습니다..

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

for (const lecture of lectures) {
let added = false
for (const takenlist of takenLecturesJsons) {
if (takenlist.year === lecture.year && takenlist.semester === lecture.semester) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

  1. 예외 처리: wishlistRepository가 필요할 때 존재하지 않는 경우를 처리하고 있습니다. 이는 좋은 접근이지만, 코드 흐름에서 예외 발생 시 더 자세한 정보(예: 어떤 함수에서 발생했는지)를 제공하는 것이 좋습니다.

  2. 유효성 검사: lecture와 관련된 데이터의 유효성을 검증하는 로직이 없습니다. 예를 들어, lecture.subject_classtime이나 lecture.subject_examtime의 길이가 0인 경우를 처리하는 로직이 필요합니다. 관련 배열이 비어 있을 경우, 코드가 오류를 발생시킬 수 있습니다.

  3. 사용하지 않는 매개변수: 기존에 사용하던 wishlistRepository를 선택적(optional)로 변경했지만, 이후 처리 로직이 두 가지 경우로 나뉘면서 복잡성이 증가했습니다. 이러한 경우에는 코드의 가독성을 향상하기 위해 전체 구조를 다시 생각해야 할 수 있습니다.

  4. 변수명: isWishlited라는 변수명은 약간의 혼동을 줄 수 있습니다. isWishlisted로 변경하여 더 직관적으로 만드는 것이 좋습니다.

  5. 타입 안정성: 반환되는 타입이 Promise<ILecture.v2Response | ILecture.v2Response2>로 되어 있는데, 이 두 타입의 차이가 명확하지 않습니다. 더 구체적인 타입을 사용하는 것을 고려해 보세요.

  6. 성능 문제: getOrCreateWishlist와 같은 비동기 호출이 두 번 발생할 수 있기 때문에, 성능 저하의 위험이 있습니다. 이를 캐싱하거나 최소화하는 방법을 고려할 수 있습니다.

개선 제안

  • 예외 처리를 상세하게 하고 유효성 검사를 추가하여 잠재적인 런타임 오류를 피하세요.
  • 변수명을 좀 더 명확하게 변경하고, 코드의 가독성을 높이기 위해 함수의 책임을 명확히 정의하세요.


import { LecturesService } from './lectures.service'

@Controller('api/lectures')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 변경에서 Promise<ILecture.v2Response[]>Promise<ILecture.v2Response[] | ILecture.v2Response2[]>로 수정했는데, 이 변경은 몇 가지 잠재적 문제를 일으킬 수 있습니다. 첫째, 두 가지 다른 타입의 배열을 반환할 수 있다면 호출하는 측에서 반한 타입을 정확하게 처리해야 하는데, 이에 대한 고려가 부족할 수 있습니다. 이는 런타임 오류를 초래할 수 있습니다. 둘째, 타입의 혼합 사용은 코드의 가독성을 낮추고 추후 유지보수를 어렵게 만들 수 있습니다. 타입 안전성을 높이기 위해 두 결과를 명확히 분리하여 각기 다른 메서드로 처리하거나, 확장 가능한 방식으로 타입을 정의하는 것을 권장합니다. 마지막으로, 문서화도 함께 고려하여 호출하는 방법과 리턴되는 타입을 명확히 설명하면 좋겠습니다.

import { toJsonReview } from '@otl/server-nest/common/serializer/review.serializer'
import { session_userprofile } from '@prisma/client'

import {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 변경 사항에서 몇 가지 잠재적인 문제가 발견되었습니다.

  1. 타입 변경: 반환 유형이 Promise<ILecture.v2Response[]>에서 Promise<ILecture.v2Response[] | ILecture.v2Response2[]>로 변경되었습니다. 이 변경은 더 많은 반환 타입을 허용하므로, 타입 안정성에 대한 위험을 증가시킬 수 있습니다. 호출하는 곳에서 이 두 개의 타입을 모두 처리할 수 있는지 확인해야 합니다.

  2. 호출 인자 변경: v2toJsonLectureWithCourseDetail 함수 호출 시 인자 순서가 변경되었습니다. user 인자와 false가 추가되었습니다. 이로 인해 기존 코드를 사용하는 부분에서 인자 순서가 맞지 않으면 오류가 발생할 수 있습니다. 모든 사용처에서 인자 순서를 수정했는지 확인하지 않으면 런타임 에러가 발생할 수 있습니다.

  3. 비동기 처리: Promise.all을 사용하여 비동기 처리를 하고 있지만, queryResult가 비어있거나 모든 호출이 실패할 경우 어떻게 처리할지에 대한 예외 처리가 없습니다. 이 부분을 보완할 수 있는 방법을 고려해야 합니다.

결론적으로, 이 변경은 일부 기능에 심각한 영향을 미칠 수 있으므로, 변경 사항을 충분히 검토하고 필요 시 적절한 테스트를 통해 안정성을 확인하는 것이 중요합니다.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

import {
toJsonLectureDetail,
v2toJsonLectureWithCourseDetail,
} from '@otl/server-nest/common/serializer/lecture.serializer'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

다음은 코드 패치에 대한 리뷰입니다:

  1. 의도 확인 필요: false에서 true로 변경된 인자는 v2toJsonLectureWithCourseDetail 함수의 동작에 큰 영향을 줄 수 있습니다. 이 인자가 어떤 의미인지, 그리고 이 값이 true로 설정될 때 어떤 결과를 초래하는지 명확히 이해하고 있는지 확인해야 합니다.

  2. 유닛 테스트 부족: 변경된 부분에 대해 충분한 유닛 테스트가 있는지 확인해야 합니다. 이 변화가 기존 기능에 어떤 영향을 주는지 테스트하는 것이 필요합니다.

  3. 비동기 처리 확인: Promise.all을 사용하여 비동기 처리를 하고 있기 때문에, v2toJsonLectureWithCourseDetail 함수 내부에서 에러가 발생할 경우 모든 프로미스가 거부될 수 있습니다. 이러한 상황을 처리하기 위한 에러 핸들링 메커니즘이 필요합니다.

  4. 주석 추가 권장: 해당 코드의 주요 변화에 주석을 추가하여 향후 코드 유지보수 시 변경된 논리를 이해하기 쉽게 만드는 것이 좋습니다.

  5. 성능 고려사항: 많은 강의 데이터가 반환될 경우, 해당 코드의 성능이 어떻게 되는지 고려해야 합니다. 코드 리뷰어는 이 부분에서 성능 테스트를 제안할 수 있습니다.

변경 사항을 충분히 이해하고 추가적인 검토를 한 후에 머지를 고려하는 것이 좋습니다.

import { v2toJsonLikedReviews } from '@otl/server-nest/common/serializer/review.serializer'
import { session_userprofile } from '@prisma/client'

import { LectureRepository, WishlistRepository } from '@otl/prisma-client'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에서 몇 가지 잠재적인 문제와 개선 사항이 있습니다.

  1. 변수 이름 변경: 함수 이름이 변경되었습니다. v2toJsonLectureWithCourseDetail2에서 v2toJsonLectureWithCourseDetail로 변경되었는데, 새로운 함수가 이전 함수의 기능과 동일하다는 보장이 없으므로, 관련 문서나 주석을 확인해 두 함수의 동작 차이에 대해 명확히 해야 합니다.

  2. 인자 수 변경: 새로 호출된 함수는 5개의 인자를 받는데, 이전 함수는 4개의 인자를 받았습니다. 추가된 falsethis.wishlistRepository 인자의 목적이 불명확하면, 해당 변화가 함수의 동작에 어떤 영향을 미칠 수 있는지 주의해야 합니다. 함수 사용자가 제공하는 인자의 의미를 문서화하는 것이 좋습니다.

  3. 에러 핸들링: throw new HttpException 부분이 사용자 정보가 없을 경우에만 발생하는 것 같습니다. 다른 오류 처리도 고려해야 합니다. 예를 들어, getLecturesByIds 호출 또는 v2attachCourseToDetails 호출에서 오류가 발생할 경우를 처리할 수 있어야 합니다.

  4. 성을 고려한 비동기 처리: Promise.all로 모든 레크리에이션을 처리하고 있는데, 이는 모든 Promise가 거부되는 경우 에러를 발생시킵니다. 개별적으로 Promise를 처리하고 에러를 기록할 방법을 고려하는 것이 좋습니다.

  5. 주석 및 문서화: 코드의 변경 사항에 대한 주석이 없으므로, 팀원이 코드를 이해하는 데 어려움을 겪을 수 있습니다. 주요 변경 사항을 주석으로 명실상부하게 설명하거나 관련 문서에 업데이트를 반영할 필요가 있습니다.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

import { LectureRepository, WishlistRepository } from '@otl/prisma-client'

import { LecturesService } from './lectures.service'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에 대한 몇 가지 개선 사항과 잠재적인 버그가 있습니다.

  1. 주석 처리된 부분: // 필터링이 제대로 동작하지 않아 코드 수정이 필요함. 주석에 대한 설명이 부족합니다. 어떤 필터링이 잘못되었는지, 어떤 수정이 필요한지를 명확히 기재하는 것이 좋습니다. 주석은 코드의 의도를 분명히 전달해야 합니다.

  2. 작업 필요성: 필터링이 제대로 동작하지 않는다면, 이를 수정하는 작업이 코드의 나머지 부분에서 발생할 수 있는 문제를 방지하는 동시에, 필터링 로직에 대한 명확한 이해를 제공해야 합니다. 구체적으로 어떤 필터링 로직이 문제인지, 어떤 예외 상황을 처리해야 하는지에 대한 논의가 필요합니다.

  3. 테스트 케이스: 필터링 로직을 수정할 경우, 새로운 테스트 케이스를 추가하여 해당 기능이 올바르게 작동하는지 확인할 필요가 있습니다.

  4. 비즈니스 로직에 대한 설명 추가: 메소드에서 어떤 매개변수를 사용하며 어떻게 데이터를 처리하는지 시각적으로 명확히 알 수 있도록 주석을 추가하는 것이 좋습니다.

이러한 부분들이 개선되면 코드 품질도 향상될 것입니다.

if (reviewsParam.response_type === 'count') {
const reviewsCount = await this.reviewsService.getReviewsCount(
reviewsParam.year,
reviewsParam.semester,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에 몇 가지 문제가 있습니다:

  1. 주석 처리된 코드: v2ReviewsController 클래스의 모든 내용이 주석으로 처리되었습니다. 이는 실제 기능이 작동하지 않도록 하여, 테스트 또는 배포에서 문제를 일으킬 수 있습니다. 주석 처리된 코드를 실제로 실행 가능한 상태로 되돌리거나, 불필요한 코드는 제거하는 것이 좋습니다.

  2. API 개발 상태 주석: "아직 개발 중인 api임. 코드 수정이 필요함." 이 주석은 불확실성을 추가합니다. 가능한 경우, 해당 API의 개발 상태를 더 명확히 하고, 필요한 수정 사항이나 TODO 목록을 추가하여 다른 개발자가 더 쉽게 이해하고 수정할 수 있도록 해야 합니다.

  3. 에러 처리 논리 없음: getReviews 메서드에서 예외 상황이나 오류를 처리하는 로직이 없습니다. 완료된 상태의 API는 사용자의 요청에 따라 적절한 에러 메시지를 반환해야 합니다.

  4. 주석 정리 필요: 코드에 남겨진 주석이 혼란스러울 수 있으므로, 주석의 목적에 맞게 정리하는 것이 좋습니다. 코드의 가독성을 높이고 유지보수를 용이하게 하기 위해 주석을 명확하고 간결하게 만들어야 합니다.

이러한 문제들을 해결한 후에 코드를 병합하는 것이 좋습니다.

controllers: [ReviewsController, v2ReviewsController],
providers: [ReviewsService, UserService],
})
export class ReviewsModule {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

변경 사항에서 v2ReviewsController를 추가한 것은 관심을 끌 수 있지만, 이것이 실제로 필요하고 의도한 대로 처음부터 설정되었는지 확인해야 합니다. 잠재적인 문제는 다음과 같습니다:

  1. 중복된 Route Conflicts: ReviewsControllerv2ReviewsController 간의 路由 충돌 여부를 검토해야 합니다. 두 컨트롤러가 동일한 경로를 제공하고 있지 않은지 확인할 필요가 있습니다.

  2. Test Coverage: v2ReviewsController의 기능에 대해 적절한 단위 테스트가 작성되었는지 확인하세요. 만약 테스트가 없다면, 기존의 코드와의 호환성을 유지할 수 없을 가능성이 있습니다.

  3. Documentation: 추가된 컨트롤러에 대한 적절한 문서화가 필요합니다. 향후 코드베이스를 유지보수 할 때 명확한 참고자료가 있는 것이 유용합니다.

  4. Dependency Injection: v2ReviewsController가 필요한 서비스들을 제대로 주입 받고 있는지 확인하세요. 새 컨트롤러에 대한 의존성이 있지만 등록되지 않은 경우, 런타임 오류를 유발할 수 있습니다.

이러한 사항들을 검토 후, 추가적인 문제를 피하고 안전한 통합을 위해 변경 사항을 지속적으로 평가해야 합니다.

LarryKwon added 16 commits July 27, 2025 02:24
* Add: optimize course search

* Add: add level column to schema.prisma and make level filter with sql
* Add: optimize course search

* Add: add level column to schema.prisma and make level filter with sql

* Add: rabbitmq refactor

* Add: restore test

* Add: make scholar server work

* Add: make server work

* Add: add console.log for test

* Add: add temp console.log for test

* Add: add temp console.log for test

* Add: create channel per instance

* Add: just throw error to retry

* Add: console.log for testing

* todo: lecture common title redlock

* Add: redlock for common title update

* Add: dockerfile make run

* Add: temporary console log for test

* Add: temporary console log for test

* Add: temporary console log for test

* Add: temporary console log for test

* Add: timeout as option and prefetch with 5

* Add: dynamic module, throw error except lock error

* Add: remove unnecessary console.log
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

import { applyOrder } from '@otl/common/utils'

import { LectureRepository, WishlistRepository } from '@otl/prisma-client'
import { ELecture } from '@otl/prisma-client/entities'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 검토

  1. 변수 이름 변경: lectureData.course_id에서 course_idcourseId로 변경되었습니다. 이 변경이 의도된 것인지 확인하시기 바랍니다. 기존 코드는 대문자(_id) 스타일을 사용하고 있었으며, 변경으로 인해 다른 부분에서도 이와의 일관성을 유지하는지 검토해야 합니다.

  2. 타입 안정성: takenLectures 배열의 타입이 any로 설정되어 있습니다. 더 구체적인 타입을 정의하여 코드의 타입 안정성을 높이고, 나중에 발생할 수 있는 버그를 예방하는 것을 권장합니다.

  3. 에러 처리: getLectureByIdgetTakenLectures 메소드에서 예외가 발생할 경우의 처리가 없습니다. 이들 함수를 호출할 때 예외를 처리하여 시스템의 안정성을 높일 수 있습니다.

  4. 비동기 호출에 대한 경량화: 두 개의 비동기 호출이 순차적으로 이루어지고 있습니다. 이러한 호출이 서로 독립적이라면 Promise.all을 사용하여 두 호출을 병렬로 처리하여 성능을 향상시킬 수 있습니다.

이와 같은 사항을 반영하여 추가적인 검토를 통해 안정성을 높이길 바랍니다.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

const mod = await import(fileUrl)
for (const classDec of classes) {
const className = classDec.getName()
if (!className) continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 코드 패치는 몇 가지 잠재적인 문제와 개선 가능성이 있습니다.

  1. 주석: 주석에 'windows를 위해 약간 수정(?)'이라고 명시했으나, 어떤 수정인지 명확하지 않습니다. 이 주석을 더 구체적으로 작성해야 코드 유지보수 시 이해하기 쉬울 것입니다.

  2. 경로 처리: pathToFileURL을 사용하는 이유가 명확하지 않습니다. 만약 파일 경로가 잘못된 경우 에러가 발생할 수 있습니다. 여기에 대한 예외 처리를 추가하는 것을 고려해 보세요.

  3. 동기/비동기 처리: await import(fileUrl) 호출 후 모듈을 비동기적으로 가져오는 것을 볼 수 있습니다. 이 경우, 오류 발생 시에 적절한 오류 처리가 필요합니다. 예를 들어, try-catch 블록을 사용하여 에러를 로깅하거나 사용자에게 알리는 방법을 고려해야 합니다.

  4. 타입 안전성: TypeScript를 사용하고 있으므로 mod의 타입을 명시하는 것이 좋습니다. 현재 mod는 어떤 타입인지 명확하지 않습니다. 이로 인해 나중에 코드에 대해 예측할 수 있는 문제가 발생할 수 있습니다.

  5. 보안 문제: 외부 파일을 동적으로 가져오는 것은 보안 취약점을 초래할 수 있습니다. 신뢰할 수 없는 출처에서 파일을 가져오는 경우 반드시 검증을 추가해야 합니다.

위의 문제점들을 해결하고, 특히 명확한 주석과 오류 처리를 추가한다면 이 코드는 더 안정적으로 보일 것입니다.


export interface Detail extends Basic {
related_courses_prior: Basic[]
related_courses_posterior: Basic[]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 코드 패치는 새로운 인터페이스 v2BasicForLecture를 추가하고 있습니다. 다음과 같은 몇 가지 우려 사항이 있습니다:

  1. 타입 안전성: type 프로퍼티는 단순한 문자열로 정의되어 있는데, 만약 기대되는 값들의 목록이 있다면, ENUM 또는 리터럴 타입으로 정의하는 것이 좋습니다. 이를 통해 타입 안전성을 높이고, 잘못된 값의 사용을 방지할 수 있습니다.

  2. 문서화 부족: v2BasicForLecture 인터페이스에 대한 설명이 없습니다. 각 프로퍼티가 어떤 의미를 가지는지 주석으로 설명을 추가하는 것이 사용성을 높일 수 있습니다.

  3. 테스트 케이스: 새로 추가된 인터페이스에 대한 테스트 케이스가 존재하는지 확인이 필요합니다. 충분한 유닛 테스트가 없다면, 향후 버그 발생 가능성이 높아집니다.

  4. 변경 사항 관리: 기존 인터페이스와의 호환성을 고려해야 합니다. Detail 인터페이스와 v2BasicForLecture 간에 어떤 관계가 있는지 명확히 정의되어 있지 않다면, 추후 코드 유지보수에 어려움이 있을 수 있습니다.

이러한 문제를 해결하면 코드로의 통합이 더 안전하고 유의미할 것입니다.

keyword?: string

@IsOptional()
@IsString()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

다음은 코드 패치에 대한 리뷰입니다:

  1. null 값에 대한 확인 부족: num_peoplenumPeople 프로퍼티가 null을 허용합니다. 이 값을 사용할 때에는 항상 null 체크를 고려해야 합니다. 만약 null 값이 들어오면 클라이언트 측에서 제대로 처리되지 않을 수 있습니다.

  2. 상수값 관리: v2Classtime, v2ExamTime와 같이 인터페이스에서 사용되는 day, begin, end와 같은 숫자 값에 대해 추가적인 상수 값 정의가 필요할 수 있습니다. 예를 들어, day가 요일을 나타내기에, enum 를 사용하여 가독성을 높이고 버그의 가능성을 줄일 수 있습니다.

  3. 의미 있는 네이밍: v2DetailwithWishlited와 같은 네이밍은 오타가 있어 v2DetailWithWishlisted로 변경하는 것이 바람직합니다. 이는 코드의 가독성을 높이는 데 도움이 됩니다.

  4. _Transform 데코레이터 사용: Transform을 사용할 때 값이 정의되지 않았을 경우를 고려하여 더 Robust한 처리를 할 수 있습니다. 예를 들어, parsing이 실패할 경우의 예외처리를 추가해야 합니다.

  5. 테스트 필요성: 새로운 인터페이스와 클래스를 추가한 만큼, 이들에 대한 테스트 케이스를 추가하는 것이 필요합니다. 특히 데이터 유효성 검사를 포함하여 테스트를 강화하는 것이 좋습니다.

  6. v2Responsev2Response2 차이: 두 인터페이스가 거의 같은 구조를 가지고 있는 것처럼 보입니다. 중복을 피하기 위해 하나의 인터페이스로 통합할 수 있는 방법을 고려해볼 수 있습니다.

위의 사항들을 검토하여 코드를 개선하고 충분한 테스트를 수행한 후에 병합하는 것이 좋습니다.

professorName: string
professerId: number
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드에 몇 가지 문제가 있습니다:

  1. professerId의 철자가 잘못되었습니다. 올바른 이름은 professorId이어야 합니다. 이 오류로 인해 직렬화 및 역직렬화 시 문제가 발생할 수 있습니다.
  2. v2Basic 인터페이스가 IProfessor 네임스페이스에 추가되었습니다. 전체적인 네임스페이스 디자인을 고려하여 이 인터페이스의 위치가 적절한지 검토할 필요가 있습니다.
  3. 타입과 관련된 코드를 더 깔끔하게 하려면 통일성을 유지하는 것이 좋습니다. 예를 들어, professor_idprofessorId의 이름 규칙을 일관되게 유지하는 것이 좋습니다.

따라서 이 코드는 병합하기에 적합한 상태가 아닙니다. 위의 문제를 해결한 후에 다시 검토해야 합니다.

@Type(() => Number)
year?: number

@IsOptional()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

잠재적 버그 및 위험 사항

  1. mode 필드의 버그 위험: @IsEnum(ReviewSearchMode)@IsString()이 동시에 적용되었습니다. 이는 enum 타입이 문자열일 경우 불필요한 검증을 추가할 수 있으며, ReviewSearchMode가 문자열이 아닐 때의 시나리오를 고려해야 합니다. Enum 사용시 @IsEnum만으로 검증이 충분합니다.

  2. Transform 데코레이터의 사용: @Transform(({ value }) => (typeof value === 'string' ? [value] : value))mode에 대한 변환을 수행하긴 하지만, 배열로 변환하는 것이 필요한지 불확실합니다. 이 로직이 의도한 동작인지 확인이 필요합니다.

  3. isDeleted 필드의 타입: v2Basic 인터페이스에서 isDeleted가 boolean으로 보이는데, 이 상태가 null이 될 수 있는지를 명확히 해야 합니다. boolean | null이 필요한지 아닌지 확인이 필요합니다.

  4. 타입 일관성: v2Basicgrade, load, speech 필드의 타입이 명확히 정의되어야 합니다. 예를 들어, 숫자 범위가 있다면 @Min, @Max 등의 데코레이터를 사용하여 해당 필드에 대한 유효성 검사를 강화할 수 있을 것입니다.

  5. 문서화 부족: 각 필드가 어떻게 사용될 것인지에 대한 문서화가 부족합니다. @ApiProperty() 데코레이터를 사용하여 각 필드에 대한 설명을 추가하는 것이 좋습니다.

개선 제안

  • mode 필드의 검증 논리를 단순화하고, 배열 변환이 정말 필요한지 다시 검토해보세요.
  • v2Basicv2Response 인터페이스에 타입에 대한 명확성을 추가하여 코드 유지보수성을 향상할 수 있습니다.
  • 각 속성에 대해 주석 또는 설명을 추가하여 코드의 가독성을 높이고, 다른 개발자가 이해하기 쉽게 만드세요.

// (order as (keyof ECourse.Details)[]) ?? DEFAULT_ORDER,
// )
// return applyOffset<ECourse.Details>(orderedResult, offset ?? 0)
return queryResult
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 검토

  1. 반환 타입 및 명확성: 각 필터 메소드(v2DepartmentFilter, v2TypeFilter, v2LevelFilter)는 object | null을 반환하지만, 반환되는 객체의 구조가 예상되는 형태를 명확하게 문서화하지 않았습니다. 각 메소드의 반환 구조가 어떤 형태일지에 대한 주석이 필요합니다.

  2. 유효성 검사: v2DepartmentFilterv2LevelFilter에서 departmentlevel0일 경우를 null로 반환하고 있습니다. 하지만 경우에 따라 0이 유효한 값일 수도 있습니다. 이 부분은 비즈니스 로직을 고려해서 다시 검토해야 합니다. 필요하다면 추가적인 유효성 검사를 도입하는 것이 좋습니다.

  3. 타입 안정성: v2TypeFilter에서 this.TYPE_ACRONYMS[type as keyof typeof this.TYPE_ACRONYMS]는 주어진 타입이 정의되지 않았을 경우 오류를 발생시킬 수 있습니다. 여기서 keyof를 사용할 때, 존재하지 않는 키일 경우에 대한 처리 로직이 필요합니다.

  4. 일관된 네이밍: v2DepartmentFilter는 부서 필터를 나타내는데, v2TypeFilterv2LevelFilter는 각각 타입 및 레벨을 나타낸다는 점에서, 이렇다 할 일관성을 지키지 않습니다. v2 접두사가 인식하기 어렵고, 좀 더 명확하게 필터의 목적을 나타내는 네이밍을 고려해야 합니다.

  5. 테스트 필요성: 새로운 필터 메소드에 대한 유닛 테스트가 포함되어 있는지 확인해야 합니다. 각 메소드에 대해 여러 케이스 (예: null, undefined, 유효/무효 값)에 대해 테스트하는 것이 중요합니다.

이러한 관점을 기반으로 코드를 재검토하고 개선하는 것이 좋습니다.


async getMajors(user: session_userprofile): Promise<subject_department[]> {
const majors = (
await this.prismaRead.session_userprofile_majors.findMany({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 검토

  1. 예외 처리: BadRequestException을 던지기 전에 invalidIds가 비어있지 않은지 확인하는 부분은 잘 처리되었습니다. 하지만, 사용자에게 어떤 ID가 유효하지 않은지 구체적으로 알려주는 것은 좋습니다. 다만, 이는 민감한 정보를 제공할 수 있으므로 데이터가 어떻게 제공되고 있는지를 고려해야 합니다.

  2. 성능 문제: deleteMany 이후에 createMany를 사용하는 패턴은 성능상 리스크가 있습니다. 만약 여러 사용자가 동시에 이 메서드를 호출한다면, 경쟁 조건이 발생할 수 있습니다. 이 부분은 트랜젝션을 사용하여 다루고 있으므로 어느 정도 안전하긴 하지만, 여전히 데이터베이스에 대한 접근이 빈번해 질 수 있습니다. 대량의 데이터를 다룰 때 곧바로 삭제 후 추가하는 방식은 성능 저하를 유발할 수 있습니다.

  3. 입력 검증: newIds 파라미터에 대한 추가적인 검증이 필요할 수 있습니다. 예를 들어, ID가 음수인지, 혹은 형식이 올바른지를 체크하는 로직이 추가된다면 보다 안전한 데이터 처리가 가능할 것입니다.

  4. API 문서화: 새로운 메서드 v2updateInerestedMajorDepartments에 대한 문서가 없다면, 다른 개발자들이 이 메서드를 어떻게 사용할지에 대한 이해가 부족할 수 있습니다. 사용 예시나 API 설명이 필요합니다.

  5. 메서드 이름: v2updateInerestedMajorDepartments는 오타가 있는 것으로 보입니다. 여기서 'Inerested'는 'Interested'로 수정하는 것이 좋습니다. 메서드 이름은 가독성을 위해 간결하고 명확해야 합니다.

  6. 타입 안정성: newIdsnumber[] 타입으로 지정되어 있습니다. 하지만, 배열에 포함된 값이 всегда 확인되기 전에는 Rust나 TypeScript의 Strict mode에서 컴파일 에러가 발생할 수 있습니다. 메서드에 들어오는 인자에 대한 타입 스크립트 검증을 더욱 철저히 하는 것이 좋습니다.

}

async getRelatedLectureById(lectureId: number): Promise<LectureBasic[]> {
async getRelatedLectureById(lectureId: number, courseId: number): Promise<LectureBasic[]> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 코드 패치는 전반적으로 잘 작성되었지만 몇 가지 주의할 점과 개선 사항이 있습니다.

  1. 타입 안전성: filters 배열의 각 필터가 object 타입으로 명시되어 있지만, 각 필터가 null이 아닐 것이라는 보장이 없습니다. 이를 해결하기 위해 각 필터가 특정 인터페이스를 따르도록 설정하는 것이 좋습니다.

  2. 에러 처리: findMany 호출에 대한 에러 처리가 없습니다. 데이터베이스 쿼리에서 오류가 발생할 경우 사용자에게 적절한 피드백을 주기 위해 try-catch 블록을 사용해 에러를 처리하는 것이 좋습니다.

  3. Query 성능: 필터가 많을 경우 AND 조건이 여러 개가 되는 상황이 발생할 수 있습니다. 이 경우 쿼리의 성능에 영향을 미칠 수 있으므로 적절한 인덱싱을 고려해야 합니다.

  4. 상수화: DEFAULT_LIMIT, DEFAULT_ORDER, researchTypes와 같은 상수 값을 분리하여 상단 혹은 별도의 설정 파일에 두는 것이 코드를 더 깔끔하게 유지하는 데 도움이 됩니다.

  5. 코드 가독성: filters를 구성하는 부분에서 각 필터의 가독성을 높이기 위해 추가적인 주석을 포함하는 것이 좋습니다. 이는 향후 코드 유지 보수 시 이해하는 데 도움이 됩니다.

이와 같은 사항들을 고려한다면 코드의 안정성과 유지보수성을 높일 수 있습니다.

day?: number
begin?: number
end?: number
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 리뷰

추가된 v2LectureQuery 인터페이스에 대한 리뷰입니다.

  1. 필드 명세: keyword, type, department, level, limit, year, semester, day, begin, end가 추가되었습니다. 각 필드는 적절하게 선택된 것으로 보이나, 데이터 타입에 대한 명확한 주석이나 설명이 없으면 사용자가 이해하는 데 어려움이 있을 수 있습니다.

  2. 데이터 유효성 검사: 추가된 필드들에 대한 유효성 검사 로직이 부족해 보입니다. 예를 들어, limit는 양수여야 할 수도 있으며, year는 특정 범위로 한정될 수 있습니다. 이러한 유효성 검사를 추가하는 것이 좋습니다.

  3. 일관성 문제: LectureQueryv2LectureQuery 간의 필드 일관성이 부족합니다. 공통 필드가 같으면 중복을 피하고, 필요한 공통 인터페이스를 생성하는 것이 좋습니다. 예를 들어, beginend가 두 인터페이스에 모두 존재하는데, 서로 다른 방식으로 정의될 경우 혼란을 초래할 수 있습니다.

  4. 테스트 부족: 새로운 인터페이스가 추가되었으므로 이에 대한 유닛 테스트를 작성하는 것이 필요합니다. 데이터가 올바르게 처리되고 필드가 예상대로 작동하는지 확인하는 테스트 커버리지를 확보하는 것이 중요합니다.

  5. 기타 제안: 인터페이스의 목적과 각 필드의 의도를 명확히하기 위한 주석을 추가하는 것도 고려해보세요. 불필요한 필드가 있거나 사용되지 않는 필드가 있는지 확인하는 것도 중요합니다.

Comment thread package.json
"@types/node": "^22.15.17",
"@types/supertest": "^6.0.2",
"@typescript-eslint/eslint-plugin": "^8.19.0",
"@typescript-eslint/parser": "^8.19.0",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드 패치에서 몇 가지 개선 사항과 잠재적인 버그 위험이 있습니다.

  1. 경로 변경 주의: openapi-generator.ts 파일의 경로가 변경되었습니다. tsx apps/server/docs/openapi-generator.ts에서 tsx ./apps/server/docs/openapi-generator.ts로 수정되었는데, 변경된 경로에 파일이 존재하는지 확인이 필요합니다. 상대 경로가 아닌 절대 경로 사용을 고려할 수 있습니다.

  2. 종속성 업데이트: @types/node@sentry/nestjs 업데이트가 포함되어 있습니다. 새로운 버전에서의 변경 사항이 코드에 영향을 미치지 않는지 검증해야 합니다.

  3. 중복 종속성: @sentry/nestjs@sentry/node 모두 9.18.0版本으로 업데이트 되었습니다. 이 같은 종속성이 서로 의존 관계를 가지거나 중복으로 포함되어 문제를 일으키지 않도록 검토가 필요합니다.

  4. 버전 관리: 패키지들의 버전을 업데이트하기 전에 각 패키지가 다른 패키지와의 호환성에 충분한 테스트를 받았는지 확인하는 것이 좋습니다. 특히 typescript-eslint과 같은 Lint 패키지가 기존 코드와 충돌하지 않는지 주의해야 합니다.

종합적으로 이 패치는 적용하기 전에 테스트와 검증이 필요합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants