Skip to content

Conversation

@kkh725
Copy link
Collaborator

@kkh725 kkh725 commented Dec 31, 2025

1. ⭐️ 변경된 내용

  • storeScreen + 다중매칭 구현으로 인해 추가로 생긴 purchase Dialog, 여러 형태의 카드의 amplitude logging
    trackClickEvent, TrackScreenViewEvent 추가

  • trackClickEvent가 최 상위 컴포넌트(screen 단) 에서 이루어지고 있던 형태를 하단(컴포넌트 단)으로 위치 이동.

    • 최 상단에 몰려있으니 가독성이 떨어지고 코드가 지저분해지는 느낌이어서 수정해봤습니다.

2. 🖼️ 스크린샷(선택)

3. 💡 알게된 부분

4. 📌 이 부분은 꼭 봐주세요!

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced user interaction tracking across matching and store features, capturing clicks on images, dialogs, and product listings.
  • Bug Fixes

    • Simplified dialog action flows by removing redundant tracking logic from confirmation handlers.
  • Chores

    • Refactored analytics implementation to use consistent screen name mapping across matching screens.
    • Updated store screen tracking to capture screen view events.

✏️ Tip: You can customize this high-level summary in your review settings.

@kkh725 kkh725 requested review from comst19 and tgyuuAn December 31, 2025 04:26
@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

This PR systematically adds analytics tracking across matching and store feature screens and dialogs. Changes include instrumenting click events on images, product cards, and dialog actions, removing duplicate analytics from detail screen handlers, and introducing an entryRoute parameter to propagate screen context through purchase dialogs.

Changes

Cohort / File(s) Summary
Dialog Analytics Integration
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/AcceptMatchingDialog.kt, RefuseMatchingDialog.kt, ProfileImageDialog.kt
Adds LocalAnalyticsHelper integration to wrap button click handlers with trackClickEvent calls; no API signature changes.
Purchase Dialog Refactoring
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt
Replaces TrackScreenViewEvent with buttonName-based click tracking; adds entryRoute: String parameter to InsufficientPuzzleDialog (public API change) to propagate screen context.
Matching Detail Screen Cleanup
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt
Removes analytics tracking from dialog click handlers; handlers now only invoke confirmation callbacks.
Contact Screen Image Tracking
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt
Adds LocalAnalyticsHelper and wraps 220dp image with clickable modifier that tracks "match_contact" / "image" event.
Matching Screen & Card Analytics
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt, component/card/MatchingCard.kt
Introduces screenName mapping for analytics consistency; refactors click handlers to compute buttonName dynamically; removes inline analytics from dialogs; updates entryRoute passing. MatchingCard changes ExpandedContent tracking logic and removes CollapsedContent analytics.
Store Analytics Integration
feature/store/src/main/java/com/puzzle/store/StoreScreen.kt, ui/list/NormalProductList.kt, ui/list/PromotionProductList.kt
Adds TrackScreenViewEvent for "store_main" screen and product-level click tracking with dynamic buttonName based on product details.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

기능 ⚒️, ㅌㄱ태규 ☀️

Suggested reviewers

  • tgyuuAn

Poem

🐰 Analytics hop across the screens so bright,
Tracking clicks with screenName, button names just right,
Purchase dialogs passed their entry route with care,
Store and matching glow with insights fair! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding Amplitude logging (trackClickEvent and TrackScreenViewEvent) related to multi-matching functionality.
Description check ✅ Passed The description covers the template structure with all required sections and clearly explains what changed: Amplitude logging additions and refactoring event tracking from screen-level to component-level.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kkh725 kkh725 added 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 ㄱㅎ근형 🌙 labels Dec 31, 2025
@kkh725 kkh725 changed the title [PC-1537] amplitude logging 추가 [PC-1537] 다중매칭 관련 Amplitude logging 추가 Dec 31, 2025
@kkh725
Copy link
Collaborator Author

kkh725 commented Dec 31, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
feature/store/src/main/java/com/puzzle/store/ui/list/PromotionProductList.kt (1)

37-44: Consider adding product-specific details to buttonName for consistency.

