Merged
Conversation
1 task
Walkthrough경로 변수나 요청 파라미터로 전달된 숫자, 초대 토큰 또는 초대 URL을 Long meetingId로 변환하는 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@ssolv-api-common/src/main/kotlin/org/depromeet/team3/common/resolver/MeetingIdArgumentResolver.kt`:
- Around line 16-20: The supportsParameter method incorrectly evaluates as true
for primitive Long parameters even when `@MeetingId` is absent; update the boolean
expression in supportsParameter so the annotation check is grouped with the type
checks (i.e., require parameter.hasParameterAnnotation(MeetingId::class.java)
AND one of the Long type checks) to prevent matching parameters without the
MeetingId annotation; modify the condition in supportsParameter (the
MethodParameter-based check) to use parentheses around the annotation check and
the OR of Long::class.javaObjectType / Long::class.java so only parameters
annotated with MeetingId pass.
In
`@ssolv-api-core/src/main/kotlin/org/depromeet/team3/meetingattendee/controller/AttendeeController.kt`:
- Around line 30-31: The `@MeetingId` annotation on the meetingId parameter in
AttendeeController is missing its path-variable value and should match the route
placeholder; update the annotation on the parameter (the method parameter named
meetingId: Long in AttendeeController) to include the path name (e.g.,
`@MeetingId`("meetingId") or value = "meetingId") just like SurveyController so
the binding matches the "{meetingId}" path mapping.
In
`@ssolv-api-core/src/main/kotlin/org/depromeet/team3/survey/controller/SurveyController.kt`:
- Around line 37-38: The `@MeetingId` annotation on the meetingId parameter in
SurveyController (parameter annotated as "@MeetingId meetingId: Long") is
missing its path-variable identifier; update it to include the same identifier
used in the route (e.g., `@MeetingId`("identifier")) so MeetingIdArgumentResolver
can locate the path variable correctly — modify the parameter in
SurveyController.kt to use the explicit identifier string consistent with other
controllers (see MeetingController) and ensure the name matches the
route/path-variable name the resolver expects.
In
`@ssolv-global-utils/src/main/kotlin/org/depromeet/team3/util/MeetingIdParser.kt`:
- Around line 13-20: The current parse in MeetingIdParser (fun parse(identifier:
String): Long) uses identifier.substringAfter("token=") when
identifier.startsWith("http") which returns the whole URL if "token=" is absent;
update the logic to first check identifier.contains("token=") when
startsWith("http") and only use substringAfter("token=") in that case, otherwise
handle the missing token explicitly (e.g., call parseToken(identifier) or throw
a clear IllegalArgumentException) so a full URL is not mistakenly treated as the
token.
...-api-common/src/main/kotlin/org/depromeet/team3/common/resolver/MeetingIdArgumentResolver.kt
Show resolved
Hide resolved
...pi-core/src/main/kotlin/org/depromeet/team3/meetingattendee/controller/AttendeeController.kt
Outdated
Show resolved
Hide resolved
ssolv-api-core/src/main/kotlin/org/depromeet/team3/survey/controller/SurveyController.kt
Outdated
Show resolved
Hide resolved
ssolv-global-utils/src/main/kotlin/org/depromeet/team3/util/MeetingIdParser.kt
Show resolved
Hide resolved
🧪 테스트 결과164 tests 164 ✅ 26s ⏱️ Results for commit 17db856. ♻️ This comment has been updated with latest results. |
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
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.
🎋 이슈 및 작업중인 브랜치
🔑 주요 내용
Check List
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.