Feature/#60 회원가입 UI 개선, Position Config API 처리 최적화 #73
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이번 변경 내역은 애플리케이션의 기본 컨텍스트 및 내비게이션 경로 수정, 테스트 파일 삭제, 코루틴과 Room 등 새로운 종속성 추가, 데이터 저장소 및 프로토콜 관련 구성 개선을 포함합니다. 또한, ConfigRepository 인터페이스와 그 구현, DI 모듈, API 인터페이스 및 모델 클래스들의 가시성 변경, 헤더 액션바 레이아웃 조정, 그리고 회원가입 프로세스 관련 UI, 상태, 유효성 검사, 텍스트 및 인텐트 수정이 이루어졌습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SignUpScreen
participant VM as SignUpViewModel
participant UC as GetPositionConfigsUseCase
participant Repo as ConfigRepositoryImpl
UI ->> VM: onIntent(EnterScreen)
VM ->> UC: invoke()
UC ->> Repo: getPositionConfigs()
Repo -->> UC: Flow<List<String>>
UC -->> VM: position data
VM -->> UI: 상태 업데이트 (positions)
Assessment against linked issues
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@ashwon12 draft 해제했습니다! |
| positionConfigDao.replacePositionConfigs(remotePositionConfigs.toEntity()) | ||
|
|
||
| emit(positionConfigDao.getPositionConfigs().map { it.label }) | ||
| }.flowOn(ioDispatcher) |
There was a problem hiding this comment.
Dispatcher 주입하니까 너무 깔끔하네요.. 굳...
만약 DataStore에 PositionConfig를 저장한다면, DataStore을 사용하고, 저장/불러오는 과정에서 Json Converter를 사용할거같아요 Json Converter를 사용한다는 것 자체가 DataStore의 역할을 벗어난다고 생각해서 Room으로 만들었어요 ... 까지가 이때까지의 생각이었고 재원님이 남겨준 코멘트보고 좀 더 찾아보니 Proto Datastore을 사용해서 List 형태를 저장할 수 있군요 (List가 아닌 객체만 되는 줄 ...) |
|
@ashwon12 proto datastore로 교체해봤습니다! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (36)
app/src/main/java/com/yapp/app/official/YappOfficialApplication.kt(2 hunks)app/src/main/java/com/yapp/app/official/ui/Navigator.kt(2 hunks)app/src/test/java/com/yapp/official/ExampleUnitTest.kt(0 hunks)core/data-api/build.gradle.kts(1 hunks)core/data-api/src/main/java/com/yapp/dataapi/ConfigRepository.kt(1 hunks)core/data/build.gradle.kts(2 hunks)core/data/src/main/java/com/yapp/core/data/data/YappDispatcher.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/data/di/DispatchersModule.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/data/di/RepositoryModule.kt(2 hunks)core/data/src/main/java/com/yapp/core/data/data/repository/ConfigRepositoryImpl.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/data/repository/UnAuthorizedUserRepositoryImpl.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/local/di/DataStoreModule.kt(3 hunks)core/data/src/main/java/com/yapp/core/data/local/proto/PositionConfigSerializer.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/api/ConfigApi.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/api/UnAuthorizedUserApi.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/di/ApiModule.kt(2 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/request/SignUpRequest.kt(2 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/response/PositionConfigResponse.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/response/SignUpResponse.kt(1 hunks)core/data/src/main/proto/position_config.proto(1 hunks)core/designsystem/src/main/java/com/yapp/core/designsystem/component/header/YappHeaderActionbar.kt(3 hunks)core/domain/src/main/java/com/yapp/domain/GetPositionConfigsUseCase.kt(1 hunks)core/model/src/main/java/com/yapp/model/Regex.kt(1 hunks)core/model/src/main/java/com/yapp/model/SignUpInfo.kt(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpContract.kt(2 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpScreen.kt(3 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt(4 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/email/EmailContract.kt(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/email/EmailPage.kt(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/password/PasswordContract.kt(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionContract.kt(2 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionPage.kt(4 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionViewModel.kt(1 hunks)feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/component/ActivityUnitInputSection.kt(6 hunks)feature/signup/src/main/res/values/strings.xml(2 hunks)gradle/libs.versions.toml(3 hunks)
💤 Files with no reviewable changes (1)
- app/src/test/java/com/yapp/official/ExampleUnitTest.kt
🧰 Additional context used
📓 Learnings (1)
core/designsystem/src/main/java/com/yapp/core/designsystem/component/header/YappHeaderActionbar.kt (1)
Learnt from: jinukeu
PR: YAPP-admin/yappu-world-android#38
File: core/designsystem/src/main/java/com/yapp/core/designsystem/component/header/HeaderActionbar.kt:59-90
Timestamp: 2025-01-24T16:33:01.628Z
Learning: In the YAPP Android project, header components (HeaderActionbar, HeaderActionbarExpanded) are intentionally kept separate to follow the Open-Closed Principle (OCP), allowing for easy extension without modifying existing components.
🪛 Buf (1.47.2)
core/data/src/main/proto/position_config.proto
1-1: Files must have a package defined.
(PACKAGE_DEFINED)
🔇 Additional comments (39)
core/data/src/main/java/com/yapp/core/data/remote/model/response/SignUpResponse.kt (1)
8-11: 가시성 제한이 적절히 적용되었습니다!데이터 클래스를 internal로 변경하여 모듈 내부로 접근을 제한한 것이 적절합니다. 이는 토큰 정보의 보안성을 향상시키고 모듈의 캡슐화를 강화합니다.
core/data/src/main/java/com/yapp/core/data/remote/api/UnAuthorizedUserApi.kt (1)
Line range hint
9-14: API 인터페이스의 가시성이 적절히 제한되었습니다!UnAuthorizedUserApi 인터페이스를 internal로 변경한 것이 적절합니다. 이는 SignUpRequest와 SignUpResponse의 가시성 변경과 일관성을 유지하며, API 계층의 캡슐화를 강화합니다.
core/data/src/main/java/com/yapp/core/data/remote/model/request/SignUpRequest.kt (2)
Line range hint
8-14: 데이터 클래스들의 가시성이 적절히 제한되었습니다!SignUpRequest와 ActivityUnit 데이터 클래스들을 internal로 변경하여 모듈의 캡슐화가 강화되었습니다.
Also applies to: 17-20
22-28: 입력값 검증이 적절히 구현되었습니다!이메일, 비밀번호, 이름에 대한 trim() 처리가 잘 되어있습니다. 또한 PositionConfigs를 통한 포지션 검증 로직이 잘 구현되어 있습니다.
core/data/src/main/java/com/yapp/core/data/remote/api/ConfigApi.kt (1)
6-9: API 인터페이스가 잘 구현되었습니다.버전 관리가 된 엔드포인트와 internal 스코프 사용이 적절합니다. suspend 함수를 사용한 코루틴 지원도 잘 되어있습니다.
core/domain/src/main/java/com/yapp/domain/GetPositionConfigsUseCase.kt (1)
1-10: 깔끔한 UseCase 구현입니다!Clean Architecture 원칙을 잘 따르고 있으며, operator function을 사용하여 가독성 높은 코드를 작성하셨습니다.
app/src/main/java/com/yapp/app/official/YappOfficialApplication.kt (1)
20-26: 접근성 고려가 필요합니다.폰트 크기를 1.0f로 고정하는 것은 시각적 일관성을 제공하지만, 다음과 같은 접근성 문제가 있습니다:
- 시각 장애가 있는 사용자가 시스템 폰트 크기 설정을 사용할 수 없게 됩니다.
- 안드로이드 접근성 가이드라인에 부합하지 않습니다.
대신 다음과 같은 대안을 고려해보세요:
- 특정 컴포넌트만 고정 크기를 사용
- 최소/최대 폰트 크기 제한 설정
- 사용자가 앱 내에서 폰트 크기를 조절할 수 있는 설정 제공
core/model/src/main/java/com/yapp/model/SignUpInfo.kt (1)
14-17: 코드가 잘 구현되었습니다!중앙 집중화된 Regex 패턴을 사용하여 이메일과 비밀번호 유효성 검사를 구현한 것이 좋습니다. 이는 다음과 같은 이점이 있습니다:
- 패턴 재사용성 향상
- 유지보수 용이성
- 일관된 검증 로직 보장
core/data/src/main/java/com/yapp/core/data/local/proto/PositionConfigSerializer.kt (1)
11-24: Proto DataStore 직렬화 구현이 잘 되었습니다!다음과 같은 우수한 구현 사항이 확인됩니다:
- 적절한 예외 처리
- Proto 파싱 오류에 대한 명확한 에러 메시지
- 의존성 주입을 위한 @Inject 생성자 사용
core/data/src/main/java/com/yapp/core/data/data/di/RepositoryModule.kt (1)
27-31: 의존성 주입 구현이 잘 되어있습니다!Dagger Hilt의 모듈 패턴을 잘 따르고 있으며, 다른 Repository 바인딩과 일관성 있게 구현되어 있습니다.
feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionContract.kt (1)
7-7: 상태 관리가 잘 구현되어 있습니다!
positions리스트의 기본값을emptyList()로 설정한 것이 좋은 구현 방식입니다. 또한EnterScreen에 positions 파라미터를 추가하여 화면 진입 시 필요한 데이터를 모두 전달할 수 있도록 했습니다.Also applies to: 17-17
feature/signup/src/main/java/com/yapp/feature/signup/signup/page/password/PasswordContract.kt (1)
6-6: 정규식 관리가 개선되었습니다!정규식 패턴을
Regex객체로 중앙 집중화하여 관리하는 방식으로 개선되었습니다. 이는 코드의 유지보수성과 재사용성을 높여줍니다.Also applies to: 13-13
core/data/src/main/java/com/yapp/core/data/data/repository/UnAuthorizedUserRepositoryImpl.kt (1)
19-19: DataStore 의존성 주입이 잘 구현되어 있습니다!
DataStore<PositionConfigs>를 생성자 주입으로 받아 사용하는 것이 좋은 구현 방식입니다.app/src/main/java/com/yapp/app/official/ui/Navigator.kt (1)
35-35: 시작 화면 변경에 대한 검증이 필요합니다.HomeRoute에서 SignUpRoute로의 시작 화면 변경이 기존 사용자의 앱 경험에 영향을 미칠 수 있습니다. 사용자의 로그인 상태에 따라 적절한 화면으로 리다이렉션하는 로직이 구현되어 있는지 확인해주세요.
core/data/src/main/java/com/yapp/core/data/data/repository/ConfigRepositoryImpl.kt (1)
17-21: 구현이 잘 되었습니다!의존성 주입과 코루틴 디스패처 사용이 깔끔하게 구현되었습니다.
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpContract.kt (2)
12-13: 상태 관리가 잘 구현되었습니다.에러 상태와 포지션 목록을 상태에 추가한 것이 적절합니다.
19-22: 백 아이콘 로직이 개선되었습니다.Complete 상태에서만 백 아이콘을 숨기도록 변경된 것이 UX 관점에서 더 명확합니다.
feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionViewModel.kt (1)
27-27: 상태 업데이트 로직이 적절합니다.
positions데이터를 상태에 포함시키는 변경이 잘 이루어졌습니다.core/designsystem/src/main/java/com/yapp/core/designsystem/component/header/YappHeaderActionbar.kt (1)
39-44: 터치 영역과 시각적 피드백이 개선되었습니다!패딩 값 조정과 아이콘 터치 영역 확대가 잘 이루어졌습니다. 리플 효과의 크기도 적절하게 조정되었습니다.
Also applies to: 84-90
feature/signup/src/main/java/com/yapp/feature/signup/signup/page/position/PositionPage.kt (3)
39-39: LGTM!positions 파라미터를 추가하여 동적으로 포지션 목록을 관리할 수 있도록 개선되었습니다.
52-52: LGTM!EnterScreen 인텐트에 positions 파라미터를 전달하여 초기 상태를 적절히 설정합니다.
104-104: LGTM!ActivityUnitInputSection과 PreviousActivityUnitInputSection 컴포넌트에 동일한 positions 목록을 전달하여 일관성을 유지합니다.
Also applies to: 117-117
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpViewModel.kt (3)
7-7: LGTM!필요한 의존성과 import문이 적절히 추가되었습니다.
Also applies to: 14-16, 19-19, 24-24
109-109: LGTM!이메일 유효성 검사 로직이
isEmailValid프로퍼티를 사용하도록 개선되었습니다.Also applies to: 187-187
41-50: 🧹 Nitpick (assertive)TODO 주석의 에러 처리 구현이 필요합니다.
position configs 가져오기 실패 시의 에러 처리가 구현되어 있지 않습니다.
에러 처리 로직 구현을 도와드릴까요?
feature/signup/src/main/java/com/yapp/feature/signup/signup/SignUpScreen.kt (2)
76-78: LGTM!화면 진입 시 EnterScreen 인텐트를 트리거하여 position configs를 적절히 가져옵니다.
125-125: LGTM!PositionPage에 positions 데이터를 전달하여 동적 포지션 목록을 구현합니다.
core/data-api/build.gradle.kts (1)
7-7: LGTM!코루틴 의존성이 적절히 추가되었습니다.
core/data/src/main/proto/position_config.proto (3)
1-4: 프로토콜 파일 헤더 및 옵션 검토
syntax = "proto3";와option java_package및option java_multiple_files가 올바르게 선언되어 있습니다. 파일의 기본 설정이 적절합니다.🧰 Tools
🪛 Buf (1.47.2)
1-1: Files must have a package defined.
(PACKAGE_DEFINED)
6-9: PositionConfig 메시지 정의 검토
PositionConfig메시지에name과label필드가 적절하게 정의되어 있으며, 본 메시지의 구조가 명확하고 확장 가능하게 작성되었습니다.
10-13: PositionConfigs 메시지 정의 검토
PositionConfigs메시지에서 반복 필드configs를 사용해 여러 개의PositionConfig를 저장할 수 있도록 구현한 점이 좋습니다.core/data/build.gradle.kts (3)
2-3: 새로운 Gradle import 추가 검토
org.gradle.internal.extensions.stdlib.capitalized와KotlinCompile를 import하여 이후 작업에서 사용하고 있는 점이 좋습니다.
25-28: protobuf 빌드 설정 개선 검토
generateProtoTasks내에register("kotlin")이 추가되어 Kotlin 빌트인에 대해 'lite' 옵션을 적용한 점이 적절합니다.
34-39: androidComponents 블록 구현 검토
새로운androidComponents블록을 사용해 각 variant에 대해 KSP 관련KotlinCompile태스크의 소스 경로를 설정한 방식이 StackOverflow의 솔루션을 참고한 것으로 보이며, 해당 설정이 기대한 대로 동작할 것으로 판단됩니다.feature/signup/src/main/res/values/strings.xml (2)
18-18: 이메일 유효성 검증 메시지 추가 검토
signup_screen_email_input_text_description에 "올바르지 않은 이메일 형식이에요. 다시 입력해주세요."를 추가하여 사용자에게 명확한 피드백을 제공하는 점이 좋습니다.
31-33: 회원가입 Step4 텍스트 리소스 업데이트 검토
회원가입 단계 4에서 역할 관련 메시지가 업데이트되었습니다.
- 제목:
%s님은 어떤 역할이신가요?- 설명: "현재 활동 중이라면 진행 중인 기수를,\n활동하지 않고 있다면 이전 기수를 입력해 주세요."
- 입력 텍스트 라벨: "활동기수"
UI 메시지가 보다 명확해진 점이 긍정적입니다.gradle/libs.versions.toml (3)
6-6: 코루틴 버전 지정 추가 검토
새롭게kotlinxCoroutines = "1.8.1"버전이 추가되어 코루틴 관련 종속성을 최신 버전으로 관리하는 점이 좋습니다.
44-45: 코루틴 라이브러리 추가 검토
kotlinx-coroutines-android와kotlinx-coroutines-core가 추가되어, 안드로이드 및 코어 코루틴 기능을 별도로 관리하도록 설정한 점이 적절합니다.
89-91: Room 라이브러리 추가 검토
room-compiler,room-ktx,room-runtime이 최신 버전 "2.6.1"로 추가되어, Room 데이터베이스 지원을 강화한 점이 긍정적입니다.
core/data/src/main/java/com/yapp/core/data/remote/model/request/SignUpRequest.kt
Show resolved
Hide resolved
| import javax.inject.Qualifier | ||
|
|
||
| @Qualifier | ||
| annotation class Dispatcher(val susuDispatcher: YappDispatchers) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
네이밍 일관성 개선이 필요합니다.
susuDispatcher 매개변수 이름이 패키지 네이밍(yapp)과 일치하지 않습니다.
다음과 같이 수정을 제안합니다:
-annotation class Dispatcher(val susuDispatcher: YappDispatchers)
+annotation class Dispatcher(val yappDispatcher: YappDispatchers)📝 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.
| annotation class Dispatcher(val susuDispatcher: YappDispatchers) | |
| annotation class Dispatcher(val yappDispatcher: YappDispatchers) |
| val email = Regex("^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}\$") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
이메일 정규식 패턴 개선이 필요합니다.
현재 이메일 정규식이 일부 유효한 이메일 주소를 거부할 수 있습니다. RFC 5322 표준을 더 잘 준수하는 패턴으로 개선이 필요합니다.
다음과 같은 개선된 패턴을 제안합니다:
- val email = Regex("^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}\$")
+ val email = Regex("^[a-zA-Z0-9.!#\$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\$")📝 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.
| val password = Regex("^(?=.*[A-Za-z])(?=.*\\d)(?=.*[@\$!%*?&])[A-Za-z\\d@\$!%*?&]{8,20}\$") | |
| val email = Regex("^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}\$") | |
| val password = Regex("^(?=.*[A-Za-z])(?=.*\\d)(?=.*[@\$!%*?&])[A-Za-z\\d@\$!%*?&]{8,20}\$") | |
| val email = Regex("^[a-zA-Z0-9.!#\$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\$") |
core/data/src/main/java/com/yapp/core/data/data/repository/UnAuthorizedUserRepositoryImpl.kt
Show resolved
Hide resolved
core/data/src/main/java/com/yapp/core/data/data/repository/ConfigRepositoryImpl.kt
Show resolved
Hide resolved
feature/signup/src/main/java/com/yapp/feature/signup/signup/page/email/EmailPage.kt
Show resolved
Hide resolved
...main/java/com/yapp/feature/signup/signup/page/position/component/ActivityUnitInputSection.kt
Show resolved
Hide resolved
💡 Issue
🌱 Key changes
피그마 변경 내용 및 자잘한 수정 사항입니다.
회원가입 - 이메일 형식 에러시 화면 추가
이메일 형식이 다를 경우 텍스트 인풋 스타일 적용된 화면 추가 했습니다.
회원가입 - step4 라이팅 변경
변경 내용
① 타이틀 텍스트 변경
② 도움말 라이팅 변경
③ 기수 인풋 컴포넌트 타이틀 : 기수 → 활동기수 로 변경
Header 영역 뒤로가기 터치영역 수정
변경 내용
① 기존 12x24px 사이즈를 24x24px로 수정했습니다.
② 아이콘 크기 변경에 따라 헤더 부분 좌우 패딩값 20px → 14, 20px로 변경되었습니다.
회원가입 보류 화면
Position Config Api
서버에서 응답값으로 name, label을 내려줍니다.
name의 경우 서버에 보내야하는 값이고 label은 ui에 노출되어야하는 값입니다.
domain, feature 레이어에서는 name을 굳이 알 필요 없게끔 코드를 설계했습니다.
앱 내의 폰트 크기가 시스템 폰트 크기에 영향을 받지 않게 설정
✅ To Reviewers
📸 스크린샷
Summary by CodeRabbit
신규 기능
개선 사항
UI 업데이트