[FEATURE] 강사 대시보드 외주 조회 기능 구현#40
Conversation
📝 WalkthroughWalkthrough
Changes강사 대시보드 외주 조회 기능 구현
Sequence Diagram(s)sequenceDiagram
actor Instructor
participant InstructorDashboardController
participant InstructorDashboardService
participant DashboardCommissionRepository
participant Commission
rect rgba(70, 130, 180, 0.5)
Note over Instructor,Commission: 매칭 중인 외주 조회
Instructor->>InstructorDashboardController: GET /api/v1/instructors/dashboards/matchings
InstructorDashboardController->>InstructorDashboardService: getMatchingCommissions(instructorId)
InstructorDashboardService->>DashboardCommissionRepository: findMatchingViews(instructorId, RECRUITING, PENDING)
DashboardCommissionRepository-->>InstructorDashboardService: List~MatchingView~(commission, distinctLevelCount, totalCount)
InstructorDashboardService->>Commission: matchedCount(distinctLevels, totalApplicants)
Commission-->>InstructorDashboardService: matched int
InstructorDashboardService-->>InstructorDashboardController: MatchingCommissionResponse
InstructorDashboardController-->>Instructor: ApiResponse(MatchingCommissionResponse)
end
rect rgba(60, 179, 113, 0.5)
Note over Instructor,Commission: 수정 중인 외주 조회
Instructor->>InstructorDashboardController: GET /api/v1/instructors/dashboards/revisions
InstructorDashboardController->>InstructorDashboardService: getRevisingCommissions(instructorId)
InstructorDashboardService->>DashboardCommissionRepository: findRevisingViews(instructorId, EDITING)
DashboardCommissionRepository-->>InstructorDashboardService: List~RevisingView~(commission, submitted, hasUpdated)
InstructorDashboardService-->>InstructorDashboardController: RevisingCommissionResponse
InstructorDashboardController-->>Instructor: ApiResponse(RevisingCommissionResponse)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 8
🧹 Nitpick comments (1)
src/main/java/ditda/backend/domain/commission/dashboard/dto/response/RevisingCommissionResponse.java (1)
16-22: 💤 Low valueMap 조회 결과에 대한 방어적 처리를 고려해보세요.
18번 라인에서
statuses.get(c.getId())가null을 반환할 경우CommissionItem.from에서 NPE가 발생할 수 있습니다.현재
RevisionService.getRevisionStatuses가 누락된 ID를new RevisionStatus(false, false)로 채워주지만(Context snippet 참조), DTO 레이어에서도 방어적 처리를 추가하면 향후 상위 계약 변경 시에도 안전합니다.♻️ 방어적 코드 제안
public static RevisingCommissionResponse of(List<Commission> commissions, Map<Long, RevisionStatus> statuses) { List<CommissionItem> items = commissions.stream() - .map(c -> CommissionItem.from(c, statuses.get(c.getId()))) + .map(c -> CommissionItem.from(c, + statuses.getOrDefault(c.getId(), new RevisionStatus(false, false)))) .toList(); return new RevisingCommissionResponse(items); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ditda/backend/domain/commission/dashboard/dto/response/RevisingCommissionResponse.java` around lines 16 - 22, In the of method of RevisingCommissionResponse class, the call to statuses.get(c.getId()) can return null, which would then be passed to CommissionItem.from() and cause a NullPointerException. Replace the direct statuses.get(c.getId()) call with a null-safe approach such as statuses.getOrDefault(c.getId(), new RevisionStatus(false, false)) to provide a default RevisionStatus when the ID is not found in the map, ensuring the DTO layer has defensive handling regardless of upstream changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/ditda/backend/domain/commission/application/service/CommissionApplicationService.java`:
- Around line 54-56: The Collectors.toMap method used in the return statement of
the stream operation does not specify a merge function to handle duplicate keys,
which causes an IllegalStateException when commissionIds contains duplicates.
Modify the Collectors.toMap call to include a third parameter that provides a
merge function to handle key collisions, such as keeping the existing value or
applying a merge strategy that makes sense for the countByCommissionId data.
In
`@src/main/java/ditda/backend/domain/commission/dashboard/controller/InstructorDashboardController.java`:
- Around line 17-53: The InstructorDashboardController endpoints
(getDraftSubmissions, getMatchingCommissions, and getRevisingCommissions)
currently lack explicit role-based access control annotations. Add
`@PreAuthorize`("hasRole('INSTRUCTOR')") annotation either at the class level
above the InstructorDashboardController declaration to secure all endpoints at
once, or add the annotation individually to each endpoint method. This ensures
that only users with the ROLE_INSTRUCTOR role can access these dashboard API
endpoints, making the authorization requirement explicit and enforced at the
method level.
In
`@src/main/java/ditda/backend/domain/commission/dashboard/dto/response/DraftSubmissionCommissionResponse.java`:
- Around line 20-25: The Map#get calls for draftSubmissionCount.get(c.getId())
and viewable.get(c.getId()) in the CommissionItem.from() method invocation can
return null values, and these nulls are being unboxed into primitive types which
causes a NullPointerException. Replace the direct Map#get calls with either
Map#getOrDefault providing appropriate default values (such as 0 for long and
false for boolean), or wrap the get results with null-checking logic to ensure
primitive type safety when assembling the response.
In
`@src/main/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardService.java`:
- Around line 89-95: The stream operation collecting commissions into the
matchedCount map is vulnerable to NullPointerException because the get() calls
on distinctLevelCount and applicantCount maps can return null before calling
intValue(). Add null checks or use getOrDefault() with appropriate default
values when retrieving from these maps in the Commission::matchedCount method
call. Alternatively, verify that the aggregation methods used to populate
distinctLevelCount and applicantCount (likely from CommissionApplicationService)
guarantee entries exist for all commission IDs in the commissions list, and if
not, filter the commissions stream to only include IDs that exist in both maps.
In
`@src/main/java/ditda/backend/domain/commission/revision/entity/RevisionResponse.java`:
- Around line 43-45: The `checked` field in the RevisionResponse entity is being
added as a NOT NULL column, but the `@Builder.Default` annotation only provides a
default at the Java level and will not apply to existing database rows, causing
schema migration to fail if the table already contains data. To fix this, add a
columnDefinition parameter to the `@Column` annotation on the `checked` field that
explicitly defines the database-level default value (e.g., "BOOLEAN NOT NULL
DEFAULT FALSE"), or alternatively verify and document that the
revision_responses table will be empty at deployment time to ensure the NOT NULL
constraint can be safely applied.
In
`@src/main/java/ditda/backend/domain/commission/revision/repository/RevisionRequestRepository.java`:
- Around line 14-23: The submitted flag calculation in the findRevisionStatuses
method has inverted logic that contradicts both the comment and the API
specification. The current CASE expression for submitted returns true when
responses are absent (resp.id IS NULL), but according to the comment and
downstream API contract, it should return false when there are no responses.
Invert the CASE condition for the submitted calculation: change it to return
false when COUNT of null response IDs is greater than 0, and true otherwise, so
that submitted=false indicates no responses have been made.
In
`@src/test/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardServiceTest.java`:
- Around line 91-92: The mock setup for
commissionService.getCommissionByInstructorAndStatus at line 91-92 is using null
for the third parameter, but the actual service call in the code being tested
uses Sort.by("applicationDeadline").ascending(). Update the given() statement to
pass Sort.by("applicationDeadline").ascending() as the third argument instead of
null to match the actual invocation made by the service under test.
- Around line 60-61: The test mock setup for
commissionService.getCommissionByInstructorAndStatus is passing null as the
third parameter, but the actual InstructorDashboardService implementation passes
Sort.by("applicationDeadline").ascending() as the third parameter, causing a
Mockito PotentialStubbingProblem. Update the given() stub configuration to use
Sort.by("applicationDeadline").ascending() instead of null as the third argument
to commissionService.getCommissionByInstructorAndStatus so the mock matches the
actual service behavior.
---
Nitpick comments:
In
`@src/main/java/ditda/backend/domain/commission/dashboard/dto/response/RevisingCommissionResponse.java`:
- Around line 16-22: In the of method of RevisingCommissionResponse class, the
call to statuses.get(c.getId()) can return null, which would then be passed to
CommissionItem.from() and cause a NullPointerException. Replace the direct
statuses.get(c.getId()) call with a null-safe approach such as
statuses.getOrDefault(c.getId(), new RevisionStatus(false, false)) to provide a
default RevisionStatus when the ID is not found in the map, ensuring the DTO
layer has defensive handling regardless of upstream changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08b2a9ab-51c6-4d6f-a4c1-26d5b854fe20
📒 Files selected for processing (22)
src/main/java/ditda/backend/domain/commission/application/repository/CommissionApplicationRepository.javasrc/main/java/ditda/backend/domain/commission/application/repository/projection/ApplicationSubmissionCount.javasrc/main/java/ditda/backend/domain/commission/application/service/CommissionApplicationService.javasrc/main/java/ditda/backend/domain/commission/core/controller/InstructorCommissionController.javasrc/main/java/ditda/backend/domain/commission/core/entity/Commission.javasrc/main/java/ditda/backend/domain/commission/core/facade/InstructorCommissionFacade.javasrc/main/java/ditda/backend/domain/commission/core/repository/CommissionRepository.javasrc/main/java/ditda/backend/domain/commission/core/service/InstructorCommissionService.javasrc/main/java/ditda/backend/domain/commission/dashboard/controller/InstructorDashboardController.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/DraftSubmissionCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/MatchingCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/RevisingCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardService.javasrc/main/java/ditda/backend/domain/commission/draft/facade/DraftFacade.javasrc/main/java/ditda/backend/domain/commission/draft/service/DraftService.javasrc/main/java/ditda/backend/domain/commission/revision/dto/RevisionStatus.javasrc/main/java/ditda/backend/domain/commission/revision/entity/RevisionResponse.javasrc/main/java/ditda/backend/domain/commission/revision/repository/RevisionRequestRepository.javasrc/main/java/ditda/backend/domain/commission/revision/repository/projection/RevisionStatusView.javasrc/main/java/ditda/backend/domain/commission/revision/service/RevisionService.javasrc/test/java/ditda/backend/domain/commission/core/entity/CommissionTest.javasrc/test/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardServiceTest.java
Jong0128
left a comment
There was a problem hiding this comment.
수고많으셨습니다!! 👍
아래 커멘트 확인 부탁드립니다 😄
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/ditda/backend/domain/commission/dashboard/dto/response/MatchingCommissionResponse.java (1)
40-43: ⚡ Quick win카운트 다운캐스팅은 안전 변환으로 바꾸는 것을 권장합니다.
Line 40-43의
intValue()는 overflow 시 값이 조용히 깨질 수 있습니다.Math.toIntExact(...)로 fail-fast 처리하는 편이 안전합니다.변경 예시
- int matched = commission.matchedCount( - view.getDistinctLevelCount().intValue(), - view.getTotalCount().intValue() - ); + int matched = commission.matchedCount( + Math.toIntExact(view.getDistinctLevelCount()), + Math.toIntExact(view.getTotalCount()) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ditda/backend/domain/commission/dashboard/dto/response/MatchingCommissionResponse.java` around lines 40 - 43, The code in the matchedCount() method call uses intValue() to convert values to int, which can silently overflow and corrupt data. Replace both intValue() calls with Math.toIntExact() instead - specifically on view.getDistinctLevelCount().intValue() and view.getTotalCount().intValue() arguments passed to commission.matchedCount(). This ensures overflow conditions throw an exception immediately rather than silently wrapping the values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/main/java/ditda/backend/domain/commission/dashboard/dto/response/MatchingCommissionResponse.java`:
- Around line 40-43: The code in the matchedCount() method call uses intValue()
to convert values to int, which can silently overflow and corrupt data. Replace
both intValue() calls with Math.toIntExact() instead - specifically on
view.getDistinctLevelCount().intValue() and view.getTotalCount().intValue()
arguments passed to commission.matchedCount(). This ensures overflow conditions
throw an exception immediately rather than silently wrapping the values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c2e5777-5912-487b-ba49-088a8b356d3c
📒 Files selected for processing (11)
src/main/java/ditda/backend/domain/commission/core/service/InstructorCommissionService.javasrc/main/java/ditda/backend/domain/commission/dashboard/controller/InstructorDashboardController.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/DraftSubmissionCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/MatchingCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/dto/response/RevisingCommissionResponse.javasrc/main/java/ditda/backend/domain/commission/dashboard/repository/DashboardCommissionRepository.javasrc/main/java/ditda/backend/domain/commission/dashboard/repository/projection/DraftSubmissionView.javasrc/main/java/ditda/backend/domain/commission/dashboard/repository/projection/MatchingView.javasrc/main/java/ditda/backend/domain/commission/dashboard/repository/projection/RevisingView.javasrc/main/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardService.javasrc/test/java/ditda/backend/domain/commission/dashboard/service/InstructorDashboardServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/ditda/backend/domain/commission/core/service/InstructorCommissionService.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/ditda/backend/domain/commission/dashboard/repository/projection/MatchingView.java
|
까먹고 approve를 안했네요 ^^;; |
🚀 Related issue
Closes #37
#️⃣ Summary
🔧 Changes
GET /api/v1/instructors/commissions/draft-submissionsGET /api/v1/instructors/commissions/matchingsGET/api/v1/instructors/commissions/revisionsrevision_response에 강사가 수정 답변을 확인했는지 여부를 파악하기 위한checked필드 추가commission/core의 controller/facade/service를InstructorXXX로 변경📸 Test Evidence
💬 Reviewer Notes
1.
checked필드수정 중인 외주 조회의 응답에
hasUpadte가 있어서, 강사가 수정 답변을 확인했는지 여부를 트래킹하기 위해revision_response에checked필드를 추가했습니다.추후 강사가 수정 답변을 확인하는 API를 호출하면 이 필드를
true로 바꿔줘야 할 것 같습니다!2.
commission/dashboard분리대시보드 관련 로직을
commission/core에 위치시키게 되면,core->application/revision과 같이 역방향 의존성이 생기게 됩니다.이를 피하기 위해 대시보드 관련 로직은
dashboard패키지로 분리해 구현했습니다.3.
matched계산매칭 중인 외주의
matched는 지원자(디자이너)의 레벨 종류 수입니다.CommissionApplicationRepository에서COUNT(DISTINCT level)로 집계한 뒤,Commission.matchedCount(distinctLevelCount, applicantCount)에서 외주의 실제 모집 인원을 고려해 최종 값을 계산합니다.위와 같이 레벨당 한자리씩 할당해놓고, 잉여 자리는 선착순으로 받도록 했습니다.
이 부분에 대해서는 테스트 코드에 작성해놨습니다.
저는 이렇게 이해했는데, 혹시나 다르게 이해하거나 이상한 부분이 있으면 편하게 말씀해주세요!!!
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항