Unlike NormalProductList, which tracks clicks with "store_main_banner_${product.name}_${product.price}", this uses a generic "store_main_banner". Including product-specific details would provide better analytics granularity and consistency across different product types.

🔎 Suggested enhancement for product-specific tracking
                    .clickable {
                        onPurchaseClick(product)

                        analyticsHelper.trackClickEvent(
                            screenName = "store_main",
-                           buttonName = "store_main_banner",
+                           buttonName = "store_main_banner_${product.name}_${product.price}",
                        )
                    }
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt (1)

140-156: Excellent refactoring: centralized screen name mapping.

The screenName mapping based on matchingStatus provides a single source of truth for analytics event names and improves maintainability. The wrapped onNewMatchingCardClick correctly differentiates between free and purchase actions.

Optional: Consider extracting the wrapped function

For improved testability, you could extract the wrapped function as a named lambda:

+    val handleNewMatchingCardClick = {
+        onNewMatchingCardClick()
+        val buttonName = if (state.canFreeMatch) "new_matching_free" else "new_matching_purchase"
+        analyticsHelper.trackClickEvent(
+            screenName = screenName,
+            buttonName = buttonName,
+        )
+    }
+
-    val onNewMatchingCardClick = {
-        onNewMatchingCardClick()
-
-        val buttonName = if (state.canFreeMatch) "new_matching_free" else "new_matching_purchase"
-
-        analyticsHelper.trackClickEvent(
-            screenName = screenName,
-            buttonName = buttonName,
-        )
-    }

Then pass handleNewMatchingCardClick to the composables instead of shadowing the parameter name.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 160654d and ee41f93.

📒 Files selected for processing (11)
  • feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/AcceptMatchingDialog.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/ProfileImageDialog.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/RefuseMatchingDialog.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt
  • feature/store/src/main/java/com/puzzle/store/StoreScreen.kt
  • feature/store/src/main/java/com/puzzle/store/ui/list/NormalProductList.kt
  • feature/store/src/main/java/com/puzzle/store/ui/list/PromotionProductList.kt
🧰 Additional context used
🧬 Code graph analysis (5)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/ProfileImageDialog.kt (2)
core/designsystem/src/main/java/com/puzzle/designsystem/component/Dialog.kt (1)
  • PieceImageDialog (291-339)
core/analytics/src/main/java/com/puzzle/analytics/AnalyticsHelper.kt (1)
  • TrackScreenViewEvent (72-95)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/RefuseMatchingDialog.kt (1)
core/designsystem/src/main/java/com/puzzle/designsystem/component/Dialog.kt (5)
  • PieceDialog (33-58)
  • PieceDialogDefaultTop (60-84)
  • PieceDialogDefaultTop (86-110)
  • PieceDialogDefaultTop (112-136)
  • PieceDialogDefaultTop (138-162)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt (2)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/RefuseMatchingDialog.kt (1)
  • RefuseMatchingDialog (17-57)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt (2)
  • PurchaseDialog (19-82)
  • InsufficientPuzzleDialog (84-119)
feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt (2)
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt (2)
  • PurchaseDialog (19-82)
  • InsufficientPuzzleDialog (84-119)
feature/matching/src/main/java/com/puzzle/matching/graph/main/dialog/MatchingPoolEmptyDialog.kt (1)
  • MatchingPoolEmptyDialog (14-36)
feature/store/src/main/java/com/puzzle/store/StoreScreen.kt (1)
core/analytics/src/main/java/com/puzzle/analytics/AnalyticsHelper.kt (1)
  • TrackScreenViewEvent (72-95)
🔇 Additional comments (10)
feature/store/src/main/java/com/puzzle/store/StoreScreen.kt (1)

103-106: LGTM! Screen view tracking correctly implemented.

The TrackScreenViewEvent is properly placed and will fire once when the StoreScreen composable enters the composition, which is the expected behavior for screen view analytics.

feature/store/src/main/java/com/puzzle/store/ui/list/NormalProductList.kt (1)

38-45: LGTM! Product-specific analytics tracking implemented well.

The click tracking includes product details in the buttonName, which provides valuable granularity for understanding user engagement with specific products.

feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt (1)

163-170: LGTM! Simplified collapsed card interaction.

