Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
1,147 changes: 777 additions & 370 deletions apps/server/docs/swagger.json

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions apps/server/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@ import { JwtCookieGuard } from './modules/auth/guard/jwt-cookie.guard'
import { MockAuthGuard } from './modules/auth/guard/mock-auth-guard'
import { CoursesModule } from './modules/courses/courses.module'
import { DepartmentsModule } from './modules/departments/departments.module'
import { DepartmentsModuleV2 } from './modules/departments/v2/departments.v2.module'
import { FeedsModule } from './modules/feeds/feeds.module'
import { LecturesModule } from './modules/lectures/lectures.module'
import { NoticesModule } from './modules/notices/notices.module'
import { PlannersModule } from './modules/planners/planners.module'
import { RatesModule } from './modules/rates/rates.module'
import { ReviewsModule } from './modules/reviews/reviews.module'
import { SchedulesModule } from './modules/schedules/schedules.module'
import { SemestersModule } from './modules/semesters/semesters.module'
import { SemestersModuleV2 } from './modules/semesters/v2/semesters.v2.module'
import { SessionModule } from './modules/session/session.module'
import { ShareModule } from './modules/share/share.module'
Copy link

Choose a reason for hiding this comment

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

코드에서 SchedulesModule을 추가하는 것은 적절해 보입니다. 하지만 몇 가지 주의할 사항이 있습니다:

  1. 모듈 의존성 확인: SchedulesModule이 다른 모듈과의 의존성을 가지고 있는지 확인해야 합니다. 만약 의존성이 있다면, 이를 적절하게 처리해야 합니다.

  2. 테스트 누락 가능성: 새로운 모듈을 추가하면 기존 기능에 영향을 줄 수 있습니다. 따라서, 새로운 모듈과 관련된 테스트가 누락되지 않았는지 확인해야 합니다.

  3. 주석 추가: SchedulesModule의 목적과 기능을 코드 주석으로 명확히 설명하는 것이 좋습니다. 이렇게 하면 코드 유지보수 시 더 이해하기 쉬워질 것입니다.

  4. 모듈 통합 체크: 전체 모듈을 통합하여 최종 빌드 또는 배포 전에 해당 모듈이 정상 작동하는지 확인해야 합니다. 이를 위해 CI/CD 파이프라인에서 적절한 빌드 및 테스트 단계를 설정하는 것이 중요합니다.

결론적으로, 코드는 잘 작성된 것 같지만 추가적인 확인과 테스트가 필요합니다.

