Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR removes the AssignmentCategory model and related infrastructure, replacing the category-based organization with a direct Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/assignment-results.service.test.ts (1)
117-117:⚠️ Potential issue | 🟡 MinorRemove the stale developer-note comment.
This comment openly admits that some
createResulttest cases were omitted. It should not be committed — either remove it (if coverage is intentionally limited) or add the missing test cases it references.🧹 Proposed fix
- // ... other createResult tests remain logically similar, omitted for brevity but should be included if rewriting whole file ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/assignment-results.service.test.ts` at line 117, Remove the stale developer-note comment that admits omitted createResult tests in the test file; locate the comment in src/services/assignment-results.service.test.ts near the createResult test suite (references to createResult tests) and either delete that developer-note line entirely or replace it by adding the missing test cases it references so the file no longer contains a comment claiming tests were omitted.
🧹 Nitpick comments (2)
src/services/grades.service.ts (1)
507-525: Consider removing the deadcategoryName: nullfield from the response shape
categoryNameis hardcoded tonulland has no downstream logic — it's kept solely in the API response object. Leaving it creates an ever-null field that could confuse future maintainers or API consumers.If no frontend currently reads
categoryName, the cleanest approach is to drop it from the returned object. If backward compat is needed, leave it as-is but document the deprecation.♻️ Optional cleanup
return { assignmentId: assignment.id, title: assignment.title, - categoryName: null, resultIndex: studentResult?.resultIndex ?? null, resultLabel, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/grades.service.ts` around lines 507 - 525, The returned object in the assignments map contains a dead field categoryName set to null; remove the categoryName: null property from the object returned in the assignments variable (inside the assignmentsOnExamReport.map callback) and update any related types/DTOs or interfaces that describe this response shape (e.g., the assignments response type or ExamReportAssignment DTO) so callers and type-checking reflect the removal; if backward compatibility is required instead, add a clear deprecation comment near the property and keep it unchanged.src/services/assignments.service.ts (1)
9-12: Remove unusedprismadependency from constructor.
PrismaClientis injected but never referenced in any method. Remove it from the constructor and update the container wiring at line 313 ofsrc/config/container.config.ts:♻️ Changes required
--- a/src/services/assignments.service.ts +++ b/src/services/assignments.service.ts @@ -8,7 +8,6 @@ import { export class AssignmentsService { constructor( private readonly assignmentsRepo: AssignmentsRepository, - private readonly prisma: PrismaClient, ) {}--- a/src/config/container.config.ts +++ b/src/config/container.config.ts @@ -310,7 +310,7 @@ const assignmentResultsRepo = new AssignmentResultsRepository(prisma); -const assignmentsService = new AssignmentsService(assignmentsRepo, prisma); +const assignmentsService = new AssignmentsService(assignmentsRepo);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/assignments.service.ts` around lines 9 - 12, The constructor of AssignmentsService currently injects a PrismaClient that is never used; remove the prisma: PrismaClient parameter and its import from src/services/assignments.service.ts, update the AssignmentsService constructor to only accept private readonly assignmentsRepo: AssignmentsRepository, and then update the DI/container registration that instantiates AssignmentsService to stop passing the PrismaClient (remove the prisma argument from the provider/factory that constructs AssignmentsService). Ensure imports and types are cleaned up so no unused PrismaClient references remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/services/assignment-results.service.test.ts`:
- Line 117: Remove the stale developer-note comment that admits omitted
createResult tests in the test file; locate the comment in
src/services/assignment-results.service.test.ts near the createResult test suite
(references to createResult tests) and either delete that developer-note line
entirely or replace it by adding the missing test cases it references so the
file no longer contains a comment claiming tests were omitted.
---
Nitpick comments:
In `@src/services/assignments.service.ts`:
- Around line 9-12: The constructor of AssignmentsService currently injects a
PrismaClient that is never used; remove the prisma: PrismaClient parameter and
its import from src/services/assignments.service.ts, update the
AssignmentsService constructor to only accept private readonly assignmentsRepo:
AssignmentsRepository, and then update the DI/container registration that
instantiates AssignmentsService to stop passing the PrismaClient (remove the
prisma argument from the provider/factory that constructs AssignmentsService).
Ensure imports and types are cleaned up so no unused PrismaClient references
remain.
In `@src/services/grades.service.ts`:
- Around line 507-525: The returned object in the assignments map contains a
dead field categoryName set to null; remove the categoryName: null property from
the object returned in the assignments variable (inside the
assignmentsOnExamReport.map callback) and update any related types/DTOs or
interfaces that describe this response shape (e.g., the assignments response
type or ExamReportAssignment DTO) so callers and type-checking reflect the
removal; if backward compatibility is required instead, add a clear deprecation
comment near the property and keep it unchanged.
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
Removed Features
New Features