The removal of analytics tracking from the collapsed state aligns with the PR's objective to reorganize analytics at the component level. The direct pass-through of onCardClick simplifies the code while maintaining the core functionality.

feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt (1)

48-48: LGTM: Analytics helper properly initialized.

The import and initialization of LocalAnalyticsHelper follow the standard Compose composition local pattern.

Also applies to: 86-86

feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/AcceptMatchingDialog.kt (1)

11-11: LGTM! Analytics integration follows the established pattern.

The analytics helper is properly initialized and the click event is tracked after the acceptance action. The screenName "match_dialog" is consistent with other dialogs in this PR.

Also applies to: 23-23, 42-49

feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/ProfileImageDialog.kt (1)

5-5: LGTM! Analytics integration is consistent.

The approval click is properly tracked with appropriate screen and button names.

Also applies to: 19-19, 25-32

feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt (1)

160-181: Good separation of concerns by delegating analytics to dialogs.

Moving analytics tracking from the parent screen to individual dialog components improves code organization and aligns with the single-responsibility principle. The addition of entryRoute to InsufficientPuzzleDialog enables context-aware analytics.

feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt (1)

36-41: Clean buttonName mapping for purchase types.

The when expression clearly maps each PurchaseType to its corresponding analytics button name, making analytics events easy to trace and maintain.

Also applies to: 70-74

feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/RefuseMatchingDialog.kt (1)

9-9: LGTM! Consistent analytics integration.

The refusal action tracking follows the same pattern as other dialogs in this PR, with consistent screen and button naming.

Also applies to: 23-23, 44-51

feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt (1)

144-144: Good simplification: analytics responsibility moved to dialogs.

Removing inline analytics tracking from these dialog handlers and delegating the responsibility to the dialog components themselves improves separation of concerns and code maintainability. Each dialog now handles its own analytics, making the parent screen code cleaner.

Also applies to: 150-150, 162-162, 169-169, 176-176

