Merged
Conversation
📝 WalkthroughWalkthroughThis change extends enrollment and lecture enrollment creation flows with automatic student and parent linking capabilities. When creating enrollments, the system now resolves missing student and parent-link IDs by querying repositories with phone, name, and parent phone criteria, either updating existing enrollments or creating new ones with these associations established. Changes
Sequence DiagramssequenceDiagram
participant Client
participant EnrollmentService
participant EnrollmentRepo
participant StudentRepo
participant ParentChildLinkRepo
Client->>EnrollmentService: createEnrollment(request)
activate EnrollmentService
EnrollmentService->>EnrollmentRepo: findManyByInstructorAndPhones
activate EnrollmentRepo
EnrollmentRepo-->>EnrollmentService: existingEnrollment?
deactivate EnrollmentRepo
alt Existing Enrollment Found
EnrollmentService->>StudentRepo: findByPhoneNameParentPhone (if appStudentId null)
activate StudentRepo
StudentRepo-->>EnrollmentService: student
deactivate StudentRepo
EnrollmentService->>ParentChildLinkRepo: findByPhoneNameParentPhone (if appParentLinkId null)
activate ParentChildLinkRepo
ParentChildLinkRepo-->>EnrollmentService: parentLink
deactivate ParentChildLinkRepo
EnrollmentService->>EnrollmentRepo: updateEnrollment with appStudentId/appParentLinkId
activate EnrollmentRepo
EnrollmentRepo-->>EnrollmentService: updated
deactivate EnrollmentRepo
else No Existing Enrollment
par StudentId Resolution
EnrollmentService->>StudentRepo: findByPhoneNameParentPhone
activate StudentRepo
StudentRepo-->>EnrollmentService: student
deactivate StudentRepo
and ParentLinkId Resolution
EnrollmentService->>ParentChildLinkRepo: findByPhoneNameParentPhone
activate ParentChildLinkRepo
ParentChildLinkRepo-->>EnrollmentService: parentLink
deactivate ParentChildLinkRepo
end
EnrollmentService->>EnrollmentRepo: createEnrollment with resolved IDs
activate EnrollmentRepo
EnrollmentRepo-->>EnrollmentService: newEnrollment
deactivate EnrollmentRepo
end
EnrollmentService-->>Client: enrollment
deactivate EnrollmentService
sequenceDiagram
participant Client
participant LecturesService
participant StudentRepo
participant ParentChildLinkRepo
participant EnrollmentRepo
Client->>LecturesService: createLectureEnrollments(request)
activate LecturesService
alt Existing Enrollment with Missing Links
LecturesService->>StudentRepo: findByPhoneNameParentPhone (if appStudentId null)
activate StudentRepo
StudentRepo-->>LecturesService: student
deactivate StudentRepo
LecturesService->>ParentChildLinkRepo: findByPhoneNameParentPhone (if appParentLinkId null)
activate ParentChildLinkRepo
ParentChildLinkRepo-->>LecturesService: parentLink
deactivate ParentChildLinkRepo
LecturesService->>EnrollmentRepo: updateEnrollment
activate EnrollmentRepo
EnrollmentRepo-->>LecturesService: updated
deactivate EnrollmentRepo
else New Enrollment
par Parallel Resolution
LecturesService->>StudentRepo: findByPhoneNameParentPhone
activate StudentRepo
StudentRepo-->>LecturesService: student
deactivate StudentRepo
and
LecturesService->>ParentChildLinkRepo: findByPhoneNameParentPhone
activate ParentChildLinkRepo
ParentChildLinkRepo-->>LecturesService: parentLink
deactivate ParentChildLinkRepo
end
LecturesService->>EnrollmentRepo: createEnrollment with resolved IDs
activate EnrollmentRepo
EnrollmentRepo-->>LecturesService: newEnrollment
deactivate EnrollmentRepo
end
LecturesService-->>Client: lectureEnrollment
deactivate LecturesService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
yoorrll
approved these changes
Mar 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/services/parents.service.test.ts (1)
65-149: Add a failure test for missing parent inregisterChild.Line [78] and Line [127] cover success lookup, but the new NotFound branch is not locked by tests. Add one case where
mockParentRepo.findByIdreturnsnulland assertNotFoundException.🧪 Suggested test case
+ it('학부모 프로필이 없으면 NotFoundException을 던진다', async () => { + mockParentChildLinkRepo.findByParentIdAndPhoneNumber.mockResolvedValue(null); + mockParentRepo.findById.mockResolvedValue(null); + (mockPrisma.$transaction as jest.Mock).mockImplementation(async (fn) => + fn(mockPrisma), + ); + + await expect( + parentsService.registerChild(UserType.PARENT, parentId, childData), + ).rejects.toThrow(NotFoundException); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/parents.service.test.ts` around lines 65 - 149, The tests lack a negative case for parentsService.registerChild when the parent record is missing; add a new unit test that mocks mockParentRepo.findById to resolve to null, then call parentsService.registerChild(UserType.PARENT, parentId, childData) and assert it throws a NotFoundException (or the service's specific error type). Ensure other dependencies (mockParentChildLinkRepo.findByParentIdAndPhoneNumber) are set to avoid short-circuiting, and verify the exception is thrown rather than any successful create or transaction; reference parentsService.registerChild, mockParentRepo.findById, mockParentChildLinkRepo.findByParentIdAndPhoneNumber, and NotFoundException in the test.src/services/lectures.service.test.ts (1)
231-292: Consider a regression test for duplicate phone with different profile fields.This block covers missing-link backfill well. Add one case with same
studentPhonebut differentstudentName/parentPhoneto ensure reuse logic does not target the wrong enrollment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/lectures.service.test.ts` around lines 231 - 292, Add a regression test that ensures reuse logic ignores enrollments that match only on studentPhone but have different studentName or parentPhone: create an existingEnrollment returned by mockEnrollmentsRepo.findManyByInstructorAndPhones with the same studentPhone but different studentName and/or parentPhone and null app links, call lecturesService.createLecture with an enrollmentRequest that has the same phone but different profile fields, then assert mockEnrollmentsRepo.update is NOT called for that existingEnrollment and mockEnrollmentsRepo.createMany is called to create a new enrollment; reference lecturesService.createLecture, mockEnrollmentsRepo.findManyByInstructorAndPhones, mockEnrollmentsRepo.update and mockEnrollmentsRepo.createMany when adding the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/enrollments.service.ts`:
- Around line 140-150: The code currently takes the first match from
enrollmentsRepository.findManyByInstructorAndPhones and reuses it
(existingEnrollments[0]), which can pick the wrong record for shared phones;
instead, in the block that sets enrollmentId/existingEnrollment, perform a
profile-aware selection: search existingEnrollments for an exact match on a
unique student identifier from the incoming data (e.g., data.studentId) or, if
unavailable, match on additional profile fields (email, fullName) before reusing
an enrollment; only set enrollmentId to an existing record if one matches those
profile fields—otherwise create a new enrollment (or return a conflict) to avoid
updating the wrong record. Ensure changes are applied where
enrollmentsRepository.findManyByInstructorAndPhones is consumed and where
enrollmentId/existingEnrollment are assigned.
In `@src/services/lectures.service.ts`:
- Around line 175-199: The code currently uses phone-only lookup when deciding
connections (see connectionData, resolveStudentId, resolveParentLinkId,
existing, enrollmentReq, enrollmentsRepository.update), which can attach the
wrong profile if studentPhone is duplicated; change the resolver calls to use a
composite key (studentPhone + studentName + parentPhone) — either extend
resolveStudentId/resolveParentLinkId to accept and match that composite tuple or
add new helper functions (e.g., resolveStudentIdByCompositeKey /
resolveParentLinkIdByCompositeKey) that perform lookups using all three fields
from enrollmentReq before setting connectionData, and then proceed to call
enrollmentsRepository.update only when the composite match is found.
---
Nitpick comments:
In `@src/services/lectures.service.test.ts`:
- Around line 231-292: Add a regression test that ensures reuse logic ignores
enrollments that match only on studentPhone but have different studentName or
parentPhone: create an existingEnrollment returned by
mockEnrollmentsRepo.findManyByInstructorAndPhones with the same studentPhone but
different studentName and/or parentPhone and null app links, call
lecturesService.createLecture with an enrollmentRequest that has the same phone
but different profile fields, then assert mockEnrollmentsRepo.update is NOT
called for that existingEnrollment and mockEnrollmentsRepo.createMany is called
to create a new enrollment; reference lecturesService.createLecture,
mockEnrollmentsRepo.findManyByInstructorAndPhones, mockEnrollmentsRepo.update
and mockEnrollmentsRepo.createMany when adding the new test.
In `@src/services/parents.service.test.ts`:
- Around line 65-149: The tests lack a negative case for
parentsService.registerChild when the parent record is missing; add a new unit
test that mocks mockParentRepo.findById to resolve to null, then call
parentsService.registerChild(UserType.PARENT, parentId, childData) and assert it
throws a NotFoundException (or the service's specific error type). Ensure other
dependencies (mockParentChildLinkRepo.findByParentIdAndPhoneNumber) are set to
avoid short-circuiting, and verify the exception is thrown rather than any
successful create or transaction; reference parentsService.registerChild,
mockParentRepo.findById, mockParentChildLinkRepo.findByParentIdAndPhoneNumber,
and NotFoundException in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92629bfb-b7bc-413a-a560-dbad9a4db391
📒 Files selected for processing (9)
.gitignoresrc/config/container.config.tssrc/repos/enrollments.repo.tssrc/services/enrollments.service.test.tssrc/services/enrollments.service.tssrc/services/lectures.service.test.tssrc/services/lectures.service.tssrc/services/parents.service.test.tssrc/services/parents.service.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
✨ 변경 사항
이번 PR에서 변경된 내용을 간단히 설명해주세요.
🧪 테스트 방법
리뷰어가 어떻게 테스트하면 되는지 적어주세요.
📸 스크린샷 (선택)
UI 변경이 있다면 첨부해주세요.
✅ 체크리스트
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores