Feature/jaino/#160 #163

jeongjaino · 2024-03-27T16:31:25Z

1. 📄 관련된 이슈 및 소개

#160 Firebase Event Log 구현

2. 🔥 변경된 점

Firebase Event Log 클래스를 구현하였습니다.

Analytics 모듈을 참조하여, Composition Scope에서는 LocalAnalyticsHelper.current로 접근할 수 있고, 일반 코어모듈 및 뷰모델에서는 Hilt를 통해 AnalyticsHelper 클래스를 주입받아서 로깅하시면 됩니다.

불필요한 클래스를 소거하였습니다.

KtLint/Management 테스트 결과 디렉토리가 레포에 잘못 올려진 것을 소거하였습니다.
common 모듈 내 사용하지 않는 익스텐션을 제거하였습니다.
UseCase내 HiltScope를 제거하였습니다.

3. ✅ 꼭 확인해줬으면 하는 부분

AnalyticsHelper 확인 부탁드립니다.
- 사용법 이해와 잘 들어오는 지 ! (1시간에 한번씩 집계되어서, 바로 확인이 안될 수 있어요.)
- 바로 확인하려면 ADB 연결해서, Firebase DebugView 확인해보세요 ! (모르시면 구글에 검색해서 독스대로 진행!)
추가로 로깅필요한 부분 언급 부탁드립니다.

추가로, Release 병합 후, Develop도 병합 진행해야 합니다.

4. 📸 스크린샷(선택)

5. 💡알게된 혹은 궁금한 사항들

tgyuuAn

진호 님이 올려주신 레퍼런스가 그렇게 어렵지도 않고,

compositionLocal 부분 새로운 키워드를 얻은 것 같아서 좋네요.

코드도 읽기 쉬워서 좋았어요..!

제가 궁금한 부분 살짝쿵 댓글로 담아봤어요.!

Analytics 모듈을 참조하여, Composition Scope에서는 LocalAnalyticsHelper.current로 접근할 수 있고,

일반 코어모듈 및 뷰모델에서는 Hilt를 통해 AnalyticsHelper 클래스를 주입받아서 로깅하시면 됩니다.

확인했씁니다.

tgyuuAn · 2024-03-28T02:28:45Z

core/analytics/src/main/java/com/wap/wapp/core/analytics/LocalAnalyticsHelper.kt

+
+import androidx.compose.runtime.staticCompositionLocalOf
+
+val LocalAnalyticsHelper = staticCompositionLocalOf<AnalyticsHelper> {


이 부분 약간 아리까리한게,

Activity가 필요해서 staticCompositionLocal에 Activity하나 선언해서 필요한 곳마다 뿌려주는 것 같은데요,

굳이 MainActivity가 아닌 LocalActivity를 사용하는 이유가 있나요 ??

약~~간 좀 이해가 안되요 제가 이걸 안써봐서 그런가 ㅠ

진호님이 올려주신 짱짱한 레퍼런스에 해당 부분을 봐도 잘 이해가...

일단 A to Z로 다 가르쳐 주긴 하는데 용도가 너무 궁금하네요.

오 태규상 제가 해당 블로그를 보냈었네요 !

이번 구현하면서, 해당 블로그는 참조하지 않았어요 ,,, ㅎㅎ

제가 원하던 방식이 아니였거든요.

제가 참조한 방식은 nowInAndroid의 analytics 모듈을 참조했어요.

CompositionLocal은 이전 MainActivity에서 Screen까지 매개변수로 전달하는 방식이 아니라,

트리 바깥에 선언해놓고, Ui트리 어디서든지 꺼내서 사용할 수 있어요 !
(저희 Color, Typograp도 알게 모르게 전달 없이 사용하는 것처럼요 ㅎㅎ)

해당 블로그 포스팅과 다르게, 저는 Firebase Analytics에 로깅을 수행하는
AnalyticsHelper 클래스를 그대로 CompositionLocal로 선언했어요.

일반적인 LifeCycleScope나 CoroutinScope와 다르게, CompositionScope내에서 쉽게 접근하게 하기 위해서죠 !

이게 보기에는 어렵게 보이는데, 실제로는 힐트랑 저는 유사하다는 느낌이 들었어요.

현재 태규님이 리뷰 달아주신 선언부가 인터페이스고, MainActivity내에서 주입하는 것처럼요 !

tgyuuAn · 2024-03-28T02:29:09Z

core/common/build.gradle.kts

@@ -3,7 +3,7 @@ plugins {
 }

 android {
-    namespace = "com.wap.wapp.core.base"


tgyuuAn · 2024-03-28T02:32:01Z

core/analytics/src/main/java/com/wap/wapp/core/analytics/AnalyticsEvent.kt

+data class AnalyticsEvent(
+    val type: String,
+    val extras: List<Param> = emptyList(),
+) {
+    data class Param(
+        val key: String,
+        val value: String,
+    )
+
+    companion object {
+        const val SCREEN_VIEW = "screen_view" // TYPE
+        const val SCREEN_NAME = "screen_name" // EXTRA_KEY
+    }
+}


이 부분 잘 이해가 안되는 게,

screen_view하고 screen_name하고 차이점을 잘 모르겠어요.

아마도.. 제 예상컨대 저 상수값은 Log찍을 때 TAG 부분에 해당하는 것 처럼 key 값으로 들어가는 것 같은데..

맞나요..?

view 와 name의 차이가 뭐죠 ㅠㅠ? 컴포넌트 단위와 화면 단위의 차이인가 ..?

Firebase Analytics Event의 구현부는 두가지로 되어있어요 !

이벤트 이름 (어떤 이벤트인가?_

이벤트 번들 (빈 값이여도 가능해요, 키-값의 형태로 해당 이벤트를 나타내는 값을 보통 넣어요)

이렇게 보면, 이벤트 이름 -> Type, 이벤트 번들 -> Param으로 정했어요

그럼 ?
이벤트 이름 : ScreenView,
이벤트 번들 : Key -> ScreenName, Value -> ManagementSurveyScreen

한글로 풀어쓴다면 ?
ScreenView의 이벤트를 기록하는데, ScreenName이 ManagementSurveyScreen인 이벤트를 기록 !

tgyuuAn · 2024-03-28T02:46:00Z

core/common/src/main/java/com/wap/wapp/core/commmon/extensions/AnalyticsExtensions.kt

+@Composable
+fun TrackScreenViewEvent(
+    screenName: String,
+    analyticsHelper: AnalyticsHelper = LocalAnalyticsHelper.current,
+) = LaunchedEffect(Unit) {
+    analyticsHelper.logScreenView(screenName)
+}


해당 함수를 LaunchedEffect로 감싸는 이유는 suspend 를 사용하기 위함 보다는,

메인 스레드에 영향을 주지 않기 위해서 백그라운드로 동작시키기 위함 이 맞을까요 ?

Analytics 내부 코드를 확인해보지 않았지만, 제가 작성한 코드 내에서는 suspend가 없어요 !

LaunchEffect로 감싼이유는

엄연히 서버에 로깅하는 백그라운드 작업

컴포지션 스코프 내에서, 안전하게 일회성으로 호출

tgyuuAn · 2024-03-28T03:01:30Z

음 이거 해당 화면 별로 로그가 찍히는 것이 아닌가요 ?!

아니면 아직 반영되지 않은 것인가 ?!

jeongjaino · 2024-03-28T12:14:30Z

음 이거 해당 화면 별로 로그가 찍히는 것이 아닌가요 ?!

아니면 아직 반영되지 않은 것인가 ?!

태규상 저도 나중에 확인해볼게요 !

혹시 Firebase 탭내에서 DebugView에서 확인하신게 맞을까요 ??

tgyuuAn

소소한 코멘트 달아봐씁니다!!!!!!!!

tgyuuAn · 2024-03-28T16:54:30Z

feature/auth/src/main/java/com/wap/wapp/feature/auth/signin/SignInScreen.kt

@@ -63,6 +68,7 @@ internal fun SignInRoute(
 @OptIn(ExperimentalMaterial3Api::class, ExperimentalComposeUiApi::class)
 @Composable
 internal fun SignInScreen(
+    viewModel: SignInViewModel,


진호상 Screen에 ViewModel을 주입하는 것 보다 route에서 viewModel의 행위를 넘겨줘서,

로직을 Screen에서 정하기 보다는 Route에서 정의하는 것으로, 행위의 주체를 역전시키는 것이 어떨까요?!

xml에서 MVVM 쓰듯이, Screen에서 행위를 결정할 수 없도록 만드는 거요!

https://www.charlezz.com/?p=45623

굳이 viewModel을 넘길필요 없이

로그 콜백을 만들어서 사용해서 구현은 가능할 것 같아요 !

그런데 태규님께서 말씀하신 행위 주체의 역전이라는 의미를 잘 모르겠아요 ,,!

결국 어디선가는 뷰모델 함수를 호출해야 하고,
해당 부분이 스크린 -> 라우트로 변경되는 것인디요 점시

@jeongjaino 실질적으로 UI를 그리는 Screen에서 ViewModel의 어떤 함수를 호출할 지를 결정하지 말고 Route에서 함수를 넘겨줘서 넘겨진 것을 받아먹는 식으로 하는 것이 어떠냐는 뜻이었어요.

지금은 Screen에서 ViewModel의 함수를 마음대로 호출할 수 있지만 그렇게 되면 UI랑 관련없는 Route에서 주는 대로 함수를 호출하니까요.

즉 Screen은 UI영역만 일을 할 수 있고 함수 호출과 같은 일들은 Screen보다 상위 레이어이면서 UI랑은 관련없는 Route에서 하는 것이죠.

++

이와 관련된 글을 어디서 봤더라.. 했는데 compositionLocal 을 공부하면서 아래 게시글이서 봤었네요.

https://velog.io/@jaewonkim1468/JetpackCompose9-CompositionLocal

이렇게 함의 이유는 함수를 주입받는 함수나 클래스의 재사용성을 위함인데,

Compose를 사용할 때는 @Preview를 사용함을 위함이라고도 볼 수 있겠네요...

저희는 지금 screen마다 Preview를 안달아놨으니 필요없다고 볼 수도 있겠군요.

저도 처음에는 "다들 이렇게 하니까 당연히 해야지" 라는 식으로 구현을 하긴 했는데,,

찾아보니 지금 상황에서는 필요 없을 수도 있겠다 라는 생각이 드네요.

저는 좋은 것 같아요.

굳이 ViewModel을 넘길 필요도 없고, ViewModel을 여러곳에 넘겨버리다가
ViewModel에 대한 접근이 많아지면 관리하기도 어려우니까요.

행위 주체의 역전이라는 의미를 잘 모르겠어서 물어봤습니다
결국은 Screen이나 라우트나 별 의미가 없다고 생각했었는데,

생각도 해보고, 태규상 의견도 들으니 Screen은 말 그대로 화면에 대한 컴포저블을 담고있고,
라우트는 화면을 그리는 컴포저블과는 나름 무관하니, 올리는게 좋아보입니다.

수완 (수정완료 캬캬(

tgyuuAn · 2024-03-28T16:55:49Z

feature/auth/src/main/java/com/wap/wapp/feature/auth/signin/SignInScreen.kt

+                            AuthState.SIGN_IN -> {
+                                logUserSignedIn()
+                                navigateToNotice()
+                            }
+
+                            AuthState.SIGN_UP -> {
+                                navigateToSignUp()
+                            }


이 부분 메소드 하나로 묶어서 넘겨주는 것은 별로일까요 ??

handle ~~()로 구현은 가능할 것 같아요 !

하지만 구현부가, 함수를 호출하는 부분이라,, 저는 분리의 필요성을 크게 못느끼겠어요

혹시 태규님이 생각하신, 분리하면 좋을 것 같은 이유를 알려주실 수 있으실까요??

@Composable private fun SignInButton( email: String, coroutineScope: CoroutineScope, signInUseCase: SignInUseCase, logUserSignedIn: () -> Unit, navigateToNotice: () -> Unit, navigateToSignUp: () -> Unit, snackBarHostState: SnackbarHostState, ) {

해당 로직이 SignInButton이라는 컴포넌트에서 사용되고 있는데,

어짜피 when절에서 SignIn을 타고 해당 메소드 두개를 호출해야하는 상황이라면,

하나의 메소드만 호출하게 바꾸어서 불필요한 파라미터를 하나라도 줄일 수 있을 것 같아요.

어떻게 생각하시나요 ?!

음 태규상 제가 이해를 제대로 한지 모르겠는데,

구체적으로 보자면, NativgateNotice() 내부에, 로깅함수를 수행하자는 말씀이실까요 ??

@Composable internal fun SignInRoute( viewModel: SignInViewModel = hiltViewModel(), signInUseCase: SignInUseCase, navigateToSignUp: () -> Unit, navigateToNotice: () -> Unit, ) { SignInScreen( signInUseCase = signInUseCase, navigateToSignUp = navigateToSignUp, navigateToNotice = { viewModel.logUserSignedIn() navigateToNotice() }, ) }

다음과 같이, 라우트 내에서 수행하도록 말씀하신게 맞을까요
??

맞긴합니다. 거기에 람다 파라미터 네이밍만 바꾸는?.?

구두 협의 후

tgyuuAn · 2024-03-28T16:56:58Z

...profile/src/main/java/com/wap/wapp/feature/profile/profilesetting/ProfileSettingViewModel.kt

+                analyticsHelper.logEvent(
+                    AnalyticsEvent(
+                        type = "signed_out",
+                        extras = listOf(
+                            AnalyticsEvent.Param(
+                                key = "user_id",
+                                value = userProfile.userId,
+                            ),
+                            AnalyticsEvent.Param(
+                                key = "user_name",
+                                value = userProfile.userName,
+                            ),
+                        ),
+                    ),
+                )


아항 요론식으로 쓰는 거군뇨 👍👍

tgyuuAn · 2024-03-28T17:01:01Z

음 이거 해당 화면 별로 로그가 찍히는 것이 아닌가요 ?!
아니면 아직 반영되지 않은 것인가 ?!

태규상 저도 나중에 확인해볼게요 !

혹시 Firebase 탭내에서 DebugView에서 확인하신게 맞을까요 ??

악 DebugView가 아니라 Realtime Analytics로 확인했네요... 내일 아침에 확인해보겠씁니닷

tgyuuAn · 2024-03-29T12:35:12Z

LGTM!!!!!!!!!!!!!!!!!!!!!!!!!!!!

jeongjaino added 10 commits March 28, 2024 01:19

[CHORE] : Analytics 모듈 생성

37ef449

[FEATURE] : Analytics Event Model 클래스 구현

66f03b3

[FEATURE] : AnalyticsHelper, AnalyticsHelperImpl 구현

670a062

[FEATURE] : common내의 Screen analytics Extenstion 구현

3eb2537

[FEATURE] : AnalyticsHelper CompositionLocal 구현체 주입 및 암시적 전달 구현

2ef1b2a

[FEATURE] : AnalyticsHelper CompositionLocal 구현체 구현

50455f5

[FEATURE] : Screen Event 로깅 구현

f8aed83

[CHORE] : 사용하지 않는 유틸 클래스 소거

348df2a

[CHORE] : 유스케이스 내 불필요한 HiltScope 소거

2f3979a

[CHORE] : ktlint 테스트 파일 소거

49bba05

jeongjaino added 🚀진호🚀 ESTP 정진호 23세 🌱기능🌱 새로운 기능 두두둥장! 🔥핫픽스🔥 릴리즈 후 프로덕션 레벨로 올리기 전 수정작업 labels Mar 27, 2024

jeongjaino self-assigned this Mar 27, 2024

jeongjaino linked an issue Mar 27, 2024 that may be closed by this pull request

[FEATURE]: Firebase Event Log 구현 #160

Closed

2 tasks

jeongjaino changed the base branch from main to release March 27, 2024 16:32

tgyuuAn self-requested a review March 27, 2024 17:00

tgyuuAn approved these changes Mar 28, 2024

View reviewed changes

tgyuuAn added the 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다. label Mar 28, 2024

jeongjaino added 4 commits March 29, 2024 00:59

[CHORE] : 로그인 Analytics Extension 구현

937ee94

[FEATURE] : 로그인 Analytics Event 로깅 구현

f7004ae

[FEATURE] : 로그아웃, 탈퇴 Analytics Event 로깅 구현

e540589

[FEATURE] : 로그아웃 Analytics Event 로깅 구현

d7aa788

tgyuuAn approved these changes Mar 28, 2024

View reviewed changes

[CHORE] : logUserSignedIn Route로 로직 올리기

ea71a76

tgyuuAn added 🌟머지 해주세요🌟 코드 리뷰가 완료된 뒤 PR을 올린사람이 Merge를 하면 되는 단계입니다. and removed 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다. labels Mar 29, 2024

[CHORE] : onSignInSucceed 네이밍 수정

ab45c86

jeongjaino merged commit bc31171 into release Mar 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/jaino/#160 #163

Feature/jaino/#160 #163

jeongjaino commented Mar 27, 2024 •

edited

Loading

tgyuuAn left a comment

tgyuuAn Mar 28, 2024

jeongjaino Mar 28, 2024

tgyuuAn Mar 28, 2024

tgyuuAn Mar 28, 2024

jeongjaino Mar 28, 2024

tgyuuAn Mar 28, 2024

jeongjaino Mar 28, 2024

tgyuuAn commented Mar 28, 2024

jeongjaino commented Mar 28, 2024

tgyuuAn left a comment

tgyuuAn Mar 28, 2024 •

edited

Loading

jeongjaino Mar 29, 2024

tgyuuAn Mar 29, 2024

jeongjaino Mar 29, 2024

jeongjaino Mar 29, 2024

tgyuuAn Mar 28, 2024

jeongjaino Mar 29, 2024

tgyuuAn Mar 29, 2024

jeongjaino Mar 29, 2024

jeongjaino Mar 29, 2024

tgyuuAn Mar 29, 2024

jeongjaino Mar 29, 2024

tgyuuAn Mar 28, 2024

tgyuuAn commented Mar 28, 2024

tgyuuAn commented Mar 29, 2024


		import androidx.compose.runtime.staticCompositionLocalOf

		val LocalAnalyticsHelper = staticCompositionLocalOf<AnalyticsHelper> {

Feature/jaino/#160 #163

Feature/jaino/#160 #163

Conversation

jeongjaino commented Mar 27, 2024 • edited Loading

1. 📄 관련된 이슈 및 소개

2. 🔥 변경된 점

3. ✅ 꼭 확인해줬으면 하는 부분

추가로, Release 병합 후, Develop도 병합 진행해야 합니다.

4. 📸 스크린샷(선택)

5. 💡알게된 혹은 궁금한 사항들

tgyuuAn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgyuuAn commented Mar 28, 2024

jeongjaino commented Mar 28, 2024

tgyuuAn left a comment

Choose a reason for hiding this comment

tgyuuAn Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgyuuAn commented Mar 28, 2024

tgyuuAn commented Mar 29, 2024

jeongjaino commented Mar 27, 2024 •

edited

Loading

tgyuuAn Mar 28, 2024 •

edited

Loading