Comment on lines +137 to +142
.clickable {
analyticsHelper.trackClickEvent(
screenName = "match_contact",
buttonName = "image",
)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd feature/matching && find . -name "ContactScreen.kt" -type f

Repository: Piece-Puzzly/Piece-Android

Length of output: 136


🏁 Script executed:

cat -n feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt | head -160 | tail -40

Repository: Piece-Puzzly/Piece-Android

Length of output: 1883


🏁 Script executed:

find feature/matching -name "*.kt" -type f | xargs grep -l "fun PieceImage\|@Composable.*PieceImage"

Repository: Piece-Puzzly/Piece-Android

Length of output: 54


🏁 Script executed:

rg -t kotlin "fun PieceImage|PieceImage\(" --max-count 20

Repository: Piece-Puzzly/Piece-Android

Length of output: 897


🏁 Script executed:

cat -n feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt | head -200

Repository: Piece-Puzzly/Piece-Android

Length of output: 8835


🏁 Script executed:

rg -t kotlin "match_contact|clickable.*image" --max-count 15

Repository: Piece-Puzzly/Piece-Android

Length of output: 213


🏁 Script executed:

cat -n core/designsystem/src/main/java/com/puzzle/designsystem/component/Image.kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 1021


🏁 Script executed:

find . -path "*/com/puzzle/common/ui*" -name "*.kt" -type f | head -20

Repository: Piece-Puzzly/Piece-Android

Length of output: 823


🏁 Script executed:

rg -t kotlin "fun clickable" --max-count 10

Repository: Piece-Puzzly/Piece-Android

Length of output: 52


🏁 Script executed:

cat -n core/common-ui/src/main/java/com/puzzle/common/ui/Modifier.kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 8046


🏁 Script executed:

rg -t kotlin "\.clickable\(" --max-count 30 -A 2

Repository: Piece-Puzzly/Piece-Android

Length of output: 4865


🏁 Script executed:

rg -t kotlin "isRipple\s*=\s*true|clickable.*isRipple" --max-count 20

Repository: Piece-Puzzly/Piece-Android

Length of output: 52


🏁 Script executed:

find . -name "README*" -o -name "*design*guide*" -o -name "*accessibility*" | head -20

Repository: Piece-Puzzly/Piece-Android

Length of output: 81


🏁 Script executed:

cat README.md | head -100

Repository: Piece-Puzzly/Piece-Android

Length of output: 3066


🏁 Script executed:

rg "isRipple" --max-count 20

Repository: Piece-Puzzly/Piece-Android

Length of output: 519


Consider adding visual feedback or user-facing action for the clickable image.

The profile image is clickable but only tracks analytics without providing visual feedback or performing any user-facing action. Users may tap the image expecting a response (such as viewing a full-screen version) but will receive none.

If the intent is solely to track engagement, consider either removing the clickable modifier or adding a feature such as a full-screen image viewer along with appropriate visual feedback (ripple effect or similar).

🤖 Prompt for AI Agents
In
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt
around lines 137-142, the profile image is marked clickable but only sends an
analytics event and gives no visual or functional feedback; either remove the
clickable modifier if you only need analytics, or implement a user-facing action
and visual feedback: add a ripple/indication to the clickable (use
Modifier.clickable(indication = rememberRipple() or default) or wrap with
Surface/ClickableText that shows pressed state), and open a full-screen image
viewer or image zoom dialog/activity on tap while still calling
analyticsHelper.trackClickEvent; also ensure the image has a contentDescription
for accessibility and that the analytics call happens after initiating the UI
action.


@Composable
internal fun InsufficientPuzzleDialog(
entryRoute: String,
Copy link

@coderabbitai coderabbitai bot Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find all usages of InsufficientPuzzleDialog and verify entryRoute is provided

# Search for InsufficientPuzzleDialog invocations
rg -nP -A5 'InsufficientPuzzleDialog\s*\(' --type=kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 96


🏁 Script executed:

#!/bin/bash
# Search for InsufficientPuzzleDialog invocations in Kotlin files without type filtering

rg -nP -A5 'InsufficientPuzzleDialog\s*\(' --glob='*.kt'

Repository: Piece-Puzzly/Piece-Android

Length of output: 3434


🏁 Script executed:

#!/bin/bash
# Get the full function signature for InsufficientPuzzleDialog

sed -n '85,89p' feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 200


🏁 Script executed:

#!/bin/bash
# Get more context around the MatchingDetailScreen call site

sed -n '175,185p' feature/matching/src/main/java/com/puzzle/matching/graph/main/MatchingScreen.kt
sed -n '175,185p' feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 825


🏁 Script executed:

#!/bin/bash
# Get precise context around MatchingDetailScreen.kt line 179

sed -n '177,183p' feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt

Repository: Piece-Puzzly/Piece-Android

Length of output: 295


Critical: Missing entryRoute parameter at MatchingDetailScreen.kt:179

The entryRoute parameter is required with no default value. While MatchingScreen correctly provides entryRoute = screenName, the InsufficientPuzzleDialog call at MatchingDetailScreen.kt:179-182 is missing this required parameter and will fail to compile.

Update MatchingDetailScreen.kt:179-182 to include:

entryRoute = "matching_detail",
🤖 Prompt for AI Agents
In
feature/matching/src/main/java/com/puzzle/matching/graph/detail/dialog/PurchaseMatchingDialog.kt
around line 86 and in MatchingDetailScreen.kt at lines 179-182, the
InsufficientPuzzleDialog invocation is missing the required entryRoute parameter
which has no default; update the call to pass entryRoute with the appropriate
value (e.g., entryRoute = "matching_detail") so the function compiles and
matches other usages like MatchingScreen's entryRoute = screenName.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 코멘트 한번 확인해주세요!
그리고 entryRoute는 로깅에만 쓰이고 있는 파라미터 인 것 같은데, Composable 함수 호출부에서 entryRoute라는 네이밍만 봐서는 이해가 안될 것 같아요.

entryRouteForLog 이라는 네이밍이나 이런 걸로 더 명확히 하면 좋을 것 같네요.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgyuuAn 확인해주셔서 감사합니다!

네, 맞습니다. 제가 지적한 MatchingDetailScreen.kt:179의 컴파일 오류는 반드시 수정이 필요합니다.

그리고 네이밍 제안도 정확한 지적입니다! entryRoute라는 이름만으로는 이 파라미터가 로깅 전용이라는 것을 알기 어렵습니다. 다음과 같은 네이밍을 권장합니다:

  • screenNameForLogging - 가장 명확하게 용도를 표현
  • analyticsScreenName - analytics 용도임을 직접 명시
  • entryRouteForLog - 현재 네이밍을 살리면서 용도 추가

이 중에서 screenNameForLogging이 가장 명확하고 이해하기 쉬울 것 같습니다. 함수 호출부에서 보더라도 바로 로깅 목적임을 알 수 있습니다.

정리하면:

  1. MatchingDetailScreen.kt:179에 누락된 파라미터 추가 필요 (컴파일 오류)
  2. entryRoutescreenNameForLogging (또는 유사한 명시적 네이밍)으로 변경 권장

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네이밍 수정 완료했습니다 ! -> f586c7e

코멘트 관련해서는 46f26ea 커밋에 반영했습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Comment on lines 179 to 182
INSUFFICIENT_PUZZLE -> InsufficientPuzzleDialog(
onDismissRequest = onDismissDialogClick,
onAcceptClick = {
onConfirmDialogClick(INSUFFICIENT_PUZZLE)
analyticsHelper.trackClickEvent(
screenName = "insufficient_puzzle_dialog",
buttonName = "purchase_puzzle",
)
}
onAcceptClick = { onConfirmDialogClick(INSUFFICIENT_PUZZLE) }
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Compilation error: Missing required entryRoute parameter.

The InsufficientPuzzleDialog now requires an entryRoute parameter (as changed in PurchaseMatchingDialog.kt), but this call site is missing it. This will prevent compilation.

🔎 Proposed fix

Following the pattern used for ProfileImageDialog (lines 156-160), derive the entry route from the current page:

 INSUFFICIENT_PUZZLE -> InsufficientPuzzleDialog(
+    entryRoute = when (state.currentPage) {
+        MatchingDetailPage.BasicInfoPage -> "basic_profile"
+        MatchingDetailPage.ValuePickPage -> "value_pick"
+        MatchingDetailPage.ValueTalkPage -> "value_talk"
+    },
     onDismissRequest = onDismissDialogClick,
     onAcceptClick = { onConfirmDialogClick(INSUFFICIENT_PUZZLE) }
 )
📝 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.

Suggested change
INSUFFICIENT_PUZZLE -> InsufficientPuzzleDialog(
onDismissRequest = onDismissDialogClick,
onAcceptClick = {
onConfirmDialogClick(INSUFFICIENT_PUZZLE)
analyticsHelper.trackClickEvent(
screenName = "insufficient_puzzle_dialog",
buttonName = "purchase_puzzle",
)
}
onAcceptClick = { onConfirmDialogClick(INSUFFICIENT_PUZZLE) }
)
INSUFFICIENT_PUZZLE -> InsufficientPuzzleDialog(
entryRoute = when (state.currentPage) {
MatchingDetailPage.BasicInfoPage -> "basic_profile"
MatchingDetailPage.ValuePickPage -> "value_pick"
MatchingDetailPage.ValueTalkPage -> "value_talk"
},
onDismissRequest = onDismissDialogClick,
onAcceptClick = { onConfirmDialogClick(INSUFFICIENT_PUZZLE) }
)
🤖 Prompt for AI Agents
In
feature/matching/src/main/java/com/puzzle/matching/graph/detail/MatchingDetailScreen.kt
around lines 179-182, the InsufficientPuzzleDialog invocation is missing the new
required entryRoute parameter; update the call to include entryRoute derived
from the current page (following the same pattern used for ProfileImageDialog on
lines 156-160) so that you pass the route created from the current page
id/context (e.g., use the same Screen.MatchingDetail.createRoute(pageId) or
equivalent helper used elsewhere) and keep the existing onDismissRequest and
onAcceptClick arguments.

Comment on lines +143 to +148
val buttonName = when {
matchInfo.matchStatus == MATCHED -> "match_matched"
matchInfo.matchType == MatchType.AUTO -> "match_auto"
matchInfo.matchType == MatchType.BASIC -> "match_basic"
else -> return@onClickExpandedContent
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

echo "=== Searching for MatchType enum definition ==="
ast-grep --pattern 'enum class MatchType {
  $$$
}'

echo ""
echo "=== Searching for MatchType usages and values ==="
rg -nP --type=kotlin 'MatchType\.\w+' -C2

echo ""
echo "=== Searching for when statement context in MatchingCard ==="
rg -n "buttonName = when" feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt -A10 -B3

Repository: Piece-Puzzly/Piece-Android

Length of output: 20521


🏁 Script executed:

rg -n "onClickExpandedContent" feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt -B5 -A20

Repository: Piece-Puzzly/Piece-Android

Length of output: 1545


Add analytics tracking for TRIAL and PREMIUM match types.

The when {} logic correctly prioritizes matchStatus == MATCHED over matchType—this is intentional, not a bug. However, the logic only handles AUTO and BASIC types; cards with TRIAL or PREMIUM types will skip analytics tracking entirely via the else → return clause.

The test preview data (MatchInfoPreviewProvider.kt) includes TRIAL and PREMIUM match types, confirming they are legitimate, trackable card states. Add analytics event names for these types:

val buttonName = when {
    matchInfo.matchStatus == MATCHED -> "match_matched"
    matchInfo.matchType == MatchType.AUTO -> "match_auto"
    matchInfo.matchType == MatchType.BASIC -> "match_basic"
    matchInfo.matchType == MatchType.TRIAL -> "match_trial"        // Add this
    matchInfo.matchType == MatchType.PREMIUM -> "match_premium"    // Add this
    else -> return@onClickExpandedContent
}
🤖 Prompt for AI Agents
In
feature/matching/src/main/java/com/puzzle/matching/graph/main/component/card/MatchingCard.kt
around lines 143-148, the when {} branch only handles MATCHED, AUTO and BASIC
and returns on any other matchType, so TRIAL and PREMIUM cards skip analytics;
add two additional branches for matchInfo.matchType == MatchType.TRIAL mapping
to "match_trial" and matchInfo.matchType == MatchType.PREMIUM mapping to
"match_premium" (keeping the existing MATCHED priority) so those types emit
analytics instead of falling through to the else-return.

@kkh725 kkh725 closed this Dec 31, 2025
@kkh725 kkh725 reopened this Dec 31, 2025
Copy link
Collaborator

@comst19 comst19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.
modifier의 clickable을 사용하는 곳은 따로 확장해서 중복코드를 줄일 수 있지 않을까요?


@Composable
internal fun InsufficientPuzzleDialog(
entryRoute: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 코멘트 한번 확인해주세요!
그리고 entryRoute는 로깅에만 쓰이고 있는 파라미터 인 것 같은데, Composable 함수 호출부에서 entryRoute라는 네이밍만 봐서는 이해가 안될 것 같아요.

entryRouteForLog 이라는 네이밍이나 이런 걸로 더 명확히 하면 좋을 것 같네요.

Comment on lines +140 to +148
onClick = onClickExpandedContent@{
onCardClick()

val buttonName = when {
matchInfo.matchStatus == MATCHED -> "match_matched"
matchInfo.matchType == MatchType.AUTO -> "match_auto"
matchInfo.matchType == MatchType.BASIC -> "match_basic"
else -> return@onClickExpandedContent
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 스코프에 네이밍 주는거 필요하나요 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onClick이 람다라서 네이밍 안주면 return 에 컴파일 에러 나서 네이밍 추가했습니다!

@kkh725
Copy link
Collaborator Author

kkh725 commented Jan 3, 2026

modifier의 clickable을 사용하는 곳은 따로 확장해서 중복코드를 줄일 수 있지 않을까요?

->> 의존성 관련해서 생각해봐야할 부분들이 있을 것 같아서 리팩토링 할 때 같이 고민해보면 좋을 것 같아요 !

@kkh725 kkh725 merged commit cc7691c into feature/PC-1514 Jan 3, 2026
1 check passed
@kkh725 kkh725 deleted the feature/PC-1537 branch January 3, 2026 08:37
comst19 added a commit that referenced this pull request Jan 3, 2026
* [PC-1507] MatchingScreen 에서 발생하는 에러 예외 처리 (#187) (#188)

* fix: gson 제거 및 proto datastore로 마이그레이션 하면서 consumer-rules 수정

* [PC-1507] MatchingViewmodel 에러 처리 로직 수정

* [PC-000] 퍼즐 부족 다이얼로그 표출되자마자 꺼지는 이슈 수정 (#189)

* [PC-1514] 매칭 풀 소진 다이얼로그 구현

* [PC-1514] 매칭 풀 소진 다이얼로그 show, hide 로직 추가

* [PC-1514] 이미 구매한 연락처 에러 처리 로직 추가

* [PC-1514] 퍼즐 부족 에러 시 사용자 퍼즐 요청

* [PC-1514] 뉴 매치일 때 checkMatchingPiece 하도록 변경

* [PC-1514] MatchingPoolEmptyDialog 미구현 로직 주석 추가

* [PC-1514] 연락처 조회 실패 시 에러 로직 함수 호출 시 matchId 인자 추가

* [PC-1514] 새 매칭 진입 시 펼쳐진 카드 변경

* [PC-1514] ci 실패 수정

* [PC-1514] getInstantNewMatch에 퍼즐 사용 파라미터 추가

* [PC-1514] onRefresh -> onPullToRefresh 변경

* [PC-1514] PreviewMatchingPoolEmptyDialog 가시성 추가

* [PC-1514] getInstantNewMatch 파라미터 명 변경

* [PC-1514] PreviewTimerCompleteDialog가시성 추가

* [PC-1537] 다중매칭 관련 Amplitude logging 추가 (#191)

* [PC-1537] analyticsHelper 미구현 screen 로깅 추가 및 최상단이 아닌 컴포넌트단으로 위치 이동

* [PC-1537] MatchingDetailScreen에 entryRoute 추가

* [PC-1537] entryRoute -> entryRouteForLog 로 파라미터명 수정

* [PC-1537] entryRoute -> entryRouteForLog 로 파라미터명 수정

* [PC-1537] AmplitudeEvent를 track, revenue 타입으로 변경

* [PC-1537] billing 모듈에 analytics 의존성 추가

* [PC-1537] trackRevenue 추가

* [PC-1534] 매칭 풀 소진 다이얼로그(기본) 구현 (#192)

* [PC-1534] 매칭 풀 다이얼로그(기본) show/hide 로직 구현

* [PC-1534] 매칭 풀 다이얼로그 show/hide 로직 수정

* [PC-1534] showPurchaseDialog -> showDialog

* [PC-1534] MatchingPoolType -> MatchingPoolEmptyType

* [PC-1534] isModalShownToday -> isBasicMatchingPoolEmptyDialogShownToday

* [PC-1534] getStandardDate -> getResetDateString

* [PC-1534] getResetDateString 함수 테스트 코드 작성

* [PC-1534] LastModal -> LastMatchingPoolEmptyDialog

* [PC-1534] 로그아웃, 탈퇴 시 매칭 풀 소진 다이얼로그 clear

* [PC-1534] LastModal -> LastMatchingPoolEmptyDialog

* [PC-1534] LastModal -> LastMatchingPoolEmptyDialog

* [PC-1534] 로그인, 로그아웃 테스트 코드에 lastMatchingPoolEmptyDialogShownDate 빈 값 확인 코드 추가

* [PC-1534] 매칭 풀 소진 다이얼로그(기본) show/hide 테스트 코드 추가

* [PC-1534] 매칭 풀 소진 다이얼로그(기본) subText 수정

* [PC-1534] 매칭 풀 소진 다이얼로그 표출 확인 테스트 코드 작성

* [PC-1534] 매칭 풀 소진 다이얼로그 본 날짜 토큰 만료시 제거하도록 변경

* [PC-1534] 맛보기 매칭 풀 소진일 때는 Basic 매칭 풀 소진 표출되도록 변경

* [PC-1534] lastMatchingPoolEmptyDialogShownDate 타입 non-null로 변경

* [PC-1514] ci 실패 수정

* [PC-1514] ci 실패 수정

---------

Co-authored-by: Kim Kuen Hyung <120651330+kkh725@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ㄱㅎ근형 🌙 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants