Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔀 :: (#496) 로그인, 로그아웃 시 Analytics UserID 변경 #498

Merged
merged 3 commits into from
Apr 23, 2024

Conversation

baekteun
Copy link
Member

💡 배경 및 개요

애널리틱스 시 User를 식별하기 위한 UserID를 지정하는 로직을 추가해요

Resolves: #496

📃 작업내용

  • 로그인 시 UserID를 새로 할당해줘요.
  • 로그아웃 시 UserID를 nil로 바꿔요

✅ PR 체크리스트

템플릿 체크리스트 말고도 추가적으로 필요한 체크리스트는 추가해주세요!

  • 이 작업으로 인해 변경이 필요한 문서가 변경되었나요? (e.g. XCConfig, 노션, README)
  • 이 작업을 하고나서 공유해야할 팀원들에게 공유되었나요? (e.g. "API 개발 완료됐어요", "XCConfig 값 추가되었어요")
  • 작업한 코드가 정상적으로 동작하나요?
  • Merge 대상 브랜치가 올바른가요?
  • PR과 관련 없는 작업이 있지는 않나요?

@baekteun baekteun changed the base branch from develop to 496-add-logging-manager April 21, 2024 15:30
@github-actions github-actions bot added 1️⃣ Priority: High 우선순위 상 ⚙ Setting 개발 환경 세팅 labels Apr 21, 2024
Copy link

github-actions bot commented Apr 21, 2024

✅ Successful finished SwiftLint

Copy link

✅ Assign 자동 지정을 성공했어요!

@baekteun

@KangTaeHoon
Copy link
Contributor

AnalyticsLogManager.setUserID(userID: nil)
이 반복되는 코드 logoutUseCase 구현체로 밀어넣을 수 없을까요?

@baekteun
Copy link
Member Author

AnalyticsLogManager.setUserID(userID: nil)

이 반복되는 코드 logoutUseCase 구현체로 밀어넣을 수 없을까요?

이 친구가 되게 고민됐던게 Analytics의 설정을 건드리는걸 도메인 로직을 다루는 Domain 레이어에서 한다? 가 굉장히 고민했었어요.🥺
그래서 결국 AnalyticsLogManager 모듈이 BaseFeature에서만 의존하고 있다보니 반복되더라도 Feature레이어의 영역에서 해결하는게 낫다고 생각해서 요렇게 되어버린거같네요ㅜ
혹시 괜찮은 아이디어있을까요.?.?

@KangTaeHoon
Copy link
Contributor

KangTaeHoon commented Apr 22, 2024

이 친구가 되게 고민됐던게 Analytics의 설정을 건드리는걸 도메인 로직을 다루는 Domain 레이어에서 한다? 가 굉장히 고민했었어요.🥺 그래서 결국 AnalyticsLogManager 모듈이 BaseFeature에서만 의존하고 있다보니 반복되더라도 Feature레이어의 영역에서 해결하는게 낫다고 생각해서 요렇게 되어버린거같네요ㅜ 혹시 괜찮은 아이디어있을까요.?.?

개인적으로는 우리가 편한게 클린한 코드라는 입장이라 너무 이론대로 따라가진 말자는 생각이에요.
Or
MainTabBarViewModel.swift 정도의 위치에서 아래의 코드를 넣는 방법도 있을 것 같아요.
PreferenceManager.userInfo가 변경될 때마다 반응하는 구조로 돼있는데,
한 가지 예외가 Intro 화면에서 일어난 로그아웃 일 때 처리는 그대로 남겨둬야하는게 있네요. (탭바 컨트롤러에 들어오기 이전 화면이기때문)

        Utility.PreferenceManager.$userInfo
            .bind { (user) in
                if let userID = user?.ID {
                    // AnalyticsLogManager.setUserID(userID: AES256.decrypt(encoded: userID))
                } else {
                    // AnalyticsLogManager.setUserID(userID: nil)
                }
            }
            .disposed(by: disposeBag)

@youn9k
Copy link
Member

youn9k commented Apr 22, 2024

이 친구가 되게 고민됐던게 Analytics의 설정을 건드리는걸 도메인 로직을 다루는 Domain 레이어에서 한다? 가 굉장히 고민했었어요.🥺
그래서 결국 AnalyticsLogManager 모듈이 BaseFeature에서만 의존하고 있다보니 반복되더라도 Feature레이어의 영역에서 해결하는게 낫다고 생각해서 요렇게 되어버린거같네요ㅜ
혹시 괜찮은 아이디어있을까요.?.?

클린 아키텍쳐 입장에서 보면 이게 맞긴한데, AnalyticsLogManager.setUserID(userID: nil) 가 휴먼 에러로 누락될 가능성도 있다고 생각해서 구구님 의견처럼 유스케이스에서 처리하는게 좋지않을까 싶어요

@baekteun
Copy link
Member Author

클린 아키텍쳐 입장에서 보면 이게 맞긴한데, AnalyticsLogManager.setUserID(userID: nil) 가 휴먼 에러로 누락될 가능성도 있다고 생각해서 구구님 의견처럼 유스케이스에서 처리하는게 좋지않을까 싶어요

아 길걷다가 생각났는데 PreferencesManager에 clear()같은 메서드를 하나 만들어서 userInfo에 nil 할당하고 Analytics userID도 nil로 할당하게하는 방안은 어떤거같나요?
호출시점은 localDataSource안에 logout()이 실행되는 시점에 호출될거같네요

@youn9k @KangTaeHoon

@KangTaeHoon
Copy link
Contributor

그러면 로그아웃()에서 실행하는데 직접이냐 간접이냐 차이가 될라나요

@baekteun
Copy link
Member Author

그러면 로그아웃()에서 실행하는데 직접이냐 간접이냐 차이가 될라나요

네네 set userID를 직접 수행하는 레이어가 달라져요

@youn9k
Copy link
Member

youn9k commented Apr 22, 2024

클린 아키텍쳐 입장에서 보면 이게 맞긴한데, AnalyticsLogManager.setUserID(userID: nil) 가 휴먼 에러로 누락될 가능성도 있다고 생각해서 구구님 의견처럼 유스케이스에서 처리하는게 좋지않을까 싶어요

아 길걷다가 생각났는데 PreferencesManager에 clear()같은 메서드를 하나 만들어서 userInfo에 nil 할당하고 Analytics userID도 nil로 할당하게하는 방안은 어떤거같나요? 호출시점은 localDataSource안에 logout()이 실행되는 시점에 호출될거같네요

@youn9k @KangTaeHoon

좋습니다! PreferenceManager랑 AnalyticsLogManager 둘다 같은 매니저 클래스니 같은 뎁스가 될테니 좋은 방안이네요

+현재는 유틸리티모듈, 애널리틱스매니저 모듈로 나뉘어져있어 같은 ManagerModule 모듈로 합치거나 아예 전부 분리하거나? 해야할 것 같아요

@baekteun baekteun force-pushed the 496-preferences-manager-userinfo branch from 77aa9f6 to 7d59254 Compare April 22, 2024 14:07
@baekteun
Copy link
Member Author

좋습니다! PreferenceManager랑 AnalyticsLogManager 둘다 같은 매니저 클래스니 같은 뎁스가 될테니 좋은 방안이네요

+현재는 유틸리티모듈, 애널리틱스매니저 모듈로 나뉘어져있어 같은 ManagerModule 모듈로 합치거나 아예 전부 분리하거나? 해야할 것 같아요

7d59254

위 논의했던 방식으로 변경했어요 의견 감사해요 👍

@KangTaeHoon @youn9k

@KangTaeHoon
Copy link
Contributor

좋습니다! PreferenceManager랑 AnalyticsLogManager 둘다 같은 매니저 클래스니 같은 뎁스가 될테니 좋은 방안이네요
+현재는 유틸리티모듈, 애널리틱스매니저 모듈로 나뉘어져있어 같은 ManagerModule 모듈로 합치거나 아예 전부 분리하거나? 해야할 것 같아요

7d59254

위 논의했던 방식으로 변경했어요 의견 감사해요 👍

@KangTaeHoon @youn9k

별건아닌데, PreferenceManager에는 userInfo만 있는것이 아니니 clear() > clearUserInfo()로 변경 함이 어떨까합니다.

@baekteun
Copy link
Member Author

별건아닌데, PreferenceManager에는 userInfo만 있는것이 아니니 clear() > clearUserInfo()로 변경 함이 어떨까합니다.

아 놓쳤었네요 감사해요!
04d2433

@baekteun baekteun force-pushed the 496-preferences-manager-userinfo branch from 04d2433 to fc7b921 Compare April 23, 2024 12:59
Base automatically changed from 496-add-logging-manager to develop April 23, 2024 14:36
@baekteun baekteun merged commit a7b6d4e into develop Apr 23, 2024
3 checks passed
@baekteun baekteun deleted the 496-preferences-manager-userinfo branch April 23, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1️⃣ Priority: High 우선순위 상 ⚙ Setting 개발 환경 세팅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

로깅용 매니저 추가
3 participants