[PC-1589] MatchingMain, Profile 관련 QA 이슈 수정 #195
Conversation
* [PC-1593] 프로필 birthDate 관련 이슈 수정 * [PC-1593] PieceRoundingOutlinedButton shape 수정
📝 WalkthroughWalkthroughAdds product identifiers to billing models and API, adjusts matching navigation and card analytics, introduces birthdate-to-year utilities and tests, tweaks store analytics to use product IDs, and makes small UI and network mapping adjustments across billing, matching, store, profile, common, network, and designsystem layers. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 1
🧹 Nitpick comments (2)
feature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.kt (1)
32-32: Verify birthdate substring extraction and consider code formatting.This line has the same substring extraction concern as MainProfileViewModel:
substring(2,4)must align with the birthdate format from the API (should beYYYYMMDDto extract the 2-digit year correctly).Additionally, there's a minor formatting inconsistency:
substring(2,4)lacks a space after the comma, while other parts of the codebase usesubstring(2, 4).Please verify this extraction logic as part of the overall birthdate format verification (see MainProfileViewModel review comment).
Optional: Fix spacing for consistency
- }.onSuccess { setState { copy(myProfileBasic = it.copy(birthdate = it.birthdate.substring(2,4))) } + }.onSuccess { setState { copy(myProfileBasic = it.copy(birthdate = it.birthdate.substring(2, 4))) }feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt (1)
64-67: Consider guarding against invalidmatchIdbefore callingcheckMatchingPiece.If
matchIddefaults to-1(when not present in SavedStateHandle), callingcheckMatchingPiece(-1)will make an API call that's guaranteed to fail. While the error is handled, it's wasteful and may pollute error logs.🔎 Proposed fix
init { initMatchDetailInfo() - checkMatchingPiece(matchId) + if (matchId != -1) { + checkMatchingPiece(matchId) + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.ktcore/billing/src/main/java/com/puzzle/billing/model/PieceProduct.ktcore/designsystem/src/main/java/com/puzzle/designsystem/component/Button.ktcore/network/src/main/java/com/puzzle/network/model/profile/GetMyProfileBasicResponse.ktfeature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.ktfeature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/ProfileImageDialog.ktfeature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.ktfeature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingViewModel.ktfeature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.ktfeature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.ktfeature/profile/src/main/java/com/puzzle/profile/graph/main/MainProfileViewModel.ktfeature/profile/src/main/java/com/puzzle/profile/graph/register/RegisterProfileViewModel.ktfeature/store/src/main/java/com/puzzle/store/StoreViewModel.ktfeature/store/src/main/java/com/puzzle/store/ui/list/NormalProductList.ktfeature/store/src/main/java/com/puzzle/store/ui/list/PromotionProductList.kt
🧰 Additional context used
🧬 Code graph analysis (3)
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingViewModel.kt (1)
core/common-ui/src/main/java/com/puzzle/common/base/BaseViewModel.kt (1)
setState(39-40)
feature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.kt (1)
core/common-ui/src/main/java/com/puzzle/common/base/BaseViewModel.kt (1)
setState(39-40)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt (8)
core/data/src/main/java/com/puzzle/data/repository/MatchingRepositoryImpl.kt (1)
checkMatchingPiece(78-79)core/data/src/test/java/com/puzzle/data/source/matching/FakeMatchingDataSource.kt (1)
checkMatchingPiece(86-86)core/domain/src/main/java/com/puzzle/domain/repository/MatchingRepository.kt (1)
checkMatchingPiece(19-19)core/network/src/main/java/com/puzzle/network/source/matching/MatchingDataSourceImpl.kt (1)
checkMatchingPiece(61-61)core/network/src/main/java/com/puzzle/network/source/matching/MatchingDataSource.kt (1)
checkMatchingPiece(26-26)core/network/src/main/java/com/puzzle/network/api/PieceApi.kt (1)
checkMatchingPiece(168-169)core/testing/src/main/java/com/puzzle/testing/domain/repository/SpyMatchingRepository.kt (1)
checkMatchingPiece(77-77)core/common/src/main/java/com/puzzle/common/ResultUtil.kt (1)
suspendRunCatching(5-13)
⏰ 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
🔇 Additional comments (15)
core/designsystem/src/main/java/com/puzzle/designsystem/component/Button.kt (1)
267-267: LGTM! Button shape now consistent with PieceRoundingSolidButton.The corner radius change from 8.dp to 46.dp creates a pill-shaped button that matches the solid variant, improving visual consistency across the design system.
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt (1)
265-265: LGTM! Product ID correctly populated from server UUID.The mapping of
serverProduct.uuidto theidfield is consistent for bothNormalProductandPromotionProduct, properly implementing the interface requirement introduced in PieceProduct.kt.Also applies to: 279-279
feature/store/src/main/java/com/puzzle/store/ui/list/PromotionProductList.kt (1)
42-42: Analytics tracking improved with product-specific IDs.Using
product.idinstead of a hardcoded string provides granular, product-specific analytics tracking. Note that this changes the event schema for existing analytics dashboards or queries that reference the oldbuttonNamevalues.If your analytics platform has dashboards, alerts, or queries that reference the previous
buttonNameformat for promotion products, please ensure they're updated to work with the new product ID format.feature/store/src/main/java/com/puzzle/store/ui/list/NormalProductList.kt (1)
43-43: Analytics tracking simplified and consistent with promotion products.Replacing the composite string
"store_main_banner_${product.name}_${product.price}"withproduct.idsimplifies analytics tracking and ensures consistency with the promotion product tracking approach. This also eliminates potential issues with name localization or price formatting affecting analytics keys.As with the promotion products, verify that any existing analytics dashboards, queries, or alerts referencing the previous composite
buttonNameformat are updated to use the new product ID-based format.core/billing/src/main/java/com/puzzle/billing/model/PieceProduct.kt (1)
6-6: Product IDs are standard UUIDs; no PII concerns.The
idproperty inPieceProductis sourced directly from server-provided UUIDs and logged in standard analytics events (trackClickEventandtrackRevenueEvent). UUIDs are pseudonymous identifiers by design and do not contain or expose personally identifiable information. Logging product IDs in analytics is standard industry practice for tracking user engagement and revenue attribution.feature/store/src/main/java/com/puzzle/store/StoreViewModel.kt (1)
117-126: LGTM!The formatting change consolidates the
promotionProductListassignment to a single line, which is consistent with thenormalProductListdeclaration on line 119. No functional changes.feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/ProfileImageDialog.kt (1)
28-31: LGTM! Analytics event naming is now more consistent.The buttonName "accept_matching" better aligns with the function name
onApproveClickand the button label, improving analytics clarity.core/network/src/main/java/com/puzzle/network/model/profile/GetMyProfileBasicResponse.kt (1)
30-30: Better separation of concerns, but requires careful ViewModel coordination.Changing from
birthdate?.takeLast(2)tobirthdatemoves the extraction logic from the network layer to ViewModels, which provides more flexibility. However, this distributed responsibility creates consistency challenges, as multiple ViewModels must now correctly extract the birthdate substring (and they currently do so inconsistently—some withtoCompactDateString(), some without).Ensure all consuming ViewModels handle the full birthdate string correctly and consistently.
Based on the earlier verification scripts, please confirm that all ViewModels extracting birthdate information are using the correct format and indices.
feature/profile/src/main/java/com/puzzle/profile/graph/main/MainProfileViewModel.kt (1)
33-33: No action required. The birthdate format from the API is confirmed to beYYYYMMDDformat (validated inTimeUtil.ktwithDateTimeFormatter.ofPattern("yyyyMMdd")), and thesubstring(2, 4)operation correctly extracts the 2-digit year (e.g., "20000101" → "00", "19991225" → "99"). This implementation is consistent across the codebase.feature/profile/src/main/java/com/puzzle/profile/graph/register/RegisterProfileViewModel.kt (1)
132-132: RegisterProfileViewModel's birthdate transformation is correct; the inconsistency exists elsewhere.The original concern is reversed.
RegisterProfileViewModelline 132 correctly appliestoCompactDateString()beforesubstring(2, 4):
- Input format: "YYYY-MM-DD"
- After
toCompactDateString(): "YYYYMMDD"- After
substring(2, 4): "YY" ✓However,
MainProfileViewModel(line 33) andProfilePreviewViewModel(line 32) usesubstring(2, 4)directly on the "YYYY-MM-DD" string, which produces "YY-" (including the dash character). This is the actual bug—not RegisterProfileViewModel's approach, which is the correct implementation pattern.Likely an incorrect or invalid review comment.
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt (1)
139-144: Correct swap of analytics screen names.The mapping now correctly aligns the status with its semantic meaning:
PENDING(under review) tracks as "match_main_reviewing", andWAITING(no match yet) tracks as "match_main_no_match". This fix ensures accurate analytics for user state tracking.feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt (1)
255-259: LGTM!The function correctly delegates to the repository and handles errors through the existing error handler. Ignoring the success case is appropriate since this is a side-effect operation that marks the piece as checked.
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingViewModel.kt (2)
129-131: Good fix for clearing loading state in edge cases.This ensures the loading indicator is dismissed for all profile statuses, preventing the infinite loading spinner that was occurring when the profile was under review (PENDING status).
256-269: Clean refactor of navigation flow.Moving
checkMatchingPiecetoMatchingDetailViewModel.init()is a good architectural choice. The user now navigates immediately to the detail screen, while the piece check happens asynchronously during ViewModel initialization. This improves perceived responsiveness.feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt (1)
360-379: Good UX improvement - entire card is now tappable.Moving the
clickablemodifier to theBoxbackground ensures the entire card surface responds to touch, rather than just specific content areas. This aligns with the PR objective to expand the matching card touch area.
| onClick = onCardClick@{ | ||
| onCardClick() | ||
|
|
||
| if (!matchInfo.isExpanded) return@onCardClick | ||
|
|
||
| val buttonName = when { | ||
| matchInfo.matchStatus == MATCHED -> "match_matched" | ||
| matchInfo.matchType == MatchType.AUTO -> "match_auto" | ||
| matchInfo.matchType == MatchType.BASIC -> "match_basic" | ||
| else -> return@onCardClick | ||
| } | ||
|
|
||
| analyticsHelper.trackClickEvent( | ||
| screenName = "match_main_home", | ||
| buttonName = buttonName, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find MatchType enum definition
find . -name "*.kt" -o -name "*.java" | xargs rg "enum class MatchType|sealed class MatchType" -A 10Repository: Piece-Puzzly/Piece-Android
Length of output: 1020
🏁 Script executed:
#!/bin/bash
# Search for MatchType values to see what types exist
rg "MatchType\." --no-heading -o | sort | uniq | head -20Repository: Piece-Puzzly/Piece-Android
Length of output: 1490
🏁 Script executed:
#!/bin/bash
# Find all analytics tracking calls with matchType context
rg "trackClickEvent|analyticsHelper" -B 5 -A 5 | grep -E "(TRIAL|PREMIUM|matchType|trackClickEvent)" | head -30Repository: Piece-Puzzly/Piece-Android
Length of output: 3423
🏁 Script executed:
#!/bin/bash
# Search specifically for TRIAL and PREMIUM in analytics or tracking contexts
rg "TRIAL|PREMIUM" -B 3 -A 3 | grep -E "(TRIAL|PREMIUM|track|analytics)" | head -40Repository: Piece-Puzzly/Piece-Android
Length of output: 1054
🏁 Script executed:
#!/bin/bash
# Get full context of MatchingCard.kt onClick handler to see all match types
cat -n feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt | sed -n '100,150p'Repository: Piece-Puzzly/Piece-Android
Length of output: 2280
🏁 Script executed:
#!/bin/bash
# Check if AUTO type is also defined in the enum
rg "enum class MatchType" -A 20Repository: Piece-Puzzly/Piece-Android
Length of output: 1930
Add analytics tracking for TRIAL and PREMIUM match types.
The analytics tracking currently covers AUTO and BASIC match types, but TRIAL and PREMIUM match types exit early without tracking. Since TRIAL (free user-created) and PREMIUM (paid user-created) are legitimate match types that can appear in the expanded card view, they should have corresponding tracking alongside AUTO (opponent-created matches). Consider adding "match_trial" and "match_premium" event names to maintain consistent analytics coverage across all match creation sources.
| } | ||
| } | ||
|
|
||
| private fun checkMatchingPiece(matchId: Int) = viewModelScope.launch { |
There was a problem hiding this comment.
checkMatchingPiece, initMatchDetailInfo 이거 두 개를 병렬로 수행 전에 isLoading = true 하고 둘 다 끝나면 isLoading = false 를 하면 좋을 거 같아요
| matchStatus = matchInfo.matchStatus, | ||
| matchType = matchInfo.matchType | ||
| matchType = matchInfo.matchType, | ||
| onClick = onCardClick@{ |
There was a problem hiding this comment.
onCardClick @{ ~~~ } 이걸 하는 이유가 있나요?
if (!matchInfo.isExpanded) return@onCardClick << 이거 대신에 isExpanded 일 때 수행하게 하면 되지 않을까요?
There was a problem hiding this comment.
onCardClick @{ ~~~ } 이걸 하는 이유가 있나요? ->> 버튼이 Matched, Auto, Basic이 아닌경우는 아날리틱스 이벤트를 처리하게 하지 않기위해 람다를 닫아주려고 사용했어요.
if (!matchInfo.isExpanded) return@onCardClick << 이거 대신에 isExpanded 일 때 수행하게 하면 되지 않을까요? ->>이거는 사실 조건문 안의 조건문이 들어가는게 마음에 안들어서 빠른 return 사용하긴했는데 그냥 조건처리해서 사용해도 될 것 같네요. ㅎㅎ
| suspendRunCatching { | ||
| profileRepository.loadMyProfileBasic() | ||
| }.onSuccess { setState { copy(myProfileBasic = it) } | ||
| }.onSuccess { setState { copy(myProfileBasic = it.copy(birthdate = it.birthdate.substring(2,4))) } |
There was a problem hiding this comment.
서버에서 제대로 내려주겠지만 혹시나 it.birthdate 문자열이 짧아서 substring(2,4) 을 못하면 앱이 죽을 거 같은데... 예외처리를 해주면 좋지 않을까요?
추가
다른 subString 사용 부분도 포함입니다.
There was a problem hiding this comment.
음.. 확장함수로 birthDate 검사하는 함수 만들면 좋을것 같네요. 수정하겠습니다
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt (1)
27-28: Remove unused imports.After changing the return type to
List<PieceProduct>, the imports forImmutableListandtoImmutableListare no longer used and should be removed.🔎 Proposed fix
import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.ktcore/billing/src/main/java/com/puzzle/billing/domain/BillingHelper.kt
⏰ 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
🔇 Additional comments (3)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt (2)
116-116: LGTM!Good catch adding the space after
ifto align with Kotlin style conventions.
270-281: UUID field exists on all Product variants—code looks good.The
uuidfield is properly defined on theProductsealed interface and correctly overridden in bothProduct.BasicCashProductandProduct.PromotionProduct. The assignments ofid = serverProduct.uuidin bothNormalProductandPromotionProductconstructors are correct and will compile without issues.core/billing/src/main/java/com/puzzle/billing/domain/BillingHelper.kt (1)
12-12: Return type change toList<PieceProduct>is already handled by existing call sites.The sole call site in
StoreViewModel(line 116) already handles theListreturn type and doesn't attempt mutation—it filters the list and converts back toImmutableList. The implementations (BillingHelperImpl,FakeBillingHelper) are already aligned with this change, with the test fake'sImmutableListreturn type being a valid covariant override.Likely an incorrect or invalid review comment.
| val sortedProductList = products.sortedBy { product -> // 가격순 내림정렬 | ||
| product.oneTimePurchaseOfferDetailsList | ||
| ?.minOfOrNull { it.priceAmountMicros } | ||
| ?: Long.MAX_VALUE | ||
| } | ||
|
|
||
| // 3. 서버 상품과 Google Play 상품 매칭 후 변환 | ||
| return products.mapNotNull { product -> | ||
| return sortedProductList.mapNotNull { product -> | ||
| val serverProduct = cashProducts.products.find { it.uuid == product.productId } | ||
| serverProduct?.let { | ||
| convertToPieceProduct( | ||
| productDetail = product, | ||
| serverProduct = it | ||
| ) | ||
| } | ||
| }.toImmutableList() | ||
| } |
There was a problem hiding this comment.
Fix sorting order mismatch: comment says descending but code does ascending.
The comment states "가격순 내림정렬" (descending order by price), but sortedBy sorts in ascending order (lowest price first). To sort in descending order, use sortedByDescending instead.
🔎 Proposed fix
- val sortedProductList = products.sortedBy { product -> // 가격순 내림정렬
+ val sortedProductList = products.sortedByDescending { product -> // 가격순 내림정렬
product.oneTimePurchaseOfferDetailsList
?.minOfOrNull { it.priceAmountMicros }
?: Long.MAX_VALUE
}Alternatively, if ascending order is actually intended, update the comment:
- val sortedProductList = products.sortedBy { product -> // 가격순 내림정렬
+ val sortedProductList = products.sortedBy { product -> // 가격순 오름정렬
product.oneTimePurchaseOfferDetailsList
?.minOfOrNull { it.priceAmountMicros }
?: Long.MAX_VALUE
}🤖 Prompt for AI Agents
In core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt
around lines 164 to 179, the comment indicates a descending price sort ("가격순
내림정렬") but the code uses sortedBy (ascending); change the sort to
sortedByDescending on the same key
(product.oneTimePurchaseOfferDetailsList?.minOfOrNull { it.priceAmountMicros }
?: Long.MAX_VALUE) so highest price comes first, or if ascending was intended,
update the comment to reflect ascending order instead.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @core/common/src/main/java/com/puzzle/common/TimeUtil.kt:
- Around line 46-50: The toBirthYear() function silently returns incorrect
substrings for missing or malformed input; update it to explicitly handle the
"UNKNOWN" sentinel and validate format before slicing: if the receiver equals
"UNKNOWN" return a clear sentinel (e.g., "00" or another agreed token), else
call isValidBirthDateFormat() (same validator used by toBirthDate()) and only
then extract the last two digits of the 4-digit year; if validation fails return
the sentinel instead of slicing arbitrary characters. Also update the KDoc for
toBirthYear() to declare the accepted input formats (yyyy-mm-dd or yyyymmdd) and
the fallback behavior so callers know what to expect.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt (1)
147-173: Update FakeBillingHelper to returnListinstead ofImmutableList.FakeBillingHelper returns
ImmutableList<PieceProduct>but the BillingHelper interface specifiesList<PieceProduct>. This type mismatch violates the interface contract and can cause test/production divergence. Update line 22 to returnList<PieceProduct>:FakeBillingHelper type mismatch
core/billing/src/main/java/com/puzzle/billing/testing/FakeBillingHelper.kt:22 - override suspend fun processAvailableProducts(cashProducts: CashProduct): ImmutableList<PieceProduct> = + override suspend fun processAvailableProducts(cashProducts: CashProduct): List<PieceProduct> =The implementation change from
ImmutableListtoListis correct; however, the test fake was not updated to match.
🧹 Nitpick comments (1)
core/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.kt (1)
193-203: Expand test coverage to include edge cases.The test validates the happy path but should also cover edge cases to prevent regressions:
- Short strings:
"98","1",""→ should they return"00"or throw an exception?- Invalid formats:
"abcd","not-a-date"→ what's the expected behavior?- Non-numeric years:
"abcd-03-21"→ should this be handled?🔎 Proposed additional test cases
@ParameterizedTest @ValueSource( strings = [ "", // 빈 문자열 "9", // 1자리 "98", // 2자리 (이미 2자리 연도) "199", // 3자리 ] ) fun `4자리 미만 문자열은 00을 반환한다`(input: String) { assertEquals("00", input.toBirthYear()) } @Test fun `정상적인 yyyy-MM-dd 형식에서 연도 추출`() { assertEquals("98", "1998-03-21".toBirthYear()) assertEquals("03", "2003-12-31".toBirthYear()) assertEquals("99", "1999-01-01".toBirthYear()) } @Test fun `yyyyMMdd 형식에서도 연도 추출 가능`() { assertEquals("98", "19980321".toBirthYear()) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.ktcore/common/src/main/java/com/puzzle/common/TimeUtil.ktcore/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.ktfeature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.ktfeature/profile/src/main/java/com/puzzle/profile/graph/main/MainProfileViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/profile/src/main/java/com/puzzle/profile/graph/main/MainProfileViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.kt (1)
core/common-ui/src/main/java/com/puzzle/common/base/BaseViewModel.kt (1)
setState(39-40)
⏰ 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
🔇 Additional comments (5)
core/billing/src/main/java/com/puzzle/billing/data/BillingHelperImpl.kt (2)
116-116: Minor formatting improvement applied.The spacing adjustment improves code readability without affecting functionality.
256-286: LGTM! Product ID assignment implemented correctly.The
idfield is now properly populated fromserverProduct.uuidfor bothNormalProductandPromotionProduct, enabling consistent product identification across the billing system.core/common/src/main/java/com/puzzle/common/TimeUtil.kt (2)
35-44: LGTM!The validation function correctly uses
DateTimeFormatterto verify the yyyyMMdd format and handles parse exceptions appropriately.
27-27: Good refactoring to centralize validation.Using
isValidBirthDateFormat()improves maintainability by consolidating the validation logic.feature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.kt (1)
5-5: Ensure birthdate format compatibility withtoBirthYear().The
toBirthYear()function safely handles edge cases by checking string length and returning"00"for strings shorter than 4 characters. However, verify server consistency: when birthdate isnull, the response mapper defaults to"UNKNOWN"(7 characters), whichtoBirthYear()processes to"UN"rather than"00".If the server guarantees birthdate is present and in
yyyy-mm-ddformat, document this assumption. Otherwise, consider whether"UN"is the intended fallback for missing birthdates, or if"00"would be more appropriate.
| /** yyyy-mm-dd -> yy 년생 ex) 1998 -> 98 */ | ||
| fun String.toBirthYear(): String = | ||
| takeIf { length >= 4 } | ||
| ?.take(4) | ||
| ?.takeLast(2) ?: "00" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the TimeUtil.kt file
find . -name "TimeUtil.kt" -type fRepository: Piece-Puzzly/Piece-Android
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Read the TimeUtil.kt file to see the context
cat -n "core/common/src/main/java/com/puzzle/common/TimeUtil.kt"Repository: Piece-Puzzly/Piece-Android
Length of output: 5079
🏁 Script executed:
#!/bin/bash
# Search for toBirthDate implementation for comparison
rg -n "fun.*toBirthDate" --type=kotlin -A 5Repository: Piece-Puzzly/Piece-Android
Length of output: 647
🏁 Script executed:
#!/bin/bash
# Search for all usages of toBirthYear in the codebase
rg -n "toBirthYear" --type=kotlin -B 2 -A 2Repository: Piece-Puzzly/Piece-Android
Length of output: 3627
🏁 Script executed:
#!/bin/bash
# Look for birthdate/birth_date field in response models or DTOs
rg -n "birthdate|birth_date|BirthDate" --type=kotlin -B 1 -A 1 | head -60Repository: Piece-Puzzly/Piece-Android
Length of output: 6538
🏁 Script executed:
#!/bin/bash
# Check if there are tests for TimeUtil
find . -name "*TimeUtil*Test*" -o -name "*Test*TimeUtil*" -type fRepository: Piece-Puzzly/Piece-Android
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Read the TimeUtilTest.kt to see tests for toBirthYear
cat -n "core/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.kt" | grep -A 30 "toBirthYear"Repository: Piece-Puzzly/Piece-Android
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Get more context around the toBirthYear test
cat -n "core/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.kt" | sed -n '190,210p'Repository: Piece-Puzzly/Piece-Android
Length of output: 446
🏁 Script executed:
#!/bin/bash
# Check all tests related to toBirthYear
rg -n "toBirthYear|birth.*year" --type=kotlin "core/common/src/test/kotlin" -B 5 -A 5Repository: Piece-Puzzly/Piece-Android
Length of output: 1007
🏁 Script executed:
#!/antml:thinking
I need to understand:
1. What the test input is for toBirthYear
2. How it's being used in the actual code (we saw it's used in ProfilePreviewViewModel and MainProfileViewModel)
3. Whether the server actually sends data in the format the function expects
4. Whether there are any error cases or edge cases being handled
Let me get the complete test context.
</thinking
bash
# Get the complete toBirthYear test function
cat -n "core/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.kt" | sed -n '190,205p'Repository: Piece-Puzzly/Piece-Android
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Check the API response model for birthdate field
rg -n "birthdate" --type=kotlin -B 3 -A 1 | grep -A 3 "data class\|class.*Response\|@SerializedName"Repository: Piece-Puzzly/Piece-Android
Length of output: 1289
🏁 Script executed:
#!/bin/bash
# Search for the profile API/repository to see what format is returned
rg -n "loadMyProfileBasic\|profileRepository" --type=kotlin -A 5 | head -40Repository: Piece-Puzzly/Piece-Android
Length of output: 52
🏁 Script executed:
#!/antml:thinking>
I need to understand what format the server actually sends for birthdate. The usage shows `it.birthdate.toBirthYear()` which means the birthdate field comes directly from the API response. Let me search for the data models.
</thinking>Repository: Piece-Puzzly/Piece-Android
Length of output: 292
🏁 Script executed:
#!/bin/bash
# Find data models/DTOs related to profile and birthdate
find . -name "*Profile*" -type f -name "*.kt" | grep -i "model\|dto\|response" | head -20Repository: Piece-Puzzly/Piece-Android
Length of output: 1581
🏁 Script executed:
#!/bin/bash
# Check the GetMyProfileBasicResponse to see birthdate format
cat -n "core/network/src/main/java/com/puzzle/network/model/profile/GetMyProfileBasicResponse.kt"Repository: Piece-Puzzly/Piece-Android
Length of output: 2077
🏁 Script executed:
#!/bin/bash
# Check the domain model MyProfile
cat -n "core/domain/src/main/java/com/puzzle/domain/model/profile/MyProfile.kt"Repository: Piece-Puzzly/Piece-Android
Length of output: 1449
🏁 Script executed:
#!/bin/bash
# Check for any other tests or usage patterns
rg -n "UNKNOWN_STRING\|birthdate.*toBirthYear\|\"00\"" --type=kotlin -B 2 -A 2 | head -50Repository: Piece-Puzzly/Piece-Android
Length of output: 52
🏁 Script executed:
#!/an/bash
# Check what UNKNOWN_STRING is set to
rg -n "UNKNOWN_STRING" --type=kotlin "core/network/src/main/java/com/puzzle/network/model" -A 1 -B 1Repository: Piece-Puzzly/Piece-Android
Length of output: 18199
🏁 Script executed:
#!/bin/bash
# Find where UNKNOWN_STRING is defined and its value
find . -name "*.kt" -type f -exec grep -l "const val UNKNOWN_STRING" {} \;Repository: Piece-Puzzly/Piece-Android
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Check GetOpponentProfileBasicResponse to see how they handle birthYear
cat -n "core/network/src/main/java/com/puzzle/network/model/matching/GetOpponentProfileBasicResponse.kt" | sed -n '1,50p'Repository: Piece-Puzzly/Piece-Android
Length of output: 2013
🏁 Script executed:
#!/bin/bash
# Check what happens when birthdate is UNKNOWN_STRING
# The value of UNKNOWN_STRING constant
grep -n "UNKNOWN_STRING = " core/network/src/main/java/com/puzzle/network/model/ApiResponse.ktRepository: Piece-Puzzly/Piece-Android
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Check if ProfilePreviewViewModel or MainProfileViewModel handle the birthdate
cat -n "feature/matching/src/main/java/com/puzzle/matching/graph/preview/ProfilePreviewViewModel.kt" | sed -n '25,40p'Repository: Piece-Puzzly/Piece-Android
Length of output: 793
🏁 Script executed:
#!/antml/bash
# Check MainProfileViewModel for birthdate handling
cat -n "feature/profile/src/main/java/com/puzzle/profile/graph/main/MainProfileViewModel.kt" | sed -n '25,40p'Repository: Piece-Puzzly/Piece-Android
Length of output: 837
🏁 Script executed:
#!/bin/bash
# Let's trace through what happens with "UNKNOWN"
python3 << 'EOF'
test_str = "UNKNOWN"
print(f"Input: '{test_str}'")
print(f"Length: {len(test_str)}")
print(f"Length >= 4: {len(test_str) >= 4}")
if len(test_str) >= 4:
result = test_str[:4][-2:]
print(f"take(4): '{result[:4]}'")
print(f"takeLast(2): '{result}'")
EOFRepository: Piece-Puzzly/Piece-Android
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check if there are any other places where birthdate could be null or UNKNOWN
rg -n "birthdate.*UNKNOWN\|\.birthdate\s*=\s*null" --type=kotlinRepository: Piece-Puzzly/Piece-Android
Length of output: 52
🏁 Script executed:
#!/antml/bash
# Let's also check the test file more completely to see if there are edge case tests
cat -n "core/common/src/test/kotlin/com/puzzle/common/TimeUtilTest.kt" | sed -n '180,204p'Repository: Piece-Puzzly/Piece-Android
Length of output: 833
Address silent data corruption when birthdate is missing or malformed.
The function lacks input validation, which can produce misleading results:
- If the API returns
nullfor birthdate, it becomes"UNKNOWN"(from fallback), andtoBirthYear()returns"WN"instead of a proper fallback - Invalid formats like
"abcd-ef-gh"silently return"gh" - This inconsistency with
toBirthDate()(which validates format and throws) makes error handling unpredictable
Instead of throwing an exception (which would crash when birthdate is missing), consider:
- Explicitly check for the
"UNKNOWN"string case and return a sentinel value - Add format validation using
isValidBirthDateFormat()liketoBirthDate()does - Document the expected input format and fallback behavior
🔎 Alternative approach
-/** yyyy-mm-dd -> yy 년생 ex) 1998 -> 98 */
+/** yyyy-mm-dd -> yy 년생 ex) 1998-03-21 -> 98. Returns placeholder for unknown/invalid dates. */
fun String.toBirthYear(): String =
- takeIf { length >= 4 }
- ?.take(4)
- ?.takeLast(2) ?: "00"
+ takeIf { this != "UNKNOWN" && length >= 4 && substring(0, 4).all { it.isDigit() } }
+ ?.take(4)
+ ?.takeLast(2)
+ ?: "00"Verify that the server birthdate format is always yyyy-mm-dd or yyyymmdd when present, and document this assumption.
🤖 Prompt for AI Agents
In @core/common/src/main/java/com/puzzle/common/TimeUtil.kt around lines 46-50,
The toBirthYear() function silently returns incorrect substrings for missing or
malformed input; update it to explicitly handle the "UNKNOWN" sentinel and
validate format before slicing: if the receiver equals "UNKNOWN" return a clear
sentinel (e.g., "00" or another agreed token), else call
isValidBirthDateFormat() (same validator used by toBirthDate()) and only then
extract the last two digits of the 4-digit year; if validation fails return the
sentinel instead of slicing arbitrary characters. Also update the KDoc for
toBirthYear() to declare the accepted input formats (yyyy-mm-dd or yyyymmdd) and
the fallback behavior so callers know what to expect.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt (1)
67-72: Consider usingjoinAllfor cleaner parallel execution.The parallel initialization logic is correct and addresses the loading state concern. However, you can simplify using
joinAll:🔎 Suggested refactor
viewModelScope.launch { val initInfoJob = launch { initMatchDetailInfo() } val checkMatchingPieceJob = launch { checkMatchingPiece(matchId) } - initInfoJob.join() - checkMatchingPieceJob.join() + joinAll(initInfoJob, checkMatchingPieceJob) setState { copy(isLoading = false) } }Add the import:
import kotlinx.coroutines.joinAll
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt
⏰ 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
🔇 Additional comments (4)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailViewModel.kt (4)
78-85: LGTM!Clean extraction of the initialization logic into a dedicated suspend function. The error handling delegation to
handleMatchingDetailErroris consistent with the rest of the codebase.
109-114: LGTM!Batching the profile and dialog state updates into a single
setStatecall is a good practice to minimize recomposition.
233-238: LGTM!The explicit multi-parameter form for
SnackBarSideEffect.Matchingimproves clarity by making the puzzle count argument visible.
269-271: LGTM!The function correctly delegates error handling while treating success as a no-op. This is appropriate for a validation/check operation where only failures need to trigger UI feedback.
1. ⭐️ 변경된 내용
2. 🖼️ 스크린샷(선택)
3. 💡 알게된 부분
4. 📌 이 부분은 꼭 봐주세요!
Summary by CodeRabbit
Style
New Features
Improvements
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.