π Fix: 3μ°¨ μΆκ° μμ²μ¬νμ μμ νλ€#164
Conversation
β¦λ νκ° κΈ°μ€λ§ μ‘°ν #159
Walkthroughκ΄λ¦¬μμ© μ§μμ κ²μ μλν¬μΈνΈμ ApplicantForMailSms DTOκ° μΆκ°λμκ³ , μ‘°μ§ μν (null) μ²λ¦¬ λ° QueryDSL κΈ°λ° μ‘°ν νμ₯μ΄ μ¬λ¬ μ μ₯μΒ·μλΉμ€μ μ μ©λμμ΅λλ€. ν νλ¦Ώ μ λ°μ΄νΈ/μμ μ μ¬μ©μ κΆν κ²μ¦μ΄ λμ λμμ΅λλ€. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Controller as AdminApplicationController
participant Service as ApplicationService
participant Repo as ApplicationRepository
participant DB
Admin->>Controller: GET /api/v1/admin/applications/recruitment/{id}/search?keyword=...
Controller->>Service: searchApplicantsForMailSms(recruitmentId, keyword)
Service->>Repo: findByRecruitmentIdAndNameOrEmail(recruitmentId, keyword)
Repo->>DB: SELECT ... WHERE recruitment_id=? AND (name ILIKE '%keyword%' OR email ILIKE '%keyword%') / or recruitment + orgRole IS NULL
DB-->>Repo: [Application...]
Repo-->>Service: [Application...]
Service->>Service: map Application -> ApplicantForMailSms
Service-->>Controller: SuccessResponse<List<ApplicantForMailSms>>
Controller-->>Admin: 200 OK
sequenceDiagram
actor User
participant Controller as TemplateController
participant Service as TemplateService
participant Repo as TemplateRepository
participant DB
participant Validator as validateUserHasAccess
User->>Controller: DELETE /templates/{id}
Controller->>Service: delete(templateId, user)
Service->>Repo: getById(templateId)
Repo->>DB: SELECT ... JOIN organization WHERE id=?
DB-->>Repo: Template
Repo-->>Service: Template
Service->>Validator: validateUserHasAccess(template, user)
alt κΆν μμ
Validator-->>Service: OK
Service->>Repo: delete(templateId)
Repo->>DB: DELETE FROM template WHERE id=?
DB-->>Repo: success
Repo-->>Service: success
Service-->>Controller: SuccessResponse("μμ μλ£")
Controller-->>User: 200 OK
else κΆν μμ
Validator-->>Service: throw CustomException(TEMPLATE_NO_PERMISSION)
Service-->>Controller: exception
Controller-->>User: ErrorResponse(TEMPLATE403)
end
Estimated code review effortπ― 3 (μ€κ°) | β±οΈ ~25λΆ
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)
π Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro π Files selected for processing (1)
β° 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)
π Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
π§Ή Nitpick comments (3)
src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/dto/DistributionRequestResponseDTO.java (1)
42-48: Null-safety ꡬνμ΄ μ¬λ°λ¦ λλ€.
organizationRoleμ΄ nullμΌ λ NPEλ₯Ό λ°©μ§νλ λ‘μ§μ΄ μ ννκ² κ΅¬νλμμ΅λλ€. "곡ν΅" μν μ λνλ΄λ κΈ°λ³Έκ° μ²λ¦¬λ PR λͺ©μ κ³Ό μ λΆν©ν©λλ€.π‘ μ νμ κ°μ μ¬ν: λ§€μ§ μ€νΈλ§μ μμλ‘ μΆμΆ
"곡ν΅" λ¬Έμμ΄μ΄ νλμ½λ©λμ΄ μμ΅λλ€. λμΌν κ°μ΄ λ€λ₯Έ κ³³μμλ μ¬μ©λ κ°λ₯μ±μ΄ μλ€λ©΄ μμλ‘ μΆμΆνλ κ²μ κ³ λ €ν΄λ³΄μΈμ.
μμ:
+ private static final String COMMON_ROLE_NAME = "곡ν΅"; + @Schema(description = "νκ° λ΄λΉμ λΆλ°° μ΅μ μ΄λ ₯ 리μ€νΈ DTO") public record Assignment( @Schema(description = "μ§μ μν λͺ ") String organizationRoleName, @Schema(description = "νκ° νμ ") EvaluationType evaluationType, @Schema(description = "μ§μμλΉ λ°°μ ν νκ°μ μ") int count ) { public static Assignment from(DistributionAssignment a) { return new Assignment( - a.getOrganizationRole() != null ? a.getOrganizationRole().getName() : "곡ν΅", + a.getOrganizationRole() != null ? a.getOrganizationRole().getName() : COMMON_ROLE_NAME, a.getEvaluationType(), a.getCount() ); } }src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java (1)
159-167: λλ κ²°κ³Όμ λν νμ΄μ§λ€μ΄μ κ³ λ €νμ¬ μλν¬μΈνΈλ
Listλ₯Ό λ°ννμ¬ λͺ¨λ κ²°κ³Όλ₯Ό ν λ²μ κ°μ Έμ΅λλ€. μ§μμ μκ° λ§μ κ³΅κ³ μ κ²½μ° μ±λ₯ λ¬Έμ κ° λ°μν μ μμ΅λλ€.λ©μΌ/SMS λ°μ‘ λͺ©μ μ μ 체 λͺ©λ‘μ΄ νμνλ€λ©΄ νμ¬ κ΅¬νμ΄ μ ν©νμ§λ§, κ²μ κ²°κ³Όκ° λ§μ κ²μΌλ‘ μμλλ€λ©΄ νμ΄μ§λ€μ΄μ λμ μ κ³ λ €ν΄ λ³΄μΈμ.
src/main/java/KUSITMS/WITHUS/domain/recruitment/position/service/PositionServiceImpl.java (1)
17-19: μ μ μν¬νΈrecruitmentκ°create()λ©μλμ λ‘컬 λ³μμ νΌλλ μ μμ΅λλ€.Line 41μμ
Recruitment recruitment = ...λ‘ μ μΈλ λ‘컬 λ³μμ μ μ μν¬νΈQRecruitment.recruitmentμ μ΄λ¦μ΄ λμΌν©λλ€. νμ¬λ λ€λ₯Έ λ©μλ μ€μ½νμ μμ΄ λ¬Έμ κ° μμ§λ§, ν₯ν 리ν©ν λ§μ΄λ μ½λ λ³΅μ¬ μ νΌλμ μΌκΈ°ν μ μμ΅λλ€.
π Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (22)
src/main/java/KUSITMS/WITHUS/domain/application/application/controller/AdminApplicationController.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/dto/ApplicationResponseDTO.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationJpaRepository.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepository.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepositoryImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationService.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/application/service/evaluator/EvaluatorAssignmentService.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/application/applicationEvaluator/dto/ApplicationEvaluatorRequestDTO.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/dto/DistributionRequestResponseDTO.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/entity/DistributionAssignment.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/repository/DistributionRequestRepositoryImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluationCriteria/repository/EvaluationCriteriaRepository.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluationCriteria/repository/EvaluationCriteriaRepositoryImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/recruitment/position/dto/PositionResponseDTO.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/recruitment/position/service/PositionServiceImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/template/controller/TemplateController.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/repository/TemplateRepository.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/repository/TemplateRepositoryImpl.java(2 hunks)src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateService.java(1 hunks)src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java(3 hunks)src/main/java/KUSITMS/WITHUS/global/exception/ErrorCode.java(1 hunks)
π§° Additional context used
𧬠Code graph analysis (3)
src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluationCriteria/repository/EvaluationCriteriaRepositoryImpl.java (2)
src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepositoryImpl.java (1)
Repository(21-183)src/main/java/KUSITMS/WITHUS/domain/recruitment/recruitment/repository/RecruitmentRepositoryImpl.java (1)
Repository(17-98)
src/main/java/KUSITMS/WITHUS/domain/recruitment/position/service/PositionServiceImpl.java (1)
src/main/java/KUSITMS/WITHUS/domain/interview/timeslot/repository/TimeSlotRepositoryImpl.java (1)
Service(22-106)
src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/repository/DistributionRequestRepositoryImpl.java (1)
src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepositoryImpl.java (1)
Repository(21-183)
β° 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 (26)
src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationService.java (1)
31-31: LGTM!λ©μλ μκ·Έλμ²κ° κΈ°μ‘΄ μΈν°νμ΄μ€ ν¨ν΄κ³Ό μΌκ΄μ± μκ² μ μλμμ΅λλ€.
recruitmentIdλ‘ λ²μλ₯Ό μ ννκ³keywordλ‘ κ²μνλ κ΅¬μ‘°κ° μ μ ν©λλ€.src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationJpaRepository.java (1)
14-14: LGTM!Spring Data JPAμ
IsNullμ λ―Έμ¬λ₯Ό μ¬μ©ν 쿼리 λ©μλ λ€μ΄λ°μ΄ μ¬λ°λ₯΄κ² μ μ©λμμ΅λλ€. κΈ°μ‘΄findByRecruitment_IdAndOrganizationRole_Idλ©μλμ μΌκ΄λ ν¨ν΄μ μ μ§νκ³ μμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluationCriteria/repository/EvaluationCriteriaRepository.java (1)
14-14: LGTM!λ©μλ μκ·Έλμ²κ° μ μ ν©λλ€.
OrganizationRoleμν°ν°λ₯Ό μ§μ λ°μ 곡ν΅(null) λ° νΉμ μν μΌμ΄μ€λ₯Ό λͺ¨λ μ²λ¦¬ν μ μλλ‘ μ€κ³λμμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/application/applicationEvaluator/dto/ApplicationEvaluatorRequestDTO.java (1)
29-31: @NotNull μ μ½ μ‘°κ±΄ μ κ±°μ λ°λ₯Έ API λ³κ²½ νμΈ νμ
organizationRoleIdμμ@NotNullμ μ½ μ‘°κ±΄μ΄ μ κ±°λμ΄ null κ°μ΄ νμ©λ©λλ€. Swagger μ€λͺ μ λͺ νν λ¬Έμνλμ΄ μμ§λ§, κΈ°μ‘΄ API ν΄λΌμ΄μΈνΈμμ νΈνμ±μ νμΈν΄ μ£ΌμΈμ.src/main/java/KUSITMS/WITHUS/domain/application/application/service/evaluator/EvaluatorAssignmentService.java (2)
49-51: LGTM!
organizationRoleIdκ° nullμΈ κ²½μ°λ₯Ό μ μ ν μ²λ¦¬νκ³ μμ΅λλ€. null μ²΄ν¬ ν 쑰건λΆλ‘ μν°ν°λ₯Ό μ‘°ννκ±°λ nullμ ν λΉνλ λ‘μ§μ΄ λͺ νν©λλ€.
82-85: LGTM!
organizationRoleIdκ° nullμΈ κ²½μ° κ³΅ν΅(μν λ―Έμ§μ ) μ§μμλ₯Ό μ‘°ννκ³ , κ·Έλ μ§ μμΌλ©΄ νΉμ μν μ μ§μμλ₯Ό μ‘°ννλ λΆκΈ° λ‘μ§μ΄ μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepository.java (2)
17-17: LGTM!null OrganizationRoleμ κ°μ§ μ§μμλ₯Ό μ‘°ννλ λ©μλκ° μ μ ν μΆκ°λμμ΅λλ€.
26-26: LGTM!μ΄λ¦ λλ μ΄λ©μΌλ‘ μ§μμλ₯Ό κ²μνλ λ©μλκ° μΆκ°λμμ΅λλ€. λ©μλλͺ μ΄ κΈ°λ₯μ λͺ νν μ€λͺ νκ³ μμ΅λλ€.
src/main/java/KUSITMS/WITHUS/domain/application/application/dto/ApplicationResponseDTO.java (1)
558-573: LGTM!
ApplicantForMailSmsDTOκ° λ©μΌ/SMS λ°μ‘ μ©λμ λ§κ² νμν νλλ§ ν¬ν¨νμ¬ μ μ μλμμ΅λλ€.from()ν©ν 리 λ©μλλ₯Ό ν΅ν 맀νλ μ μ ν©λλ€.μ°Έκ³ : λ€λ₯Έ DTOλ€μ
idνλλͺ μ μ¬μ©νλ λ°λ©΄ μ΄ DTOλapplicationIdλ₯Ό μ¬μ©ν©λλ€. μλλ μ€κ³λΌλ©΄ λ¬Έμ μμ§λ§, μΌκ΄μ±μ μν΄ κ²ν ν΄ λ³΄μ€ μ μμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/evaluation/evaluationCriteria/repository/EvaluationCriteriaRepositoryImpl.java (1)
44-59: LGTM!QueryDSL ꡬνμ΄ μ¬λ°λ¦ λλ€.
organizationRoleμ΄ nullμΌ λλ κ³΅ν΅ κΈ°μ€(null role)λ§ μ‘°ννκ³ , κ°μ΄ μμ λλ κ³΅ν΅ κΈ°μ€κ³Ό νΉμ μν κΈ°μ€μ λͺ¨λ μ‘°ννλ λ‘μ§μ΄ μ νν©λλ€. λ€λ₯Έ Repository ꡬν체λ€κ³Ό μΌκ΄λ ν¨ν΄μ λ°λ₯΄κ³ μμ΅λλ€.src/main/java/KUSITMS/WITHUS/domain/application/application/service/ApplicationServiceImpl.java (2)
209-216: LGTM!νκ° κΈ°μ€ μ‘°ν λ‘μ§μ΄ μ¬λ°λ₯΄κ² λ³κ²½λμμ΅λλ€.
findCommonAndByOrganizationRoleμ μ¬μ©νμ¬ κ³΅ν΅ κΈ°μ€(organizationRoleμ΄ null)κ³Ό μ§μμμ νΉμ μν μ ν΄λΉνλ κΈ°μ€μ ν¨κ» μ‘°νν©λλ€.app.getOrganizationRole()μ΄ nullμΈ κ²½μ°λ μ λ©μλμμ μ¬λ°λ₯΄κ² μ²λ¦¬λ©λλ€.
472-484: LGTM!λ©μΌ/λ¬Έμ λ°μ‘μ© μ§μμ κ²μ λ©μλκ° κΉλνκ² κ΅¬νλμμ΅λλ€. Javadoc λ¬Έμνλ μ λμ΄ μκ³ , κΈ°μ‘΄ μ½λλ² μ΄μ€μ ν¨ν΄μ λ°λ₯΄κ³ μμ΅λλ€.
src/main/java/KUSITMS/WITHUS/domain/application/application/repository/ApplicationRepositoryImpl.java (2)
65-68: LGTM!JPA Repositoryλ‘μ λ¨μ μμμ΄λ©°,
organizationRoleμ΄ nullμΈ μ§μμλ₯Ό μ‘°ννλ κΈ°λ₯μ΄ μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.
163-182: LGTM!ν€μλ κΈ°λ° κ²μ ꡬνμ΄ μ¬λ°λ¦ λλ€. κ°μ νμΌμ
findEligibleCandidatesλ©μλ(lines 97-101)μ λμΌν null predicate ν¨ν΄μ μ¬μ©νμ¬ μΌκ΄μ±μ΄ μμ΅λλ€. QueryDSLμwhere()μ μμ null predicateλ μλμΌλ‘ 무μλλ―λ‘, ν€μλκ° μμ λ μ 체 λͺ©λ‘μ΄ μ‘°νλ©λλ€.src/main/java/KUSITMS/WITHUS/domain/recruitment/position/dto/PositionResponseDTO.java (2)
3-3: LGTM!μλ‘μ΄
from(OrganizationRole)λ©μλλ₯Ό μ§μνκΈ° μν΄ νμν importμ λλ€.
28-38: λ©μλ ꡬνμ΄ μ¬λ°λ¦ λλ€.
OrganizationRoleμν°ν°λ Lombokμ@Getterμ λν μ΄μ μΌλ‘ μΈν΄getId(),getName(),getColor()λ©μλκ° μλ μμ±λ©λλ€.from(OrganizationRole)λ©μλλ μ΄ getter λ©μλλ€μ μ¬λ°λ₯΄κ² μ¬μ©νλ©°, null μ²΄ν¬ μ²λ¦¬λ μ μ ν ꡬνλμ΄ μκ³ , κΈ°μ‘΄from(Position)λ©μλμ μΌκ΄λ ν¨ν΄μ λ°λ¦ λλ€.src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/repository/DistributionRequestRepositoryImpl.java (1)
4-4: QueryDSL μΈνλΌ μΆκ° μΉμΈQueryDSLμ λμ νμ¬ νμ μμ ν 쿼리μ fetch join μ΅μ νλ₯Ό κ°λ₯νκ² νλ λ³κ²½μ¬νμ λλ€. νμν λͺ¨λ Q-ν΄λμ€κ° μν¬νΈλμμΌλ©°,
JPAQueryFactoryκ° μμ±μ μ£Όμ μ ν΅ν΄ μ¬λ°λ₯΄κ² μ£Όμ λ©λλ€.Also applies to: 10-12, 19-19
src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/entity/DistributionAssignment.java (1)
24-26: μ‘°μ§ μν nullable λ³κ²½μ μ΄λ―Έ μ μ ν μ²λ¦¬λμ΄ μμ΅λλ€
organizationRoleνλμ nullable λ³κ²½μ λν νμΈμ΄ μλ£λμμ΅λλ€:
- Null μμ μ±: EvaluatorAssignmentServiceμμ null μ²΄ν¬ ν ν λΉ (λΌμΈ 49-51), DistributionRequestResponseDTOμμ "곡ν΅" ν΄λ°± μ²λ¦¬ (λΌμΈ 44) λ± λͺ¨λ νΈμΆ μ§μ μμ μ μ ν μ²λ¦¬λ¨
- λ°μ΄ν°λ² μ΄μ€ μ€ν€λ§: application-prod.ymlμ
spring.jpa.hibernate.ddl-auto: updateμ€μ μΌλ‘ Hibernateμ΄ μλμΌλ‘ μ€ν€λ§ λ³κ²½μ κ΄λ¦¬- 쿼리 λ‘μ§: DistributionRequestRepositoryImplμμ leftJoinμΌλ‘ nullable FK μ¬λ°λ₯΄κ² μ²λ¦¬, EvaluatorAssignmentServiceμμ organizationRoleId null μ¬λΆμ λ°λΌ λ³λ 쿼리 μ€ν
λͺ¨λ κ΄λ ¨ DTOμ μλΉμ€ κ³μΈ΅μμ null κ°μ μ¬λ°λ₯΄κ² μ²λ¦¬νκ³ μμ΅λλ€.
src/main/java/KUSITMS/WITHUS/global/exception/ErrorCode.java (1)
137-137: LGTM!ν νλ¦Ώ κΆν κ²μ¦μ μν μλ¬ μ½λκ° μ μ νκ² μΆκ°λμμ΅λλ€. λ€μ΄λ°, HTTP μν μ½λ(403 FORBIDDEN), κ·Έλ¦¬κ³ λ©μμ§ λͺ¨λ κΆν κ±°λΆ μλ리μ€μ μ ν©ν©λλ€.
src/main/java/KUSITMS/WITHUS/domain/template/repository/TemplateRepositoryImpl.java (1)
25-35: μ‘°μ§ eager loading μ²λ¦¬κ° μ μ ν©λλ€.QueryDSLμ fetchJoinμ μ¬μ©νμ¬ Organizationμ 미리 λ‘λ©ν¨μΌλ‘μ¨ N+1 쿼리 λ¬Έμ λ₯Ό λ°©μ§ν©λλ€. μ΄λ μλΉμ€ λ μ΄μ΄μμ κΆν κ²μ¦ μ μΆκ° 쿼리 μμ΄ μ‘°μ§ μ 보μ μ κ·Όν μ μκ² ν΄μ€λλ€.
src/main/java/KUSITMS/WITHUS/domain/template/repository/TemplateRepository.java (1)
12-12: LGTM!Repository μΈν°νμ΄μ€μ delete λ©μλκ° μ μ νκ² μΆκ°λμμ΅λλ€. κΈ°μ‘΄ CRUD λ©μλλ€κ³Ό μΌκ΄λ μκ·Έλμ²λ₯Ό μ μ§νκ³ μμ΅λλ€.
src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateService.java (1)
14-15: μ¬μ©μ κΈ°λ° κΆν κ²μ¦μ μν μΈν°νμ΄μ€ μ€κ³κ° μ μ ν©λλ€.updateμ delete λ©μλ λͺ¨λ User νλΌλ―Έν°λ₯Ό μΆκ°νμ¬ μ‘°μ§ κΈ°λ° μ κ·Ό μ μ΄λ₯Ό μΌκ΄λκ² μ μ©νμ΅λλ€. μ΅μ κΆν μμΉμ λ°λ₯΄λ μ’μ μ€κ³μ λλ€.
src/main/java/KUSITMS/WITHUS/domain/template/controller/TemplateController.java (2)
54-63: LGTM!update μλν¬μΈνΈκ° μ¬μ©μ μΈμ¦μ ν¬ν¨νλλ‘ μ μ νκ² μμ λμμ΅λλ€. @currentuser μ΄λ Έν μ΄μ μ ν΅ν μ¬μ©μ μΆμΆκ³Ό μλΉμ€ λ μ΄μ΄λ‘μ μμμ΄ μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.
65-73: μμ μλν¬μΈνΈ ꡬνμ΄ μ μ ν©λλ€.RESTful κ·μΉμ λ°λ₯΄λ©°, μ¬μ©μ μΈμ¦ λ° μ‘°μ§ κΈ°λ° μ κ·Ό μ μ΄κ° update μλν¬μΈνΈμ μΌκ΄λκ² κ΅¬νλμμ΅λλ€.
src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java (2)
67-79: κΆν κ²μ¦ λ‘μ§μ΄ μ¬λ°λ₯΄κ² ꡬνλμμ΅λλ€.μ¬μ©μκ° ν νλ¦Ώμ μ‘°μ§μ μν΄μλμ§ νμΈνλ λ‘μ§μ΄ λͺ ννκ³ μ νν©λλ€. anyMatchλ₯Ό μ¬μ©νμ¬ ν¨μ¨μ μΌλ‘ 첫 λ²μ§Έ μΌμΉ νλͺ©μμ λ¨λ½ νκ°λ₯Ό μνν©λλ€.
83-95: update λ©μλμ κΆν κ²μ¦ ν΅ν©μ΄ μ μ ν©λλ€.ν νλ¦Ώ μ‘°ν ν μ¦μ κΆνμ κ²μ¦νμ¬ λ³κ²½ μμ μ μ μ κ·Ό μ μ΄λ₯Ό μνν©λλ€. νΈλμμ μ²λ¦¬λ μ¬λ°λ₯΄κ² μ μ©λμμ΅λλ€.
| public List<DistributionRequest> findAllByRecruitmentIdOrderByCreatedAtDesc(Long recruitmentId) { | ||
| return distributionRequestJpaRepository.findAllByRecruitmentIdOrderByCreatedAtDesc(recruitmentId); | ||
| return queryFactory | ||
| .selectFrom(distributionRequest) | ||
| .distinct() | ||
| .leftJoin(distributionRequest.assignments, distributionAssignment).fetchJoin() | ||
| .leftJoin(distributionAssignment.organizationRole, organizationRole).fetchJoin() | ||
| .where(distributionRequest.recruitmentId.eq(recruitmentId)) | ||
| .orderBy(distributionRequest.createdAt.desc()) | ||
| .fetch(); | ||
| } |
There was a problem hiding this comment.
N+1 쿼리 λ¬Έμ ν΄κ²°μ μν fetch join μ΅μ ν
QueryDSLμ μ¬μ©νμ¬ λͺ
μμ μΌλ‘ fetch joinμ μννλ 리ν©ν λ§μ κΆμ₯λ©λλ€. μ΄λ₯Ό ν΅ν΄ assignmentsμ organizationRoleμ μ§μ° λ‘λ© μ λ°μν μ μλ N+1 쿼리 λ¬Έμ λ₯Ό λ°©μ§ν©λλ€.
ꡬν μμΈ:
- fetch joinμΌλ‘ κ΄λ ¨ λ°μ΄ν°λ₯Ό μ¦μ λ‘λ©νλ©°, distinct()λ OneToMany μ‘°μΈμμ λ°μνλ μ€λ³΅ μν°ν°λ₯Ό μ κ±°ν©λλ€
- leftJoin μ¬μ©μΌλ‘ μμ§ ν λΉμ΄ μλ DistributionRequestλ μ‘°ν κ°λ₯ν©λλ€
- μ΄ ν¨ν΄μ μ κ³ νμ€ λ°©μμ λλ€
Hibernate λ²μ κ³ λ €μ¬ν:
- Hibernate 6μμλ DISTINCT ν€μλκ° λΆνμνλ©° μλμΌλ‘ μ€λ³΅μ μ κ±°ν©λλ€
- Hibernate 5λ₯Ό μ¬μ© μ€μ΄λΌλ©΄ distinct()κ° λ°μ΄ν°λ² μ΄μ€ λ 벨μμ μΆκ° μ²λ¦¬λ₯Ό μννλ―λ‘, νλ‘μ νΈμ Hibernate λ²μ μ νμΈνμΈμ. νμμ
hibernate.query.passDistinctThrough=falseννΈ μ¬μ©μ κ²ν νμΈμ.
π€ Prompt for AI Agents
In
src/main/java/KUSITMS/WITHUS/domain/application/distributionRequest/repository/DistributionRequestRepositoryImpl.java
around lines 37 to 46, ensure the fetch-join optimization is correct: keep the
leftJoin(...).fetchJoin() calls to eagerly load assignments and organizationRole
(so DistributionRequest without assignments is still returned), but verify your
Hibernate versionβif using Hibernate 6 you can remove distinct() because
Hibernate will deduplicate results automatically; if using Hibernate 5, keep
distinct() but add the query hint hibernate.query.passDistinctThrough=false (or
set the same property globally) to avoid pushing DISTINCT to the DB and causing
extra work.
| // Recruitmentμ RecruitmentOrganizationRole 리μ€νΈμ κ°κ°μ OrganizationRoleμ ν¨κ» μ‘°ννκΈ° μν΄ fetch join μ¬μ© | ||
| Recruitment found = queryFactory | ||
| .selectFrom(recruitment) | ||
| .leftJoin(recruitment.positions, recruitmentOrganizationRole).fetchJoin() | ||
| .leftJoin(recruitmentOrganizationRole.organizationRole, organizationRole).fetchJoin() | ||
| .where(recruitment.id.eq(recruitmentId)) | ||
| .fetchOne(); | ||
|
|
||
| if (found == null) { | ||
| throw new CustomException(ErrorCode.RECRUITMENT_NOT_EXIST); | ||
| } | ||
|
|
||
| return found.getPositions().stream() | ||
| .map(ror -> PositionResponseDTO.Detail.from(ror.getOrganizationRole())) | ||
| .toList(); |
There was a problem hiding this comment.
organizationRoleμ΄ nullμΌ κ²½μ° NPE λ°μ κ°λ₯μ±μ΄ μμ΅λλ€.
Line 69μμ leftJoinμ μ¬μ©νκ³ μμ΄ organizationRoleμ΄ nullμΌ μ μμ΅λλ€. Line 78μμ ror.getOrganizationRole()μ΄ nullμ λ°ννλ©΄ PositionResponseDTO.Detail.from(null) νΈμΆ μ NullPointerExceptionμ΄ λ°μν μ μμ΅λλ€.
π null νν°λ§μ μΆκ°ν μμ μ μ
return found.getPositions().stream()
+ .filter(ror -> ror.getOrganizationRole() != null)
.map(ror -> PositionResponseDTO.Detail.from(ror.getOrganizationRole()))
.toList();λλ organizationRoleμ΄ νμ μ‘΄μ¬ν΄μΌ νλ λΉμ¦λμ€ μꡬμ¬νμ΄λΌλ©΄, leftJoin λμ joinμ μ¬μ©νλ κ²μ΄ μ μ ν©λλ€:
- .leftJoin(recruitmentOrganizationRole.organizationRole, organizationRole).fetchJoin()
+ .join(recruitmentOrganizationRole.organizationRole, organizationRole).fetchJoin()Committable suggestion skipped: line range outside the PR's diff.
π€ Prompt for AI Agents
In
src/main/java/KUSITMS/WITHUS/domain/recruitment/position/service/PositionServiceImpl.java
around lines 65 to 79, the current leftJoin on
recruitmentOrganizationRole.organizationRole allows organizationRole to be null
and calling PositionResponseDTO.Detail.from(ror.getOrganizationRole()) may throw
an NPE; either change the fetch join to an inner join (replace leftJoin with
join) if business rules guarantee an OrganizationRole exists, or keep the
leftJoin and filter out null organizationRole values before mapping (e.g.,
stream().map(ror ->
ror.getOrganizationRole()).filter(Objects::nonNull).map(PositionResponseDTO.Detail::from)
or explicitly handle nulls by throwing a CustomException); implement the chosen
approach consistently so no null is passed into PositionResponseDTO.Detail.from.
| /** | ||
| * λ¬Έμ/λ©μΌ ν νλ¦Ώ μμ | ||
| * @param templateId μμ ν ν νλ¦Ώ ID | ||
| * @param user νμ¬ μ¬μ©μ | ||
| */ | ||
| @Override | ||
| @Transactional | ||
| public void delete(Long templateId, User user) { | ||
| Template template = templateRepository.getById(templateId); | ||
| validateUserHasAccess(template, user); | ||
| templateRepository.delete(templateId); | ||
| } |
There was a problem hiding this comment.
ν νλ¦Ώ μν°ν°κ° μ€λ³΅ μ‘°νλλ μ±λ₯ μ΄μκ° μμ΅λλ€.
delete λ©μλμμ λ€μκ³Ό κ°μ νλ¦μΌλ‘ ν νλ¦Ώμ΄ λ λ² μ‘°νλ©λλ€:
- Line 105:
templateRepository.getById(templateId)- 첫 λ²μ§Έ μ‘°ν - Line 107:
templateRepository.delete(templateId)νΈμΆ - TemplateRepositoryImpl.delete (line 56)μμ λ΄λΆμ μΌλ‘
getById(templateId)μ¬νΈμΆ - λ λ²μ§Έ μ‘°ν
λΆνμν λ°μ΄ν°λ² μ΄μ€ μΏΌλ¦¬κ° λ°μνμ¬ μ±λ₯μ μν₯μ μ€λλ€.
π μ μνλ μμ λ°©μ
λ°©μ 1: μ΄λ―Έ μ‘°νν Template μν°ν°λ₯Ό νμ©
TemplateRepositoryImpl.javaλ₯Ό μμ νμ¬ Template μν°ν°λ₯Ό μ§μ λ°λλ‘ λ³κ²½:
@Override
-public void delete(Long templateId) {
- Template template = getById(templateId);
+public void delete(Template template) {
templateJpaRepository.delete(template);
}TemplateRepository.java μΈν°νμ΄μ€λ μμ :
-void delete(Long templateId);
+void delete(Template template);TemplateServiceImpl.javaμμ μ‘°νν μν°ν°λ₯Ό μ λ¬:
public void delete(Long templateId, User user) {
Template template = templateRepository.getById(templateId);
validateUserHasAccess(template, user);
- templateRepository.delete(templateId);
+ templateRepository.delete(template);
}λ°©μ 2: Repository delete λ©μλμμ μ€λ³΅ μ‘°ν μ κ±°
TemplateRepositoryImpl.java:
@Override
public void delete(Long templateId) {
- Template template = getById(templateId);
- templateJpaRepository.delete(template);
+ templateJpaRepository.deleteById(templateId);
}λ¨, μ΄ λ°©μμ μμ μ μ‘΄μ¬ μ¬λΆ κ²μ¦μ΄ μλ΅λλ―λ‘ μλΉμ€ λ μ΄μ΄μμ μ΄λ―Έ getByIdλ₯Ό νΈμΆνλ νμ¬ κ΅¬μ‘°μμλ§ μ ν©ν©λλ€.
Committable suggestion skipped: line range outside the PR's diff.
π€ Prompt for AI Agents
In src/main/java/KUSITMS/WITHUS/domain/template/service/TemplateServiceImpl.java
around lines 97 to 108, the Template entity is being fetched (getById) and then
delete(templateId) causes a second fetch inside the repository, producing
duplicate DB queries; fix by reworking the delete flow so the already-fetched
Template is used: update the TemplateRepository interface to add a
delete(Template template) overload (or replace delete(Long) with
delete(Template)), update TemplateRepositoryImpl.delete to accept and delete the
passed Template without re-querying, and change TemplateServiceImpl.delete to
call repository.delete(template) after validateUserHasAccess(template, user).
Ensure transactional semantics remain the same and adjust any callers/tests
accordingly.
EunjinWoo
changed the title
EunjinWoo
changed the title
2 participants
β¨ Related Issue
π Task Details
π¬ Review Requirements (Optional)
Summary by CodeRabbit
New Features
Improvements
βοΈ Tip: You can customize this high-level summary in your review settings.