import { StatusModule } from './modules/status/status.module'
Expand Down Expand Up @@ -83,7 +86,9 @@ async function createCacheStoreWithFallback(): Promise<Keyv> {
LecturesModule,
ReviewsModule,
UserModule,
SchedulesModule,
SemestersModule,
SemestersModuleV2,
TimetablesModule,
TimetablesModuleV2,
RatesModule,
Expand All @@ -93,6 +98,7 @@ async function createCacheStoreWithFallback(): Promise<Keyv> {
NoticesModule,
SessionModule,
DepartmentsModule,
DepartmentsModuleV2,
PlannersModule,
TracksModule,
ShareModule,
Copy link

Choose a reason for hiding this comment

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

코드 리뷰 요약

  1. 의존성 추가: DepartmentsModuleV2SemestersModuleV2가 새롭게 추가되었습니다. 이 모듈들의 내부 구현이 호출될 때 예상치 못한 문제가 발생할 수 있습니다.

    • 버전 관리: 기존 모듈과 V2 모듈의 버전 차별이 명확히 정의되어 있는지 확인하세요. V2 모듈이 기존 모듈과 API 차이를 가질 경우, 호환성 문제를 유발할 수 있습니다.
  2. 에러 핸들링: 해당 모듈의 초기화 또는 사용 중 발생할 수 있는 예외에 대한 핸들링이 필요할 수 있습니다. 특히 캐시 스토어를 다룰 때는 예상치 못한 상태가 발생할 가능성이 있으니 이에 대한 예외 처리를 고려하세요.

  3. 주석 및 문서화: 새롭게 추가된 모듈에 대한 문서화가 부족해 보입니다. 각 모듈의 역할 및 사용법에 대한 주석을 추가하여 코드 가독성을 높이는 것이 좋습니다.

  4. 테스트 케이스: 새로운 모듈에 대한 유닛 테스트가 생성되지 않았다면, 통합 테스트와 유닛 테스트를 작성하여 기존 기능과의 충돌을 방지하는 것이 좋습니다.

이러한 문제점들이 해결된 후에 코드를 병합하는 것이 바람직합니다.

Expand Down
6 changes: 0 additions & 6 deletions apps/server/src/common/interfaces/IDepartmentV2.ts

This file was deleted.

150 changes: 0 additions & 150 deletions apps/server/src/common/interfaces/ILectureV2.ts

This file was deleted.

6 changes: 0 additions & 6 deletions apps/server/src/common/interfaces/IProfessorV2.ts

This file was deleted.

8 changes: 8 additions & 0 deletions apps/server/src/common/interfaces/ISchedules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export namespace ISchedules {
export interface Basic {
year: number
from: Date
to: Date
name: string
}
}
Copy link

Choose a reason for hiding this comment

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

이 코드 패치는 다음과 같은 문제점과 개선사항을 포함하고 있습니다:

  1. 타입 안정성: fromto 속성이 Date 타입으로 정의되어 있지만, 값이 잘못된 형식으로 제공되면 런타임 오류가 발생할 수 있습니다. Date 객체의 유효성을 검증하는 로직을 추가하는 것이 좋습니다.

  2. 이름의 명료성: fromto는 명확하지만, 좀 더 읽기 쉽게 startDateendDate로 이름을 변경하는 것을 고려해 볼 수 있습니다. 이는 코드의 가독성을 향상시킵니다.

  3. 세부사항 부족: 인터페이스에 대한 설명이 없습니다. 각 속성의 목적을 문서화하거나 주석을 추가하여 다른 개발자들이 쉽게 이해할 수 있도록 하는 것이 좋습니다.

  4. 예외 처리: 해당 인터페이스를 사용하는 부분에서 예외 처리를 고려해야 합니다. 특히 날짜와 관련된 작업은 예상치 못한 오류를 초래할 수 있기 때문에 적절한 유효성 검사와 오류 처리가 필요합니다.

  5. 네임스페이스 사용: ES6 모듈 시스템에서 네임스페이스를 사용하는 것은 일반적이지 않습니다. 팀에서 특정 패턴을 따를 필요가 없다면, 그냥 interface를 export하는 것이 더 간단하고 명확할 수 있습니다.

Copy link

Choose a reason for hiding this comment

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

코드 패치에서 몇 가지 개선사항 및 잠재적인 버그가 발견되었습니다. 첫째, fromto 속성이 Date 타입으로 정의되어 있으나, 입력 값이 유효한 날짜인지 확인하는 로직이 없습니다. 이는 런타임 오류를 초래할 수 있습니다. 둘째, fromto의 날짜 범위가 유효한지 (즉, fromto보다 이전인지 확인) 확인하는 검증 로직도 포함하는 것이 좋습니다. 마지막으로, name 속성의 유효성을 검사하여 빈 문자열로 설정되는 것을 방지하는 방법도 검토할 필요가 있습니다. 전체적으로, 이러한 검증 절차를 추가함으로써 코드의 안정성을 높일 수 있습니다.

5 changes: 1 addition & 4 deletions apps/server/src/common/interfaces/index.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
export * from './IAuth'
export * from './ICourse'
export * from './ICourseV2'
export * from './IDepartment'
export * from './IDepartmentV2'
export * from './IFeed'
export * from './ILecture'
Copy link

Choose a reason for hiding this comment

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

변경 사항을 보면, ISchedules 모듈을 추가하고 있습니다. 이 모듈이 신뢰할 수 있는지, 다른 엔트리와의 의존성이나 충돌 가능성이 없는지 확인해야 합니다. 또한, 만약 ISchedules가 이전에 없던 기능을 포함하고 있다면, 이 기능에 대한 테스트가 충분히 이루어졌는지 검토해야 합니다. "export" 행 추가로 인해 프로젝트의 빌드 결과나 동작에 영향을 미칠 수 있으니 주의가 필요합니다. 추가적으로, 모듈의 문서화가 되어 있는지 확인하여 다른 개발자들이 이를 이해하는 데 도움이 되도록 하는 것이 좋습니다.

Copy link

Choose a reason for hiding this comment

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

코드 패치는 새로운 인터페이스인 ISchedules를 가져오고 있습니다. 다음과 같은 점에서 검토가 필요합니다:

  1. 파일 존재 여부: ISchedules 파일이 실제로 존재하는지 확인하세요. 파일이 없어도 코드는 빌드되지 않거나 런타임 오류가 발생할 수 있습니다.
  2. 의존성 관리: 만약 다른 모듈이나 파일에서 ISchedules를 사용할 계획이라면, 해당 모듈들에서의 의존성을 점검해야 합니다. 의존성이 많아지면 코드 유지보수가 어려울 수 있습니다.
  3. 테스트: ISchedules의 기능이 구현된 후, 관련된 테스트 케이스가 추가되어야 합니다. 새로운 인터페이스를 도입함으로써 발생할 수 있는 잠재적인 버그에 대비하기 위해서입니다.
  4. 문서화: ISchedules 인터페이스의 기능에 대한 문서화가 필요합니다. 다른 개발자들이 이 인터페이스를 이해하고 사용할 수 있도록 도와줄 수 있습니다.

이러한 사항들을 고려한 후, 제안된 패치를 재검토해야 합니다.

export * from './ILectureV2'
export * from './INotice'
export * from './IPlanner'
export * from './IProfessor'
export * from './IProfessorV2'
export * from './IRate'
export * from './IReview'
export * from './ISchedules'
export * from './ISemester'
Copy link

Choose a reason for hiding this comment

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

이 코드 패치는 여러 모듈을 내보내는 부분에서 'V2' 버전의 모듈을 제거하였습니다. 여기에는 몇 가지 잠재적인 문제가 있습니다. 첫째, V2 버전을 사용하는 다른 코드가 있을 수 있으며, 이들 코드에서 오류가 발생할 수 있습니다. 둘째, 코드의 의도가 불명확합니다. 왜 V2 버전을 제거했는지 명시적인 주석이나 커밋 메시지가 없습니다. 따라서 코드 변경의 이유를 문서화해 추가적인 혼란을 피하는 것이 좋습니다. 마지막으로, V2와 V1 버전 간의 주요 차이점을 문서화하여 새로운 코드가 이전 버전의 기능을 얼마나 유지하는지 유의해야 합니다. 이러한 사항을 고려하여 패치를 더 면밀히 검토하는 것이 좋습니다.

export * from './ISession'
export * from './IShare'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { ApiPropertyOptional } from '@nestjs/swagger'
import { IProfessorV2 } from '@otl/server-nest/common/interfaces/IProfessorV2'
import { Transform } from 'class-transformer'
import {
IsArray, IsIn, IsInt, IsNumber, IsOptional, IsString,
} from 'class-validator'

import { IDepartmentV2 } from './IDepartmentV2'
import { IProfessorV2 } from './IProfessorV2'

export type CourseOrderQuery = 'code' | 'popular' | 'studentCount'
export type level = 'ALL' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'
Expand Down
6 changes: 6 additions & 0 deletions apps/server/src/common/interfaces/v2/IDepartmentV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,10 @@ export namespace IDepartmentV2 {
id: number
name: string
}

export interface Detail {
id: number
name: string
code: string
}
}
Copy link

Choose a reason for hiding this comment

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

리뷰 코멘트:

  1. 명확한 문서화 부족: Detail 인터페이스의 속성들이 설명 없이 추가되었습니다. 각 속성이 어떤 의미를 가지는지 설명이 필요합니다. 문서화가 없으면 향후 유지보수 시 혼동을 초래할 수 있습니다.

  2. 데이터 타입 검토: code 속성이 추가되었지만, 이 속성의 데이터 타입이 string으로만 정의되어 있습니다. 만약 특정 형식이나 제약이 필요하다면 string 대신 적절한 타입을 사용하는 것이 좋습니다.

  3. 기능적 요구사항 검토: Detail 인터페이스의 목적이 명확하지 않습니다. 이 인터페이스가 어떤 상황에서 사용 되는지 이해할 수 있도록 추가적인 맥락이 필요합니다. 예를 들어, Department와의 관계 또는 이 인터페이스가 데이터를 어떻게 처리하는지에 대한 정보가 필요합니다.

  4. 일관성 있는 네이밍: IDepartmentV2라는 네임스페이스와 Detail이라는 인터페이스 이름이 일관성이 있는지 검토해보세요. 네이밍 컨벤션은 코드의 가독성을 높이는 데 중요한 역할을 합니다.

Loading