-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/#32] 온보딩 api #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR implements comprehensive onboarding API integration across data, domain, and presentation layers. It introduces new service, data source, and repository abstractions for onboarding operations, refactors onboarding screens to use entity-based models, and updates navigation to support authentication flows alongside app main flows. Changes
Sequence DiagramsequenceDiagram
actor User
participant OnboardingScreen
participant OnboardingViewModel
participant OnboardingRepository
participant OnboardingDataSource
participant OnboardingService
participant API
User->>OnboardingScreen: Enters onboarding flow
OnboardingScreen->>OnboardingViewModel: moveToNextStep()
alt Step: Email
OnboardingScreen->>OnboardingViewModel: updateEmail(email)
else Step: Career (Experienced Path)
OnboardingViewModel->>OnboardingRepository: getCareers()
OnboardingRepository->>OnboardingDataSource: getCareers()
OnboardingDataSource->>OnboardingService: getCareers()
OnboardingService->>API: GET /careers/mycareers
API-->>OnboardingService: List<CareerResponse>
OnboardingService-->>OnboardingDataSource: BaseResponse<List<CareerResponse>>
OnboardingDataSource-->>OnboardingRepository: BaseResponse<List<CareerResponse>>
OnboardingRepository-->>OnboardingViewModel: Result<List<CareerItemEntity>>
OnboardingViewModel->>OnboardingScreen: Display careerList
User->>OnboardingScreen: Click "Add Career"
OnboardingScreen->>OnboardingViewModel: openJobInfoForm()
User->>OnboardingScreen: Fill job info (temp fields)
User->>OnboardingScreen: Submit career
OnboardingScreen->>OnboardingViewModel: submitJobInfoAndReturnToCareerList()
OnboardingViewModel->>OnboardingRepository: submitCareer(CareerRequest)
OnboardingRepository->>OnboardingDataSource: submitCareer(CareerRequest)
OnboardingDataSource->>OnboardingService: submitCareer(CareerRequest)
OnboardingService->>API: POST /careers (CareerRequest)
API-->>OnboardingService: CareerResponse
OnboardingService-->>OnboardingDataSource: BaseResponse<CareerResponse>
OnboardingDataSource-->>OnboardingRepository: BaseResponse<CareerResponse>
OnboardingRepository->>OnboardingRepository: Refresh careers/links
OnboardingRepository-->>OnboardingViewModel: Result<CareerItemEntity>
OnboardingViewModel->>OnboardingScreen: Update state with new career
else Step: Final Submission
User->>OnboardingScreen: Confirm all fields
OnboardingScreen->>OnboardingViewModel: submitOnboarding()
OnboardingViewModel->>OnboardingRepository: submitOnboarding(OnboardingRequest)
OnboardingRepository->>OnboardingDataSource: submitOnboarding(OnboardingRequest)
OnboardingDataSource->>OnboardingService: onboarding(OnboardingRequest)
OnboardingService->>API: POST /auth/onboarding (OnboardingRequest)
API-->>OnboardingService: BaseResponse<Unit>
OnboardingService-->>OnboardingDataSource: BaseResponse<Unit>
OnboardingDataSource-->>OnboardingRepository: BaseResponse<Unit>
OnboardingRepository-->>OnboardingViewModel: Result<Unit>
end
OnboardingViewModel-->>OnboardingScreen: State updates propagate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Key areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/JobInfoScreen.kt (1)
44-167: Department input is missing from the form.
department/onDepartmentChangewere added to the public API but never rendered, so the user has no way to enter or edit this required field. This regresses onboarding because department always stays blank. Please surface the department text field (with its own focus state) in the form.@@ - var positionFocused by remember { mutableStateOf(false) } + var positionFocused by remember { mutableStateOf(false) } + var departmentFocused by remember { mutableStateOf(false) } @@ Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { Text( text = buildAnnotatedString { append("직무명 ") withStyle(style = SpanStyle(color = HsLinkTheme.colors.Red500)) { @@ HsLinkTextField( value = position, placeholder = "직무명을 입력해주세요 (예: Android 개발자)", onValueChanged = onPositionChange, borderColor = if (positionFocused) HsLinkTheme.colors.SkyBlue500 else HsLinkTheme.colors.Grey300, backgroundColor = HsLinkTheme.colors.Common, onFocusChanged = { positionFocused = it }, modifier = Modifier.fillMaxWidth() ) } + + Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { + Text( + text = buildAnnotatedString { + append("부서명 ") + withStyle(style = SpanStyle(color = HsLinkTheme.colors.Red500)) { + append("*") + } + }, + style = HsLinkTheme.typography.title_14Strong, + color = HsLinkTheme.colors.Grey700 + ) + HsLinkTextField( + value = department, + placeholder = "부서를 입력해주세요 (예: 개발팀)", + onValueChanged = onDepartmentChange, + borderColor = if (departmentFocused) HsLinkTheme.colors.SkyBlue500 else HsLinkTheme.colors.Grey300, + backgroundColor = HsLinkTheme.colors.Common, + onFocusChanged = { departmentFocused = it }, + modifier = Modifier.fillMaxWidth() + ) + }
🧹 Nitpick comments (3)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/YearMonthTransform.kt (2)
48-54: Consider consolidating duplicate formatting logic.The
toYearMonthFormat()function duplicates the formatting logic fromYearMonthVisualTransformation.filter(). Consider extracting this to a shared private function to follow the DRY principle.private fun formatYearMonth(input: String): String { val digits = input.filter { it.isDigit() }.take(6) return when { digits.length <= 4 -> digits else -> "${digits.substring(0, 4)}-${digits.substring(4)}" } } fun String.toYearMonthFormat(): String = formatYearMonth(this)
56-70: Minor documentation inconsistency.The KDoc comment states "yyyy-MM 또는 yyyy.MM 형식 검증" (validation for yyyy-MM or yyyy.MM format), but the regex pattern
^\d{4}[-./]\d{2}$also accepts forward slash (/) as a separator. Either update the documentation to mention all three separators or restrict the regex to only[-.]if slash is not intended.app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt (1)
10-33: Consider removing redundant @SerialName annotations.All
@SerialNameannotations use the exact same value as the field name. Since kotlinx.serialization automatically maps field names to JSON keys, these annotations are redundant unless you need to protect against future field name refactoring.Example simplification:
@Serializable data class CareerResponse( - @SerialName("id") val id: Int, - @SerialName("companyName") val companyName: String, - @SerialName("position") val position: String, - @SerialName("jobType") val jobType: JobType, - @SerialName("employed") val employed: Boolean, - @SerialName("startYm") val startYm: String, - @SerialName("endYm") val endYm: String?, + val id: Int, + val companyName: String, + val position: String, + val jobType: JobType, + val employed: Boolean, + val startYm: String, + val endYm: String?, )Apply similar changes to
LinkResponseandLinkListResponseDto.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
app/src/main/java/com/hsLink/hslink/core/navigation/AppRoute.kt(0 hunks)app/src/main/java/com/hsLink/hslink/data/di/DataSourceModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/CareerRequest.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/OnboardingRequest.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasource/OnboardingDataSource.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/remote/datasourceimpl/OnboardingDataSourceImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/onboarding/OnboardingRepositoryImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/onboarding/OnboardingService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/model/search/UserProfileEntity.kt(3 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/onboarding/OnboardingRepository.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt(4 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt(4 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt(8 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/YearMonthTransform.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt(4 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmailScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/JobInfoScreen.kt(5 hunks)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkAddScreen.kt(1 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/viewmodel/OnboardingViewModel.kt(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/hsLink/hslink/core/navigation/AppRoute.kt
🧰 Additional context used
🧬 Code graph analysis (6)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/JobInfoScreen.kt (2)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmailScreen.kt (2)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (2)
app/src/main/java/com/hsLink/hslink/presentation/main/MainScreen.kt (1)
MainScreen(12-35)app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt (1)
KaKaoLoginScreen(41-161)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkAddScreen.kt (2)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/OnboardingScreen.kt (5)
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt (1)
CareerScreen(24-117)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/JobInfoScreen.kt (1)
JobInfoScreen(22-220)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/EmailScreen.kt (1)
EmailScreen(26-80)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkScreen.kt (1)
LinksScreen(32-112)app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/LinkAddScreen.kt (1)
LinkAddScreen(29-124)
🔇 Additional comments (12)
app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/CareerRequest.kt (2)
8-22: Serialization setup looks good.The
CareerRequestdata class is properly configured with@Serializableand@SerialNameannotations for all fields, supporting correct JSON serialization.
24-30: Serialization setup looks good.The
LinkRequestdata class is properly configured with@Serializableand@SerialNameannotations, following the same pattern asCareerRequest.app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/OnboardingRequest.kt (1)
6-20: LGTM!The
OnboardingRequestdata class is properly structured with@Serializableannotation and all fields correctly mapped with@SerialNamefor JSON serialization.app/src/main/java/com/hsLink/hslink/domain/model/search/UserProfileEntity.kt (2)
19-27: Entity structure is well-defined.
CareerItemEntityhas a clear structure with appropriate fields for representing career information in the domain layer.
45-49: Entity structure is well-defined.
LinkItemEntityfollows a clean and simple structure appropriate for representing link information in the domain layer.app/src/main/java/com/hsLink/hslink/data/di/DataSourceModule.kt (1)
35-38: LGTM!The
bindsOnboardingDataSourcebinding follows the established pattern in this module and correctly wiresOnboardingDataSourceImplto theOnboardingDataSourceinterface for dependency injection.app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
74-78: LGTM!The
provideOnboardingServiceprovider follows the established pattern for service provisioning in this module, correctly using@Providesand@Singletonannotations to create the Retrofit service instance.app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt (1)
51-54: LGTM!The
bindsOnboardingRepositorybinding follows the established pattern and correctly wiresOnboardingRepositoryImplto theOnboardingRepositoryinterface for dependency injection.app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (2)
79-79: Consistent refactoring of JobType values.The changes from
FULL_TIME/CONTRACTtoPERMANENT/TEMPORARYare applied consistently across default value, validation logic, and UI labels. The refactoring maintains correctness.Also applies to: 93-93, 257-261, 265-269
128-129: Good improvement to exit handling.Consolidating both the back arrow and close button to use
handleExit()ensures consistent behavior for unsaved changes dialog, improving the user experience.app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/YearMonthTransform.kt (1)
12-43: Visual transformation implementation looks correct.The
YearMonthVisualTransformationproperly formats digit input toyyyy-MMformat by inserting a dash after the 4th character, and theOffsetMappingcorrectly handles cursor position mapping between original and transformed text.app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt (1)
35-42: LGTM: Extension mappers are correctly implemented.The
toEntity()extension functions properly map all DTO fields to their corresponding domain entities. Once the enum imports are moved to the domain layer (as noted in the earlier comment), these mappers will align with clean architecture.
| import com.hsLink.hslink.presentation.onboarding.model.JobType | ||
| import com.hsLink.hslink.presentation.onboarding.model.LinkType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix layering violation: Data layer should not import presentation layer types.
The data layer (DTOs) is importing JobType and LinkType from the presentation layer (com.hsLink.hslink.presentation.onboarding.model). This violates clean architecture principles where data/domain layers should not depend on the presentation layer.
Move JobType and LinkType to the domain layer (e.g., com.hsLink.hslink.domain.model.onboarding) so they can be shared across layers without creating circular dependencies or layering violations.
-import com.hsLink.hslink.presentation.onboarding.model.JobType
-import com.hsLink.hslink.presentation.onboarding.model.LinkType
+import com.hsLink.hslink.domain.model.onboarding.JobType
+import com.hsLink.hslink.domain.model.onboarding.LinkTypeCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/data/dto/request/onboarding/CareerRequest.kt
around lines 3-4, the DTO is importing presentation-layer types JobType and
LinkType which breaks layering; move those types to the domain layer (suggested
package: com.hsLink.hslink.domain.model.onboarding), update their package
declarations, replace imports in CareerRequest.kt to reference the new domain
package, and update any other files that currently import the presentation
versions to use the domain types; rebuild and run tests to ensure no remaining
references to the old presentation package.
| import com.hsLink.hslink.presentation.onboarding.model.JobType | ||
| import com.hsLink.hslink.presentation.onboarding.model.LinkType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Architecture violation: data layer depends on presentation layer.
The DTO is importing JobType and LinkType from the presentation package, which violates clean architecture principles. The data layer should not depend on the presentation layer.
Consider one of these solutions:
- Recommended: Move
JobTypeandLinkTypeenums to the domain layer where they logically belong as business entities. - Use
Stringin the DTOs and map to the domain enums in thetoEntity()functions.
Apply this approach if moving enums to domain:
-import com.hsLink.hslink.presentation.onboarding.model.JobType
-import com.hsLink.hslink.presentation.onboarding.model.LinkType
+import com.hsLink.hslink.domain.model.JobType
+import com.hsLink.hslink.domain.model.LinkTypeThen move the enum definitions from presentation/onboarding/model/OnboardingState.kt to the domain layer.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt
around lines 5-6, the DTO imports JobType and LinkType from the presentation
layer which violates clean architecture; move the JobType and LinkType enum
definitions from presentation/onboarding/model/OnboardingState.kt into the
domain layer (e.g., app/src/main/java/com/hsLink/hslink/domain/model/) and
update their package declarations, then change the imports in CareerResponse to
reference the new domain package and adjust any toEntity()/mapping code to use
the domain enums; alternatively, if you prefer not to move enums, change the DTO
fields to String and perform String→enum mapping in the mapper/toEntity
functions in the data layer.
|
|
||
| @Serializable | ||
| data class CareerResponse( | ||
| @SerialName("id") val id: Int, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify ID type inconsistency.
CareerResponse.id uses Int (Line 12) while LinkResponse.id uses Long (Line 23). This inconsistency might be intentional based on your backend API design, but please verify that these types match the actual API response types.
Run this script to check if there are API documentation or integration tests that confirm the ID types:
Also applies to: 23-23
🏁 Script executed:
#!/bin/bash
# Search for API endpoint definitions or documentation mentioning career and link IDs
rg -n -C3 -g '!*.kt' -e 'career.*id|link.*id' -iLength of output: 333
🏁 Script executed:
#!/bin/bash
# Get full content of CareerResponse and LinkResponse DTOs
echo "=== CareerResponse.kt ==="
cat app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt
echo -e "\n=== LinkResponse.kt ==="
cat app/src/main/java/com/hsLink/hslink/data/dto/response/LinkResponse.ktLength of output: 1549
🏁 Script executed:
#!/bin/bash
# Search for Retrofit API interface definitions
fd -e 'kt' -p '.*[Aa]pi.*' --type f | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for where CareerResponse and LinkResponse are deserialized or used
rg -n -C2 'CareerResponse|LinkResponse' --type=kt -g '!test'Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for CareerItemEntity and LinkItemEntity to see their ID types
rg -n 'data class CareerItemEntity|data class LinkItemEntity' --type rustLength of output: 148
🏁 Script executed:
#!/bin/bash
# Search for API service interface definitions (alternative syntax for Kotlin)
fd 'Service\|Api\|Repository' -e 'kt' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for usages of CareerResponse and LinkResponse to understand context
rg -n -C3 'CareerResponse\|LinkResponse'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for test files or fixtures showing API response structure
fd -e 'json' -o -e 'kt' | xargs rg -l 'career.*id\|link.*id' -i | head -10Length of output: 116
🏁 Script executed:
#!/bin/bash
# Simple search for entity classes without type filters
rg -n 'data class CareerItemEntity' --no-headingLength of output: 108
🏁 Script executed:
#!/bin/bash
# Search for any API or Service Kotlin files
find . -name '*Api.kt' -o -name '*Service.kt' -o -name '*Repository.kt' 2>/dev/null | head -15Length of output: 933
🏁 Script executed:
#!/bin/bash
# Look at CareerResponse file again and grep for imports/usages
grep -r 'CareerResponse\|LinkResponse' . --include='*.kt' 2>/dev/null | head -20Length of output: 2197
🏁 Script executed:
#!/bin/bash
# Check build.gradle for serialization library and backend info
find . -name 'build.gradle*' -o -name 'settings.gradle*' | xargs grep -l 'serialization\|retrofit\|okhttp' 2>/dev/nullLength of output: 41
🏁 Script executed:
#!/bin/bash
# Get CareerItemEntity and LinkItemEntity definitions
cat app/src/main/java/com/hsLink/hslink/domain/model/search/UserProfileEntity.ktLength of output: 1074
🏁 Script executed:
#!/bin/bash
# Get OnboardingService API interface to see endpoint signatures
cat app/src/main/java/com/hsLink/hslink/data/service/onboarding/OnboardingService.ktLength of output: 764
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation about ID types
grep -r 'id.*Int\|id.*Long' app/src/main/java/com/hsLink/hslink/domain/model/search/ --include='*.kt' -B2 -A2Length of output: 1923
Resolve inconsistency between CareerItemEntity and CareerEntity ID types.
While CareerResponse.id (Int) and LinkResponse.id (Long) correctly map to their respective entity types, a domain-level inconsistency exists: CareerItemEntity.id is Int while CareerEntity.id is Long. Both represent career data but have different ID types. Verify this is intentional or align them for consistency across the domain model.
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/dto/response/CareerResponse.kt
around line 12, CareerResponse.id is declared as Int while the domain has
CareerEntity.id as Long and CareerItemEntity.id as Int; verify intended ID type
and make the model consistent: either change CareerResponse.id to Long to match
CareerEntity (and update LinkResponse if needed), or change CareerEntity.id to
Int (and update persistence layer and migrations) so all career-related
DTOs/entities use the same ID type across the domain; update all mapping code,
constructors, serializers, and any database/ORM types accordingly to avoid type
mismatches.
| import com.hsLink.hslink.presentation.onboarding.model.JobType | ||
| import com.hsLink.hslink.presentation.onboarding.model.LinkType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix layering violation: Domain layer should not import presentation layer types.
The domain layer is importing JobType and LinkType from the presentation layer. Domain entities should not depend on presentation layer types to maintain clean architecture boundaries.
Move JobType and LinkType to the domain layer so they can be properly shared without violating layering principles:
-import com.hsLink.hslink.presentation.onboarding.model.JobType
-import com.hsLink.hslink.presentation.onboarding.model.LinkType
+import com.hsLink.hslink.domain.model.onboarding.JobType
+import com.hsLink.hslink.domain.model.onboarding.LinkTypeCommittable suggestion skipped: line range outside the PR's diff.
| KaKaoLoginScreen( | ||
| paddingValues = PaddingValues(), | ||
| onNavigateToHome = { | ||
| navController.navigate(AppMain) { | ||
| popUpTo(Login) { inclusive = true } | ||
| } | ||
| }, | ||
| onNavigateToOnboarding = { | ||
| navController.navigate(AppMain) { | ||
| popUpTo(Login) { inclusive = true } | ||
| } | ||
| } | ||
| paddingValues = paddingValues, | ||
| onNavigateToHome = onNavigateToOnboarding, | ||
| onNavigateToOnboarding = onNavigateToOnboarding | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix navigation callback wiring
KaKaoLoginScreen navigates to onboarding when needsOnboarding is true, otherwise it calls onNavigateToHome. Passing onNavigateToOnboarding into both callback slots means already-onboarded users are forced back into the onboarding flow on every login, effectively blocking them from reaching the main app. Wire onNavigateToHome to onNavigateToAppMain instead so the success path reaches the home graph.
- onNavigateToHome = onNavigateToOnboarding,
+ onNavigateToHome = onNavigateToAppMain,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| KaKaoLoginScreen( | |
| paddingValues = PaddingValues(), | |
| onNavigateToHome = { | |
| navController.navigate(AppMain) { | |
| popUpTo(Login) { inclusive = true } | |
| } | |
| }, | |
| onNavigateToOnboarding = { | |
| navController.navigate(AppMain) { | |
| popUpTo(Login) { inclusive = true } | |
| } | |
| } | |
| paddingValues = paddingValues, | |
| onNavigateToHome = onNavigateToOnboarding, | |
| onNavigateToOnboarding = onNavigateToOnboarding | |
| ) | |
| KaKaoLoginScreen( | |
| paddingValues = paddingValues, | |
| onNavigateToHome = onNavigateToAppMain, | |
| onNavigateToOnboarding = onNavigateToOnboarding | |
| ) |
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt around
lines 152 to 156, the KaKaoLoginScreen is wired with onNavigateToHome and
onNavigateToOnboarding both set to onNavigateToOnboarding, which causes
already-onboarded users to be sent back to onboarding; change the
onNavigateToHome argument to use the success/home callback (onNavigateToAppMain)
instead of onNavigateToOnboarding so successful logins route to the main/home
graph while keeping onNavigateToOnboarding for the onboarding path.
| val onSkipPathClick = { | ||
| onCareerSelect(false) | ||
| onNextClick() | ||
| } | ||
|
|
||
| val onExperiencedPathClick = { | ||
| onCareerSelect(true) | ||
| onAddCareerClick() | ||
| } | ||
|
|
||
| OnboardingScreen( | ||
| title = "지금까지의 커리어를 알려주세요", | ||
| progress = progress, | ||
| subtitle = "아직 경력이 없다면 넘겨도 괜찮아요", | ||
| subtitle = "아직 경력이 없다면 '다음' 버튼으로 넘겨도 괜찮아요.", | ||
| paddingValues = paddingValues, | ||
| showPreviousButton = true, | ||
| nextButtonEnabled = true, | ||
| onPreviousClick = onPreviousClick, | ||
| onNextClick = onNextClick | ||
| onNextClick = onSkipPathClick | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next button forces career selection to false.
onSkipPathClick runs for every “Next” tap, so even users who already picked the experienced path (or have careers loaded) get reset to false right before navigation. That wipes their choice/state. Gate the reset behind an actual skip condition instead of doing it unconditionally.
- val onSkipPathClick = {
- onCareerSelect(false)
- onNextClick()
- }
-
@@
- onNextClick = onSkipPathClick
+ onNextClick = {
+ if (careerList.isEmpty()) {
+ onCareerSelect(false)
+ }
+ onNextClick()
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/onboarding/component/screen/CareerScreen.kt
around lines 36 to 55, the Next button is wired to onSkipPathClick which
unconditionally calls onCareerSelect(false) and thus clears any previously
chosen experienced path; change the wiring so Next does not always reset career
state—pass the original onNextClick to OnboardingScreen (onNextClick =
onNextClick) and only call onCareerSelect(false) when the user explicitly
chooses the skip path (e.g., invoke onSkipPathClick from the actual "Skip"
control or add a conditional that checks whether the skip action was requested
before resetting). Ensure existing experienced-path flow still triggers
onAddCareerClick without being overwritten by the Next button.
| if (result.isSuccess) { | ||
|
|
||
| fetchCareersAndLinks() | ||
|
|
||
| _state.update { | ||
| it.copy( | ||
| currentStep = OnboardingStep.CAREER, | ||
| isLoading = false, | ||
| tempCompanyName = "", | ||
| tempPosition = "", | ||
| tempDepartment = "", | ||
| tempJobType = null, | ||
| tempStartYm = "", | ||
| tempEndYm = null, | ||
| tempIsEmployed = false, | ||
| apiError = null | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve refresh error messaging after career submit
If fetchCareersAndLinks() fails, it already updates _state with the error via apiError, but this block immediately overwrites it with null. The user never sees the failure even though the list wasn’t refreshed. Please retain the value coming back from the refresh (e.g., reuse it.apiError) instead of forcing it to null.
- apiError = null
+ apiError = it.apiError🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt
around lines 128 to 144, the success branch after career submit calls
fetchCareersAndLinks() which may set _state.apiError on failure, but the
subsequent _state.update unconditionally sets apiError = null and thus
suppresses any refresh error; change the update to preserve the existing state's
apiError (e.g., apiError = it.apiError) so any error produced by
fetchCareersAndLinks() remains visible to the user, and keep all other field
resets the same.
| if (result.isSuccess) { | ||
|
|
||
| fetchCareersAndLinks() | ||
|
|
||
| _state.update { | ||
| it.copy( | ||
| currentStep = OnboardingStep.LINKS_LIST, | ||
| isLoading = false, | ||
| tempLinkType = null, | ||
| tempLinkUrl = "", | ||
| apiError = null | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep link refresh errors visible
Same pattern here: after a successful submit we refresh, but if the refresh call fails the resulting apiError is wiped out by setting it to null. Users lose feedback that their link list didn’t update. Let’s keep the propagated error (e.g., apiError = it.apiError) so the UI can react accordingly.
- apiError = null
+ apiError = it.apiError🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/onboarding/viewmodel/OnboardingViewModel.kt
around lines 183 to 195, the success branch clears any propagated refresh error
by setting apiError = null; change that to preserve the current state's apiError
(e.g., apiError = it.apiError) so if fetchCareersAndLinks() failed the UI still
sees the error, leaving the rest of the copied fields unchanged.
ISSUE
❗ WORK DESCRIPTION
📸 SCREENSHOT
📢 TO REVIEWERS
Summary by CodeRabbit
Release Notes
New Features
Improvements