π Fix: 3μ°¨ μ€νλ¦°νΈ κ°μ μ μ μμ νλ€#162
Conversation
Walkthroughκ΄λ¦¬μ μ ν리μΌμ΄μ λͺ©λ‘μ λ€μ€ νν°(organizationRoleIds, statuses, keyword)μ κΈ°λ³Έ μ λ ¬ λ³κ²½(LATEST/DESC)μ λμ νκ³ , Excel λ΄λ³΄λ΄κΈ°(Workbook μμ±Β·μ€νΈλ¦¬λ°) κΈ°λ₯ λ° Apache POI μμ‘΄μ±μ μΆκ°νμ΅λλ€. νκ°/μ§μμ λ°°μΉ μ‘°νμ© λ ν¬μ§ν 리 λ©μλμ ν νλ¦Ώ μμ APIΒ·μλΉμ€Β·μν°ν° μ λ°μ΄νΈλ μΆκ°λμμ΅λλ€. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin Client
participant Controller as AdminApplicationController
participant Service as ApplicationServiceImpl
participant AppRepo as Application Repository
participant AvailRepo as ApplicantAvailability Repository
participant EvalRepo as Evaluation Repository
participant Exporter as ExcelExporter
participant Response as HttpServletResponse
Admin->>Controller: GET /admin/.../applications/excel (recruitmentId, stage, sort, filters)
Controller->>Service: getAllDetailForExcel(recruitmentId, stage, sortBy, direction, organizationRoleIds, statuses, keyword, userId)
Service->>AppRepo: findByRecruitmentIdAndStage(...)
AppRepo-->>Service: List<Application>
Service->>AvailRepo: findAllByApplicationIdIn(appIds)
AvailRepo-->>Service: List<ApplicantAvailability>
Service->>EvalRepo: findAllByApplicationIdIn(appIds)
EvalRepo-->>Service: List<Evaluation>
Service->>Service: νν°Β·μ λ ¬ μ μ©, DTO λ§€ν (toDetail)
Service-->>Controller: List<ApplicationResponseDTO.Detail>
Controller->>Exporter: createExcel(detailList)
Exporter-->>Controller: Workbook
Controller->>Response: set headers & write workbook bytes
Response-->>Admin: Excel νμΌ μλ΅
sequenceDiagram
participant Admin as Admin Client
participant Controller as AdminApplicationController
participant Service as ApplicationServiceImpl
participant AppRepo as Application Repository
Admin->>Controller: GET /admin/.../applications/list (recruitmentId, stage, sort, pageable, filters)
Controller->>Service: getByRecruitmentIdForAdmin(recruitmentId, stage, pageable, sortBy, direction, organizationRoleIds, statuses, keyword)
Service->>AppRepo: findByRecruitmentIdAndStage(...)
AppRepo-->>Service: List<Application>
Service->>Service: νν° μ μ© (organizationRoleIds, statuses, keyword)
Service->>Service: μ λ ¬ μ μ© (LATEST λ±) -> νμ΄μ§ λ° λ¨κ³λ³ μΉ΄μ΄νΈ μμ±
Service-->>Controller: AdminPageWithStageCounts
Controller-->>Admin: νμ΄μ§λ μλ΅
Estimated code review effortπ― 4 (Complex) | β±οΈ ~45 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touchesβ Failed checks (1 warning, 1 inconclusive)
β Passed checks (1 passed)
β¨ Finishing touches
π§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
π§Ή Nitpick comments (7)
src/main/java/KUSITMS/WITHUS/domain/application/applicantAvailability/repository/ApplicantAvailabilityRepository.java (1)
12-12: LGTM! Excel λ΄λ³΄λ΄κΈ°λ₯Ό μν λ°°μΉ μ‘°ν λ©μλκ° μ¬λ°λ₯΄κ² μΆκ°λμμ΅λλ€.λ©μλ μκ·Έλμ²κ° Spring Data JPA κ·μΉμ λ°λ₯΄κ³ μμΌλ©°, μ¬λ¬ Application IDλ‘ ApplicantAvailabilityλ₯Ό μ‘°ννλ κΈ°λ₯μ΄ μ ννκ² κ΅¬νλμμ΅λλ€.
λ€λ§, κΈ°μ‘΄ λ©μλλ€(
findByApplicationIn,findByApplicationId)μ "All" μ λμ¬λ₯Ό μ¬μ©νμ§ μλ λ°λ©΄, μλ‘ μΆκ°λ λ©μλλfindAllBy...λ₯Ό μ¬μ©νμ¬ λ€μ΄λ° μΌκ΄μ±μ΄ λ€μ λ¨μ΄μ§λλ€. Spring Data JPAμμ "All"μ μ νμ μ΄λ©° λμμλ μν₯μ μ£Όμ§ μμ§λ§, μΌκ΄μ±μ μν΄ λ€μ μ€ νλλ₯Ό κ³ λ €ν΄λ³Ό μ μμ΅λλ€:- List<ApplicantAvailability> findAllByApplicationIdIn(List<Long> appIds); + List<ApplicantAvailability> findByApplicationIdIn(List<Long> appIds);src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationService.java (1)
10-30: κ΄λ¦¬μ μ‘°ν/μμ λ€μ΄λ‘λμ© νν° νλΌλ―Έν° νμ₯ μ μ ,idλͺ νν μ μκ΄λ¦¬μ 리μ€νΈ/μμ κ²½λ‘ λͺ¨λμμ λμΌν νν°(μ‘°μ§ μν , μν, ν€μλ)λ₯Ό λ°λλ‘ μκ·Έλμ²λ₯Ό νμ₯ν μ μ μꡬμ¬νκ³Ό μ λ§μ΅λλ€. λ€λ§
getAllDetailForExcelμ λ§μ§λ§ μΈμ μ΄λ¦μ΄ λ¨μidλΌμ,currentUserIdλadminIdλ± μλ―Έκ° λλ¬λλ μ΄λ¦μΌλ‘ λ°κΎΈλ©΄ μ΄ν μ μ§λ³΄μ μ νΌλμ μ€μ΄λ λ° λμμ΄ λ κ² κ°μ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationRepositoryImpl.java (1)
88-92: JPA λ ν¬μ§ν 리 μμ ꡬνμ μ μ , λλ μ‘°ν μ νμ₯ μ¬μ§
findAllByApplicationIdInμ JPA λ ν¬μ§ν 리μ λ¨μ μμνλ ꡬνμ κΉλνκ³ μ΄ν΄νκΈ° μ½μ΅λλ€. ν₯ν μμ λ€μ΄λ‘λ λ±μμ λ§€μ° λ§μ μ§μμ IDμ λν΄ μ¬μ©λλ€λ©΄, λ³λ νμ΄μ§ 쿼리λ μ€νΈλ¦¬λ° λ°©μμΌλ‘ νμ₯ν μ μλλ‘λ§ μΌλμ λμλ©΄ λ κ² κ°μ΅λλ€.src/main/java/KUSITMS/WITHUS/global/util/excel/ExcelExporter.java (1)
40-45: μ«μ νλμ null μ²λ¦¬ κ³ λ €
documentAverageScore,interviewAverageScore,acquaintanceCountλ±μ νλκ°Doubleμ΄λIntegerλνΌ νμ μ΄κ³ nullμΌ κ²½μ°,setCellValue()μμ unboxing μ NPEκ° λ°μν μ μμ΅λλ€. κΈ°λ³Έκ° μ²λ¦¬λ₯Ό κ³ λ €ν΄ λ³΄μΈμ.- row.createCell(8).setCellValue(dto.documentAverageScore()); - row.createCell(9).setCellValue(dto.interviewAverageScore()); + row.createCell(8).setCellValue(dto.documentAverageScore() != null ? dto.documentAverageScore() : 0.0); + row.createCell(9).setCellValue(dto.interviewAverageScore() != null ? dto.interviewAverageScore() : 0.0); row.createCell(10).setCellValue(dto.address()); row.createCell(11).setCellValue(dto.birthDate() != null ? dto.birthDate().toString() : ""); row.createCell(12).setCellValue(dto.university()); - row.createCell(13).setCellValue(dto.acquaintanceCount()); + row.createCell(13).setCellValue(dto.acquaintanceCount() != null ? dto.acquaintanceCount() : 0);src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
62-63: Swagger@Operationμ΄λ Έν μ΄μ λλ½λ€λ₯Έ μλν¬μΈνΈλ€κ³Ό λ¬λ¦¬
downloadExcelλ©μλμ@Operationμ΄λ Έν μ΄μ μ΄ μμ΄ API λ¬Έμνκ° λλ½λ©λλ€.@GetMapping("/recruitment/{recruitmentId}/excel") + @Operation(summary = "μ§μμ μμ λ€μ΄λ‘λ", description = "κ³΅κ³ λ³ μ§μμ λͺ©λ‘μ νν° μ‘°κ±΄μ λ°λΌ μμ νμΌλ‘ λ€μ΄λ‘λν©λλ€.") public void downloadExcel(src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (2)
324-379: Comparator λ΄μμ DTO μμ±μ μ±λ₯ μ νλ₯Ό μ λ°ν©λλ€.
allApps.sort()λΉκ΅ ν¨μ λ΄μμ λ§€λ²SummaryForAdmin.from()μ νΈμΆνκ³ μμ΅λλ€. μ λ ¬ μκ³ λ¦¬μ¦μ O(n log n) λΉκ΅λ₯Ό μννλ―λ‘, nκ°μ μμμ λν΄ μλ§ λ²μ DTO μμ±μ΄ λ°μν μ μμ΅λλ€.λν,
Double.parseDouble()μ΄ μ ν¨νμ§ μμ λ¬Έμμ΄μ λν΄NumberFormatExceptionμ λμ§ μ μμ΅λλ€.μ λ ¬ μ μ DTOλ₯Ό ν λ²λ§ μμ±νκ±°λ, μ λ ¬μ νμν κ°λ§ 미리 μΆμΆνλ λ°©μμ κΆμ₯ν©λλ€:
+ @NotNull + private static List<Application> getSortedApps(AdminApplicationSortField sortBy, Sort.Direction direction, List<Application> allApps) { + // μ λ ¬ μ μ DTOλ₯Ό 미리 μμ±νμ¬ MapμΌλ‘ μΊμ± + Map<Long, ApplicationResponseDTO.SummaryForAdmin> dtoCache = allApps.stream() + .collect(Collectors.toMap(Application::getId, a -> ApplicationResponseDTO.SummaryForAdmin.from(a, 0L))); + allApps.sort((a, b) -> { - var sa = ApplicationResponseDTO.SummaryForAdmin.from(a, 0L); - var sb = ApplicationResponseDTO.SummaryForAdmin.from(b, 0L); + var sa = dtoCache.get(a.getId()); + var sb = dtoCache.get(b.getId()); // ... λλ¨Έμ§ λ‘μ§ λμΌ
469-478: νν°λ§ λ‘μ§ μ€λ³΅ - κ³΅ν΅ λ©μλ μΆμΆ κΆμ₯
getByRecruitmentIdForAdminκ³ΌgetAllDetailForExcelλ©μλμμorganizationRoleIds,statuses,keywordνν°λ§ λ‘μ§μ΄ κ±°μ λμΌνκ² λ°λ³΅λ©λλ€. μ μ§λ³΄μμ±μ μν΄ κ³΅ν΅ private λ©μλλ‘ μΆμΆνλ κ²μ κΆμ₯ν©λλ€.private List<Application> applyFilters( List<Application> apps, List<Long> organizationRoleIds, List<ApplicationStatus> statuses, String keyword ) { Stream<Application> stream = apps.stream(); if (organizationRoleIds != null && !organizationRoleIds.isEmpty()) { stream = stream.filter(app -> app.getOrganizationRole() != null && organizationRoleIds.contains(app.getOrganizationRole().getId())); } if (statuses != null && !statuses.isEmpty()) { stream = stream.filter(app -> statuses.contains(app.getStatus())); } if (keyword != null && !keyword.isBlank()) { String lower = keyword.toLowerCase(); stream = stream.filter(app -> app.getName() != null && app.getName().toLowerCase().contains(lower)); } return stream.collect(Collectors.toList()); }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (15)
build.gradle(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/applicantAvailability/repository/ApplicantAvailabilityRepository.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/enumerate/AdminApplicationSortField.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationService.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java(4 hunks)src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationJpaRepository.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationRepository.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationRepositoryImpl.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/controller/TemplateController.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/dto/TemplateRequestDTO.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/entity/Template.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateService.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java(1 hunks)src/main/java/KUSITMS/WITHUS/global/util/excel/ExcelExporter.java(1 hunks)
π§° Additional context used
𧬠Code graph analysis (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
src/main/java/KUSITMS/WITHUS/global/util/excel/ExcelExporter.java (1)
ExcelExporter(9-51)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-docker-image
π Additional comments (10)
build.gradle (1)
78-80: Excel μμ‘΄μ± μΆκ° μ μ κ΄λ¦¬μ μμ λ€μ΄λ‘λ κΈ°λ₯μ μν΄
org.apache.poi:poi-ooxml:5.2.3μ μΆκ°ν μ νμ μμ°μ€λ½κ³ , νμ¬λ‘μλ λ³λ€λ₯Έ λ¬Έμ μμ΄ λ³΄μ λλ€.src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationRepository.java (1)
21-21: μ¬λ¬ μ§μμ IDλ‘ νκ° μΌκ΄ μ‘°ν λ©μλ μΆκ° μ μ
findAllByApplicationIdInμΆκ°λ λλ©μΈ μꡬ(μ¬λ¬ μ§μμμ λν νκ° μΌκ΄ μ‘°ν)μ μμ°μ€λ½κ³ , κΈ°μ‘΄existsByApplicationId...ν¨ν΄κ³Όλ μΌκ΄μ μ΄λΌ μ΄ν΄νκΈ° μ½μ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluation/repository/EvaluationJpaRepository.java (1)
17-17: νμ 쿼리 λ©μλ μΆκ° λ¬Έμ μμ
findAllByApplicationIdInνμ 쿼리 λ©μλ μΆκ°κ° μμ μΈν°νμ΄μ€/ꡬν체μ μ λ§κ³ , κΈ°μ‘΄ λ©μλ λ€μ΄λ° μ€νμΌκ³Όλ μΌκ΄μ μ λλ€.src/main/java/KUSITMS/WITHUS/domain/application/application/enumerate/AdminApplicationSortField.java (1)
4-4: μ λ ¬ νλμ LATEST μΆκ° μ μ , λ§€ν κΈ°μ€λ§ ν λ² λ νμΈ νμκ΄λ¦¬μ νλ©΄μμ μ΅μ μ μ λ ¬ μ΅μ μ μ 곡νκΈ° μν΄
LATESTμμλ₯Ό μΆκ°ν λΆλΆμ UX κ΄μ μμ μμ°μ€λ½μ΅λλ€. λ€λ§ μλΉμ€/쿼리 ꡬνλΆμμLATESTκ° μμ±μΌ, μ§μ ID μμ λ± μ΄λ€ μ»¬λΌ κΈ°μ€μΌλ‘ λ§€νλμλμ§ ν λ² λ μ κ²ν΄ μ£ΌμΈμ(μΆν μ λ ¬ κΈ°μ€ λ³κ²½ μμλ enum μ΄λ¦κ³Ό μλ―Έκ° μ΄κΈλμ§ μλλ‘).src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateService.java (1)
14-15: ν νλ¦Ώ μμ κΈ°λ₯μ μν Service μΈν°νμ΄μ€ νμ₯ μ μ
createμ λμΌνκ²Detailμ λ°ννλupdateλ©μλ μΆκ°λ‘ ν νλ¦Ώ CRUD νλ¦μ΄ κΉλν΄μ‘μ΅λλ€. νμ¬ κ΅¬ν/컨νΈλ‘€λ¬/DTOμ μκ·Έλμ²κ° μΌκ΄λλ€λ©΄ μΆκ° μμ μμ΄λ 무리 μμ κ² κ°μ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/template/controller/TemplateController.java (1)
54-62: LGTM!μλν¬μΈνΈ ꡬνμ΄ κΈ°μ‘΄ ν¨ν΄μ λ°λ₯΄κ³ μμΌλ©°,
@Validλ₯Ό ν΅ν μ λ ₯ κ²μ¦κ³Ό Swagger λ¬Έμνκ° μ μ νκ² μ μ©λμ΄ μμ΅λλ€. μλΉμ€ λ μ΄μ΄μμ κΆν κ²μ¦μ΄ μΆκ°λλ€λ©΄ μμ ν ꡬνμ΄ λ κ²μ λλ€.src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
45-60: LGTM!νν° νλΌλ―Έν° μΆκ°μ κΈ°λ³Έ μ λ ¬ μμ λ³κ²½(LATEST/DESC)μ΄ PR μꡬμ¬νμ λ§κ² μ ꡬνλμμ΅λλ€. μ νμ νλΌλ―Έν°λ‘ μ€μ νμ¬ κΈ°μ‘΄ νΈνμ±λ μ μ§λ©λλ€.
src/main/java/KUSITMS/WITHUS/global/util/excel/ExcelExporter.java (1)
36-45: μν νλμ NPE κ°λ₯μ±μ μμ΅λλ€.
statusνλλ Application μν°ν°μμ@Column(nullable = false)μ μ½μ‘°κ±΄μ΄ μμΌλ―λ‘ nullμ΄ λ μ μμ΅λλ€. λ°λΌμdto.status().name()μ NPEλ₯Ό λ°μμν€μ§ μμ΅λλ€.genderμbirthDateλ NOT NULL μ μ½μ΄ μλ nullable νλμ΄κΈ° λλ¬Έμ null 체ν¬κ° νμνμ§λ§,statusλ λ°μ΄ν°λ² μ΄μ€ μ μ½μ μν΄ νμ κ°μ κ°μ§λλ€.Likely an incorrect or invalid review comment.
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (2)
512-555: N+1 λ¬Έμ ν΄κ²°μ μν bulk μ‘°ν μ΅μ ν - μ’μ΅λλ€.λ©΄μ κ°λ₯ μκ°κ³Ό νκ° λ°μ΄ν°λ₯Ό
findAllByApplicationIdInμΌλ‘ bulk μ‘°ννμ¬ N+1 λ¬Έμ λ₯Ό λ°©μ§ν μ μ΄ μ’μ΅λλ€. Mapμ νμ©ν κ·Έλ£Ήνλ μ μ ν©λλ€.
368-370: LATEST μ λ ¬ μΌμ΄μ€ μΆκ° - LGTM
createdAtκΈ°μ€ μ λ ¬μ΄ μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (1)
324-379: μ λ ¬ Comparatorκ° βλ¬Έμβμ«μ νμ±/λ κ°λ₯ νλ λΉκ΅βμ μ·¨μ½ν©λλ€
DOCUMENT_SCORE/INTERVIEW_SCOREμμDouble.parseDouble(...)μ΄ λΉ λ¬Έμμ΄/λΉμ μ κ°μ΄λ©΄NumberFormatExceptionμ΄ λ μ μκ³ ,NAME/POSITION_NAMEλ nullμ΄λ©΄ NPEκ° κ°λ₯ν©λλ€(DTO νλκ° νμ non-nullμ΄λΌλ 보μ₯μ΄ μμΌλ©΄). μ΅μν μμ νμ±/λ μΈμ΄ν λΉκ΅λ₯Ό λ£λ νΈμ΄ μ’μ΅λλ€.- case DOCUMENT_SCORE: - cmp = Double.compare( - Double.parseDouble(sa.documentAverageScore()), - Double.parseDouble(sb.documentAverageScore()) - ); + case DOCUMENT_SCORE: + cmp = Double.compare( + safeParseDouble(sa.documentAverageScore()), + safeParseDouble(sb.documentAverageScore()) + ); break; case INTERVIEW_SCORE: - cmp = Double.compare( - Double.parseDouble(sa.interviewAverageScore()), - Double.parseDouble(sb.interviewAverageScore()) - ); + cmp = Double.compare( + safeParseDouble(sa.interviewAverageScore()), + safeParseDouble(sb.interviewAverageScore()) + ); break; case POSITION_NAME: - cmp = sa.organizationRoleName().compareToIgnoreCase(sb.organizationRoleName()); + cmp = nullSafeString(sa.organizationRoleName()) + .compareToIgnoreCase(nullSafeString(sb.organizationRoleName())); break; ... case NAME: default: - cmp = sa.name().compareToIgnoreCase(sb.name()); + cmp = nullSafeString(sa.name()) + .compareToIgnoreCase(nullSafeString(sb.name())); break;(μ diffλ₯Ό μ μ©νλ €λ©΄ νμΌ λ΄μ
safeParseDouble,nullSafeStringν¬νΌ λ©μλ μΆκ°κ° νμν©λλ€.)Also applies to: 327-377
β»οΈ Duplicate comments (3)
src/main/java/KUSITMS/WITHUS/domain/template/entity/Template.java (1)
49-57: MAIL ν νλ¦Ώ μ λ°μ΄νΈμλ μ λͺ© λΆλ³μ‘°κ±΄(Subject required) μ μ©ν μ μ’μ΅λλ€.
μμ±μμ λμΌν κ²μ¦μupdate()μλ λ£μ΄μ λλ©μΈ κ·μΉμ΄ μ λ°μ΄νΈ μμ μ κΉ¨μ§λ λ¬Έμ λ₯Ό λ§μμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java (1)
65-80: ν νλ¦Ώ μμ κΆν(μ‘°μ§ μμ κΆ) κ²μ¦μ΄ TODOλ‘ λ¨μ μμ΄ IDOR μ·¨μ½μ μ΄ λ©λλ€.
listAll()μ²λΌ βνμ¬ μ¬μ©μκ° μν μ‘°μ§μ ν νλ¦Ώλ§β μμ κ°λ₯νλλ‘, μ‘°ν λ¨κ³μμλΆν° μ‘°μ§ λ²μλ₯Ό κ±Έμ΄λκ±°λ(κΆμ₯), μ‘°ν ν μ‘°μ§ μΌμΉ κ²μ¦μ λ°λμ μΆκ°ν΄μΌ ν©λλ€.@@ - public TemplateResponseDTO.Detail update(Long templateId, TemplateRequestDTO.Update dto) { - Template template = templateRepository.getById(templateId); - - // TODO: μ¬μ©μκ° μν μ‘°μ§μ ν νλ¦Ώλ§ μμ νλλ‘ κ²μ¦ μΆκ° νμ + public TemplateResponseDTO.Detail update(Long templateId, TemplateRequestDTO.Update dto, User user) { + List<Long> organizationIds = user.getUserOrganizations().stream() + .map(uo -> uo.getOrganization().getId()) + .toList(); + + // κΆμ₯: repositoryμμ "id + organizationIds"λ‘ ν¨κ» μ‘°νλκ² ν΄μ μ°ν μ체λ₯Ό μ°¨λ¨ + Template template = templateRepository.getById(templateId); + if (!organizationIds.contains(template.getOrganization().getId())) { + // TODO: νλ‘μ νΈμ νμ€ κΆν μμΈ/μλ¬μ½λλ‘ κ΅μ²΄ + throw new CustomException(ErrorCode.FORBIDDEN); + } @@ template.update( dto.name(), dto.subject(), dto.body(), dto.medium() );src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (1)
253-322: (μ΄μ μ§μ λ°μ) νν° ν mutable list μ μ§ + name null-guard μ²λ¦¬ μ’μμ
Stream.toList()λμcollect(Collectors.toList())λ‘ μ λ ¬ μUnsupportedOperationException리μ€ν¬λ₯Ό μ κ±°νκ³ , ν€μλ νν°μμgetName()null 체ν¬λ λ€μ΄κ°μ΅λλ€.Also applies to: 266-292
π§Ή Nitpick comments (3)
src/main/java/KUSITMS/WITHUS/domain/template/dto/TemplateRequestDTO.java (1)
29-42:subjectμ μ‘°κ±΄λΆ νμ(= medium==MAILμΌ λλ§) κ²μ¦μ μμ² DTO λ¨κ³μμλ μ 리νλ κ² μ’μ΅λλ€.
νμ¬λ μν°ν°μμλ§ λ§νλ―λ‘, API κ΄μ μμ 4xx μλ΅/λ©μμ§ μΌκ΄μ±μ μν΄mediumκ°μ λ°λ₯Έ μ‘°κ±΄λΆ κ²μ¦(컀μ€ν validator λλ DTO compact constructorμμ 체ν¬)μ κ³ λ €ν΄ μ£ΌμΈμ.src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (1)
284-292: ν€μλ νν°μμtrim()μ μ€μ λ§€μΉμλ μ μ©νλ κ² μμ ν©λλ€
νμ¬λ 곡백 μ¬λΆ 체ν¬λ§trim()μ μ°κ³ , μ€μ λΉκ΅λ μλ¬Έkeywordλ‘ λ΄λ €κ°μ" kim "κ°μ μ λ ₯μ΄ κΈ°λμ λ€λ₯΄κ² λμν μ μμ΄μ.- if (keyword != null && !keyword.trim().isEmpty()) { - String lower = keyword.toLowerCase(); + if (keyword != null && !keyword.trim().isEmpty()) { + String lower = keyword.trim().toLowerCase(Locale.ROOT); allApps = allApps.stream() .filter(app -> app.getName() != null && app.getName().toLowerCase().contains(lower) ) .collect(Collectors.toList()); }src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
62-94: μμ λ€μ΄λ‘λ: λμ©λ λ°μ΄ν°λ©΄ λ©λͺ¨λ¦¬ μ¬μ©λ(Workbook) 리μ€ν¬κ° ν½λλ€
ExcelExporter.createExcelκ°XSSFWorkbookκΈ°λ°μ΄λ©΄ λ°μ΄ν°κ° λ§μ λ νμ ν¬κ² μ‘μλ¨Ήμ μ μμ΄μ, νμνλ©΄SXSSFWorkbook(μ€νΈλ¦¬λ°) μ νμ κ²ν νλ κ² μμ ν©λλ€.Also applies to: 75-93
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (5)
src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java(4 hunks)src/main/java/KUSITMS/WITHUS/domain/template/dto/TemplateRequestDTO.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/entity/Template.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java(1 hunks)
π§° Additional context used
𧬠Code graph analysis (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
src/main/java/KUSITMS/WITHUS/global/util/excel/ExcelExporter.java (1)
ExcelExporter(9-51)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-docker-image
π Additional comments (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
45-60: κ΄λ¦¬μ λͺ©λ‘ API: κΈ°λ³Έ μ λ ¬μ LATEST/DESCλ‘ λ°κΎΌ 건 μꡬμ¬νμ λΆν©ν©λλ€
νν° νλΌλ―Έν°λ μλΉμ€λ‘ μ μ λ¬λκ³ μμ΄μ.
| import KUSITMS.WITHUS.global.infra.upload.dto.FileResponseDTO; | ||
| import KUSITMS.WITHUS.global.infra.upload.service.FileUploadService; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.jetbrains.annotations.NotNull; |
There was a problem hiding this comment.
org.jetbrains.annotations.NotNull λμ
μ νλ‘μ νΈ νμ€κ³Ό μΌμΉνλμ§ νμΈ νμ
μλ² μ½λ μ λ°μμ @NotNullμ μ΄λ€ μ΄λ
Έν
μ΄μ
(μ: jakarta.validation.constraints.NotNull, org.springframework.lang.NonNull)λ‘ ν΅μΌνλμ§μ λ°λΌ μΌκ΄μ±μ΄ κΉ¨μ§ μ μμ΅λλ€.
π€ Prompt for AI Agents
In
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java
around line 49, the file imports org.jetbrains.annotations.NotNull which may not
match the project's standard annotation; replace this import with the project's
agreed-upon nullability/validation annotation (e.g.,
jakarta.validation.constraints.NotNull or org.springframework.lang.NonNull) or
remove the import if unused, and update any @NotNull usages in this class to the
chosen annotation to keep consistency across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
β»οΈ Duplicate comments (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (1)
49-49: μ΄μ 리뷰μμ μ§μ λ@NotNullμ΄λ Έν μ΄μ μ΄μκ° μ¬μ ν λ¨μμμ΅λλ€.νλ‘μ νΈ μ λ°μ νμ€ μ΄λ Έν μ΄μ (μ:
jakarta.validation.constraints.NotNullλλorg.springframework.lang.NonNull)κ³Ό μΌμΉνλμ§ νμΈμ΄ νμν©λλ€.
π§Ή Nitpick comments (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (1)
324-379: μ λ ¬ λ‘μ§μ΄ μ€μνλμ΄ μ½λ μ¬μ¬μ©μ±μ΄ ν₯μλμμ΅λλ€.μλ‘ μΆκ°λ
LATESTμΌμ΄μ€(Lines 368-370)λcreatedAtμ κΈ°μ€μΌλ‘ μ λ ¬νλ©°,DESCλ°©ν₯μΌ λ μ΅μ μμΌλ‘ μ λ ¬λ©λλ€.μ°Έκ³ : μ΄ λ©μλλ μ λ ₯ 리μ€νΈλ₯Ό μ μ리μμ μ λ ¬ν ν λμΌν 리μ€νΈλ₯Ό λ°νν©λλ€. μ΄λ κΈ°λ₯μ μΌλ‘λ λ¬Έμ μμ§λ§, λ°νλ¬Έμ μ κ±°νκ³
voidλ‘ λ³κ²½νλ κ²μ΄ μλλ₯Ό λ λͺ νν ν μ μμ΅λλ€.Optional: λ©μλλ₯Ό voidλ‘ λ³κ²½
- @NotNull - private static List<Application> getSortedApps(AdminApplicationSortField sortBy, Sort.Direction direction, List<Application> allApps) { + private static void sortApps(AdminApplicationSortField sortBy, Sort.Direction direction, List<Application> allApps) { allApps.sort((a, b) -> { // ... sorting logic ... }); - return allApps; }κ·Έλ¦¬κ³ νΈμΆ λΆλΆμ λ€μκ³Ό κ°μ΄ λ³κ²½:
- List<Application> sortedApps = getSortedApps(sortBy, direction, allApps); + sortApps(sortBy, direction, allApps);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java(4 hunks)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-docker-image
π Additional comments (2)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (2)
258-292: μ΄μ 리뷰μμ μ§μ λ λΆλ³ 리μ€νΈ λ° NPE μ΄μκ° λͺ¨λ μμ λμμ΅λλ€.
collect(Collectors.toList())λ₯Ό μ¬μ©νμ¬ κ°λ³ 리μ€νΈ μμ± βgetName()null μ²΄ν¬ μΆκ° (Line 288) βtrim()λ°Locale.ROOTμ¬μ©μΌλ‘ λ¬Έμμ΄ λΉκ΅ μΌκ΄μ± ν보 βνν°λ§ λ‘μ§μ΄ μ ννκ³ μμ νκ² κ΅¬νλμμ΅λλ€.
469-559: μμ λ΄λ³΄λ΄κΈ° λ©μλκ° μμ νκ² κ΅¬νλμμ΅λλ€.μ΄μ 리뷰μμ μ§μ λ λ¬Έμ λ€μ΄ λͺ¨λ ν΄κ²°λμμ΅λλ€:
getName()null μ²΄ν¬ ν¬ν¨ (Line 504) β- λΉ λ¦¬μ€νΈ μ‘°κΈ° λ°νμΌλ‘ λΉ IN 쿼리 λ°©μ§ (Lines 511-513) β
- Bulk μ‘°νλ₯Ό ν΅ν μ±λ₯ μ΅μ ν (Lines 520-526) β
- κ΄λ¦¬μ λͺ©λ‘ APIμ λμΌν νν°/μ λ ¬ λ‘μ§ μ μ©μΌλ‘ μΌκ΄μ± ν보 β
ꡬνμ΄ μ ννκ³ μ±λ₯ κ³ λ €μ¬νλ μ λ°μλμμ΅λλ€.
β¨ Related Issue
π Task Details
π¬ Review Requirements (Optional)
Summary by CodeRabbit
βοΈ Tip: You can customize this high-level summary in your review settings.