[fix/#38] fix fuking shjit #40
Conversation
WalkthroughThis PR refactors the onboarding flow by replacing EmploymentStatus with AcademicStatus, integrates career edit navigation into the onboarding screen, updates CareerDto ID type from Long to Int, makes MentorItemDto's academicStatus nullable, and updates app icon resources from ic_launcher to ic_main with new drawable assets. Changes
Sequence Diagram(s)sequenceDiagram
participant OnboardingScreen
participant CareerScreen
participant NavController
participant CareerEditScreen
OnboardingScreen->>OnboardingScreen: Receive navController
OnboardingScreen->>CareerScreen: Pass careerList (CareerDto[])
CareerScreen->>CareerScreen: Safe conversion with try-catch
rect rgb(200, 220, 255)
Note over CareerScreen,NavController: Career Selection Flow (NEW)
CareerScreen->>OnboardingScreen: onCareerClick(CareerDto)
OnboardingScreen->>NavController: navigateToCareerEdit(careerId: Int)
NavController->>CareerEditScreen: Route to edit screen
end
rect rgb(240, 240, 240)
Note over OnboardingScreen: State Updates
OnboardingScreen->>OnboardingScreen: updateAcademicStatus(AcademicStatus)
OnboardingScreen->>OnboardingScreen: Store academicStatus in OnboardingRequest
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ 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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkScreen.kt (1)
115-139: Dead code: LinkItem composable is now unused.Since the LazyColumn that calls
LinkItem(line 68) has been commented out, this entire composable is now dead code.If the link display functionality will be re-enabled (as it should be), keep this composable. Otherwise, remove it to avoid maintaining unused code.
app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt (1)
235-256: AvoidacademicStatus!!to prevent potential crashesDereferencing
currentState.academicStatus!!will crash the app if, for any reason, the user reaches this step without having a status selected (or future changes alter the flow). Safer to guard and fail gracefully instead of relying on a hard invariant.You can extract
academicStatuswith a null check before building the request:fun submitOnboarding() { viewModelScope.launch { val currentState = _state.value + val academicStatus = currentState.academicStatus ?: run { + // Option A: expose an error in state and bail out + _state.update { it.copy(apiError = "학적 상태가 선택되지 않았습니다.") } + return@launch + } val request = OnboardingRequest( name = currentState.name, major = currentState.major, studentNumber = currentState.studentId, jobSeeking = currentState.isJobSeeking ?: false, mentor = currentState.wantsMentorship ?: false, - email = currentState.email, - academicStatus = currentState.academicStatus!! + email = currentState.email, + academicStatus = academicStatus )Adjust the error handling message / mechanism to match your UX pattern.
🧹 Nitpick comments (6)
app/src/main/java/com/hsLink/hslink/data/repositoryimpl/search/SearchRepositoryImpl.kt (1)
36-36: Reconsider defaulting null to empty string.While the null-coalescing operator handles the newly nullable
academicStatusfield, defaulting to an empty string might obscure the fact that the data is actually missing. Consider:
- Whether
MentorEntity.academicStatuscould also be nullable to preserve this semantic distinction- If empty string has a special meaning (e.g., "status unknown") vs. a legitimate empty value from the backend
- Consistency with how other optional fields are handled in entity mappings
Verify the target entity definition and consider keeping the nullability:
#!/bin/bash # Check MentorEntity definition to see if academicStatus can be nullable echo "=== Checking MentorEntity definition ===" ast-grep --pattern $'data class MentorEntity( $$$ )' echo -e "\n=== Checking how MentorEntity.academicStatus is used in UI ===" rg -n "mentor\.academicStatus" --type=kt -C 3If
MentorEntity.academicStatuscan be nullable, consider this alternative:- academicStatus = dto.academicStatus ?: "" + academicStatus = dto.academicStatusapp/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/OnboardingRequest.kt (1)
20-22: Add trailing comma for consistency.The
Apply this diff:
@SerialName("email") - val email : String, + val email : String, @SerialName("academicStatus") val academicStatus: AcademicStatusapp/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt (1)
29-30: Remove commented code.The commented line should be removed to keep the codebase clean.
Apply this diff:
- //careerList: List<CareerItemEntity>, careerList: List<CareerDto>,app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmploymentStatusScreen.kt (1)
37-42: Consider updating the title text for clarity.The title asks "현재 재직 중이신가요?" (Are you currently employed?), but the field now uses
AcademicStatuswhich includes academic statuses like "재학 중" (enrolled), "졸업" (graduated), "휴학" (leave), etc. Consider updating the title to better reflect academic status rather than employment status.Example:
title = buildAnnotatedString { append("현재 학적 상태를 알려주세요 ") withStyle(style = SpanStyle(color = HsLinkTheme.colors.Red500)) { append("*") } }app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt (1)
224-227: NewupdateAcademicStatuslooks good; consider cleaning up commented codeThe new
updateAcademicStatuscorrectly updatesacademicStatusin state and replaces the old employment status path. The commented-outupdateEmploymentStatusis now dead code and can be removed to reduce noise and avoid confusion about which path is active.- //fun updateEmploymentStatus(status: EmploymentStatus) { _state.update { it.copy(employmentStatus = status) } } - fun updateAcademicStatus(status: AcademicStatus) { - _state.update { it.copy(academicStatus = status) } // ← employmentStatus → academicStatus - } + fun updateAcademicStatus(status: AcademicStatus) { + _state.update { it.copy(academicStatus = status) } + }app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt (1)
106-137: Career list mapping + edit navigation mostly solid; consider refining error handlingThe new
safeCareerListmapping toCareerDtoand theonCareerClicknavigation vianavController.navigateToCareerEdit(career.id)correctly align with the updated career edit API (IntcareerId) andCareerScreen’sonCareerClickcontract.The broad
try { state.careerList.map { ... } } catch (e: Exception)that falls back toemptyList()can completely hide partially valid data if a single item is problematic, which may make debugging harder and silently degrade UX.You might prefer a more targeted, item‑level safeguard so one bad item doesn’t drop the entire list, for example:
- val safeCareerList = try { - state.careerList.map { career -> - CareerDto( - id = career.id, - companyName = career.companyName, - position = career.position, - jobType = career.jobType, - employed = career.employed, - startYm = career.startYm, - endYm = career.endYm - ) - } - } catch (e: Exception) { - Log.e("OnboardingRoute", "타입 변환 실패", e) - emptyList() - } + val safeCareerList = state.careerList.mapNotNull { career -> + try { + CareerDto( + id = career.id, + companyName = career.companyName, + position = career.position, + jobType = career.jobType, + employed = career.employed, + startYm = career.startYm, + endYm = career.endYm + ) + } catch (e: Exception) { + Log.e("OnboardingRoute", "타입 변환 실패: $career", e) + null + } + }This keeps valid entries visible while still logging and skipping any that fail conversion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/ic_main-playstore.pngis excluded by!**/*.png
📒 Files selected for processing (17)
app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/OnboardingRequest.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/search/SearchResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/search/SearchRepositoryImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt(4 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmploymentStatusScreen.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/model/OnboardingState.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/navigation/OnboardingNavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt(5 hunks)app/src/main/res/drawable/ic_main_background.xml(1 hunks)app/src/main/res/mipmap-anydpi-v26/ic_main.xml(1 hunks)app/src/main/res/mipmap-anydpi-v26/ic_main_round.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (1)
CareerEditRoute(56-93)
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt (1)
careerNavGraph(17-32)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/navigation/OnboardingNavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt (1)
OnboardingRoute(45-224)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt (1)
CareerScreen(26-134)
⏰ 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-and-test
🔇 Additional comments (14)
app/src/main/res/drawable/ic_main_background.xml (1)
1-74: Well-formed adaptive icon background drawable.The vector drawable is properly structured with valid SVG path syntax, appropriate use of opacity (0x33 alpha on white strokes), and clean geometric design. The green background and subtle grid pattern align with modern Material Design principles.
app/src/main/res/mipmap-anydpi-v26/ic_main.xml (1)
1-5: Adaptive icon structure is correct.The XML properly defines an adaptive icon for API 26+. The background and foreground references follow Android conventions. Depends on verification that ic_main_foreground resource exists (flagged in companion review comment for ic_main_round.xml).
app/src/main/res/mipmap-anydpi-v26/ic_main_round.xml (1)
1-5: Adaptive icon correctly references existing foreground resource.Verification confirms @mipmap/ic_main_foreground is properly defined across all required density buckets (hdpi, mdpi, xhdpi, xxhdpi, xxxhdpi). The adaptive icon will render correctly.
app/src/main/AndroidManifest.xml (1)
12-12: Icon resource updates verified and correct.Verification confirms all new ic_main resources exist and are properly configured: ic_main_background.xml, adaptive icon definitions (ic_main.xml and ic_main_round.xml), and corresponding mipmap assets across all density buckets. Icon dependencies in the adaptive icon XML files correctly reference @mipmap/ic_main_foreground and @drawable/ic_main_background.
The manifest changes are safe and functional. Old ic_launcher resources remain in the codebase but are no longer referenced—these can be removed in a future cleanup if desired.
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (1)
164-172: LGTM!The integration of career navigation into the onboarding flow is properly implemented. The
navControlleris correctly passed through to enable navigation to career edit screens.app/src/main/java/com/hsLink/hslink/presentation/onboarding/navigation/OnboardingNavigation.kt (1)
22-29: LGTM!The
navControllerparameter addition is correctly implemented and aligns with the changes inMainActivity.ktandOnboardingScreen.kt.app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt (1)
75-88: LGTM!The integration of
CareerCardwithCareerDtois correctly implemented. The date range formatting properly handles both employed and completed careers.app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmploymentStatusScreen.kt (1)
29-32: LGTM!The type migration from
EmploymentStatustoAcademicStatusis correctly implemented throughout the component.Also applies to: 55-67
app/src/main/java/com/hsLink/hslink/presentation/onboarding/model/OnboardingState.kt (1)
11-11: Migration from EmploymentStatus to AcademicStatus is complete.All enum type references have been properly updated to
AcademicStatus. The ripgrep search shows no problematic references—only the screen component's nameEmploymentStatusScreenremains, which is a UI naming convention unrelated to the enum type. The commented-out legacy function in the ViewModel is acceptable cleanup.app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt (2)
9-9: AcademicStatus import aligns with new state and UI usageThe new
AcademicStatusimport is correct and matches the updated state + UI wiring elsewhere in this PR. No issues here.
110-156: No behavioral change in CareerRequest constructionThe only change in this block is the trailing comma on
employed = s.tempIsEmployed,which doesn’t affect behavior. The overall request construction and submission flow remain consistent.app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt (3)
3-30: Imports for NavController, CareerDto, and logging are consistent with new behaviorThe added imports (
NavController,CareerDto,navigateToCareerEdit, andLog) match the new navigation and error‑logging behavior introduced in this file and are used appropriately in the code below.
95-103: Academic status wiring into EmploymentStatusScreen looks correct
EmploymentStatusScreennow readsselectedStatus = state.academicStatusand writes back throughviewModel::updateAcademicStatus, which is consistent with the ViewModel and request changes. Behavior should mirror the old employmentStatus flow with the new type.
46-56: All OnboardingRoute call sites already pass the navController parameter correctlyVerification confirms both usages at
OnboardingNavigation.kt:25andMainActivity.kt:161are already updated to passnavController = navController. No compile errors should occur from this change.
| @Serializable | ||
| data class CareerDto( | ||
| val id: Long, | ||
| val id: Int, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify API contract supports Int range for career IDs.
The change from Long to Int could cause issues if the backend returns IDs exceeding Int.MAX_VALUE (2,147,483,647). Ensure the API specification guarantees career IDs will remain within the Int range.
🏁 Script executed:
#!/bin/bash
# Search for career ID usages that might still expect Long
rg -nP '\bcareer.*[iI]d.*Long\b|\bLong.*career.*[iI]d\b' --type=kotlin -C3Length of output: 8992
Update careerId type from Long to Int across repository, service, and view model layers.
The change to use Int in CareerResponse.kt is incompletely migrated. Multiple layers still expect Long:
- CareerRepository.kt:
getCareer(careerId: Long)andupdateCareer(careerId: Long, ...) - CareerService.kt: Service methods with
@Path("careerId") careerId: Long - CareerRepositoryImpl.kt: Implementation methods still using
Long - CareerViewModel.kt:
updateCareer(careerId: Long)andloadCareer(careerId: Long) - CareerEditScreen.kt: Route parameter
careerId: Long?
Workarounds like .toLong() conversions in navigation don't address the underlying type inconsistency. Update all method signatures and parameter types to use Int consistently.
🤖 Prompt for AI Agents
app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt around
line 31: the repository/service/viewmodel layers are still using Long for
careerId while CareerResponse now uses Int; update all method signatures,
parameters and route types to Int to maintain consistency — change
CareerRepository.kt (getCareer, updateCareer) and CareerRepositoryImpl.kt method
parameter types to Int, change CareerService.kt path parameter annotations and
method signatures to use Int, update CareerViewModel.kt (loadCareer,
updateCareer) and CareerEditScreen.kt route parameter parsing to Int (and remove
any .toLong() conversions), and ensure any callers and tests are adjusted
accordingly and project builds cleanly.
| val jobSeeking: Boolean, | ||
| val employed: Boolean, | ||
| val academicStatus: String | ||
| val academicStatus: String? = null |
There was a problem hiding this comment.
🧩 Analysis chain
Verify backend API compatibility and null handling.
Making academicStatus nullable is a significant API change. While this likely addresses the HTTP 500 errors mentioned in the PR description by handling missing fields gracefully, please confirm:
- The backend API indeed returns null or omits this field in some responses
- All consumers of
MentorItemDtoproperly handle nullacademicStatusvalues
Run the following script to check how academicStatus is used across the codebase:
🏁 Script executed:
#!/bin/bash
# Check MentorItemDto usage and academicStatus handling
echo "=== Searching for MentorItemDto usage ==="
rg -n "MentorItemDto" --type=kt -C 3
echo -e "\n=== Searching for academicStatus field access ==="
rg -n "\.academicStatus" --type=kt -C 2Length of output: 146
🏁 Script executed:
#!/bin/bash
# Check MentorItemDto usage and academicStatus handling using corrected syntax
echo "=== Searching for MentorItemDto usage ==="
rg -n "MentorItemDto" -C 3
echo -e "\n=== Searching for academicStatus field access ==="
rg -n "\.academicStatus" -C 2
echo -e "\n=== Searching for SearchResponseDto and related DTOs ==="
rg -n "SearchResponseDto\|MentorItemDto" -C 2Length of output: 6100
🏁 Script executed:
#!/bin/bash
# Check the function implementations that consume academicStatus
echo "=== Searching for buildStatusText function definition ==="
rg -n "fun buildStatusText" -C 5
echo -e "\n=== Searching for getAcademicStatusText function definition ==="
rg -n "fun getAcademicStatusText" -C 5
echo -e "\n=== Checking MentorItemDto data class full definition ==="
rg -n "data class MentorItemDto" -A 15Length of output: 6221
Fix null handling in function consumers before release.
The change to make academicStatus nullable introduces type incompatibilities at multiple call sites:
SearchScreen.kt:196passes nullablementor.academicStatustobuildStatusText()which expects non-nullStringProfileCard.kt:174passes nullableprofile.academicStatustogetAcademicStatusText()which expects non-nullStringMypageScreen.kt:175passes nullableuserSummary.academicStatustobuildStatusText()which expects non-nullString
Update function signatures to accept String? or apply null-coalescing (e.g., ?: "") at call sites to match the non-null consumer functions' expectations.
🤖 Prompt for AI Agents
app/src/main/java/com/hsLink/hslink/data/dto/response/search/SearchResponseDto.kt
around line 27: academicStatus was made nullable (val academicStatus: String? =
null) but several consumers still require non-null String, causing type errors;
either update the consumer function signatures (buildStatusText,
getAcademicStatusText) to accept String? and handle null internally, or keep
consumers as-is and coalesce the nullable value at each call site (e.g., pass
academicStatus ?: "" or provide a sensible default). Modify only the call sites
in SearchScreen.kt:196, ProfileCard.kt:174, and MypageScreen.kt:175 or the three
consumer functions consistently so all usages compile and nulls are handled
explicitly.
| fun NavController.navigateToCareerEdit(careerId: Int, navOptions: NavOptions? = null) { | ||
| navigate(CareerEdit(careerId = careerId), navOptions) |
There was a problem hiding this comment.
🧩 Analysis chain
Incomplete type migration creates inconsistency.
The navigation layer now uses Int for careerId, but CareerEditRoute still expects Long?, requiring a conversion on line 29. This partial migration creates type inconsistency.
Consider completing the migration by updating CareerEditRoute and related components to use Int consistently. This would eliminate the need for type conversions and improve code clarity.
Also applies to: 23-29, 35-35
🏁 Script executed:
#!/bin/bash
# Find CareerEditRoute and related components that might need Int migration
rg -nP 'CareerEditRoute|careerId.*Long' --type=kotlin -C5Length of output: 14660
Let me search directly in the repository:
Let me also check for CareerEdit:
Based on the script output from earlier, I can see line 35 shows:
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt-34-@Serializable
Let me search for the full CareerEdit definition:
Type boundary change creates clarity issue, but verify design intent.
The migration changed navigation to use Int (line 13) while CareerEditRoute still expects Long? (line 60), requiring conversion on line 29. This pattern suggests an intentional type boundary—using smaller Int at UI layer while maintaining Long in business logic.
Before enforcing the suggested refactor to use Int consistently, clarify:
- Is this intentional layer separation (UI: Int, data: Long) or incomplete migration?
- If intentional, document this boundary decision in code comments for clarity
- If incomplete, update both navigation and CareerEditRoute to use the same type
🏁 Script executed:
#!/bin/bash
# Find CareerDto definition to check id field type
fd -e '.kt' | xargs grep -l 'data class CareerDto\|class CareerDto'Length of output: 71
🏁 Script executed:
#!/bin/bash
# Find CareerDto definition
rg -nP 'data class CareerDto|class CareerDto' --type=kotlin -A10 | head -50Length of output: 1023
🏁 Script executed:
#!/bin/bash
# Find CareerEdit serializable data class
rg -nP '@Serializable.*\ndata class CareerEdit|@Serializable\s+data class CareerEdit' --type=kotlin -A5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for CareerEdit more broadly
rg -nP 'data class CareerEdit|CareerEdit\s*\(' --type=kotlin -B2 -A2Length of output: 3400
Complete the type migration: update CareerEditRoute to accept Int instead of Long.
The migration changed navigation to Int (CareerDto.id, CareerEdit class, navigateToCareerEdit function), but CareerEditRoute parameter still expects Long?. This requires an unnecessary .toLong() conversion on line 29. Update CareerEditRoute signature to accept Int? (CareerEditScreen.kt line 60) to align with the navigation layer and eliminate the type conversion.
CareerEditRoute(
paddingValues = padding,
navController = navController,
- careerId = careerEdit.careerId.toLong() // removed: no longer needed
+ careerId = careerEdit.careerId
)
Separately, verify that the downstream ViewModel/Repository layer can accept Int or add conversion boundary at the ViewModel entry point if Long is required by backend.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt
around lines 13-14 and related CareerEditScreen.kt at ~line 60, the navigation
layer was migrated to use Int for careerId but CareerEditRoute still expects
Long?, forcing a .toLong() conversion; update CareerEditRoute signature to
accept Int? instead of Long? (change parameter type in CareerEditScreen.kt at
the Route composable) and remove the unnecessary .toLong() call where
CareerEditRoute is invoked (pass careerEdit.careerId directly), then verify
downstream ViewModel/repository boundaries accept Int or perform conversion at
the ViewModel entry point if backend requires Long.
| // if (linkList.isNotEmpty()) { | ||
| // LazyColumn( | ||
| // modifier = Modifier.fillMaxSize(), | ||
| // verticalArrangement = Arrangement.spacedBy(12.dp) | ||
| // ) { | ||
| // items(linkList) { link -> | ||
| // LinkItem(link) | ||
| // } | ||
| // } | ||
| // } |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: Link display functionality is completely disabled.
Commenting out the link list rendering removes essential functionality from the onboarding flow. Users can still add links via the button below (line 103-107), but they cannot view what they've added. This creates a confusing and broken user experience.
Based on the PR description mentioning HTTP 500 errors, this appears to be a temporary workaround rather than a proper fix. However:
- This should not be merged as-is - it degrades functionality without providing a solution
- If this is intentionally temporary, add a clear TODO/FIXME comment explaining:
- Why it was disabled
- What needs to be fixed
- When it will be re-enabled
- Consider adding a loading state or error boundary instead of hiding the feature entirely
If the underlying issue is related to API errors when fetching/displaying links, please verify the actual error and implement proper error handling:
🏁 Script executed:
#!/bin/bash
# Search for LinkItemEntity usage and API calls that might be causing HTTP 500 errors
rg -n "LinkItemEntity" --type=kotlin -C3
# Search for related API/network calls in the onboarding flow
rg -n "linkList|getLlinks|fetchLinks" --type=kotlin -C3Length of output: 15859
Critical: Link display functionality is completely disabled.
Commenting out the link list rendering removes essential functionality from the onboarding flow. Users can still add links via the button below (lines 103-107), but they cannot view what they've added. This creates a confusing and broken user experience.
Based on the PR description mentioning HTTP 500 errors, this appears to be a temporary workaround. However:
- This should not be merged as-is — it degrades functionality without providing a solution
- If this is intentionally temporary, add a clear TODO/FIXME comment explaining:
- Why it was disabled
- What needs to be fixed
- When it will be re-enabled
- Consider adding a loading state or error boundary instead of hiding the feature entirely
The underlying infrastructure is intact (the getLinks() API call and LinkItem composable both exist), so the proper fix is to restore the rendering and handle any API errors appropriately.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkScreen.kt
around lines 62-71 the LazyColumn that renders linkList was commented out,
removing link display; restore the original rendering block (re-enable the if
(linkList.isNotEmpty()) { LazyColumn ... items(linkList) { LinkItem(link) } } )
so users can see added links, and add proper error/loading handling around the
data source (show a loading indicator while fetching, show an error message or
retry UI if getLinks() fails) to avoid re-commenting UI for backend errors; if
this disablement was intended as a temporary workaround, instead of deleting the
UI add a TODO/FIXME above the restored block stating why it was disabled, what
needs to be fixed, and the target re-enable condition.
|
고생했으요.. 온보딩은 본선가면 내가 다 뜯어 고칠게.. |
ISSUE
❗ WORK DESCRIPTIONAdd commentMore actions
📸 SCREENSHOT
시연영상으로 단톡방에오ㅓㄹ리겠습니다
📢 TO REVIEWERS
Summary by CodeRabbit
New Features
Chores
Bug Fixes