-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] Firebase Eventlogger #329
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
Conversation
c141806 to
6578c60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces Firebase Event Logging functionality to the EatSSU Android app. The implementation creates a centralized EventLogger for tracking user interactions and refactors existing enum classes into a shared core:common module.
- Implements Firebase Analytics integration with comprehensive event tracking throughout the app
- Creates a new
core:commonmodule to house shared enums and the EventLogger - Refactors existing enum classes from
app/data/enumstocore:common/enumsfor better code organization
Reviewed Changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| core/common/src/main/java/com/eatssu/common/EventLogger.kt | Central Firebase Analytics event logging implementation |
| core/common/src/main/java/com/eatssu/common/enums/* | Relocated enum classes (Restaurant, Time, MenuType, etc.) from app module |
| Multiple app files | Updated import statements to reference relocated enums from core:common |
| Widget-related files | Enhanced widget functionality to pass Restaurant enum and log events |
| UI components | Added event logging calls throughout cafeteria, map, and review flows |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| likes: Long, | ||
| photoAttached: Boolean, | ||
| ) { | ||
| firebaseAnalytics.logEvent("complete_review_v1") { |
Copilot
AI
Sep 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event name should be 'complete_review_v2' for the completeReviewV2 function, not 'complete_review_v1'.
| firebaseAnalytics.logEvent("complete_review_v1") { | |
| firebaseAnalytics.logEvent("complete_review_v2") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇ코파일럿 코멘트처럼 추후에 completeReviewV2 함수에서 왜 complete_review_v1를 호출하는지 헷갈릴 수도 있을 것 같아요.
백엔드와 협업해서 complete_review 이름 자체를 공통되게 하나로 변경하거나, 이름을 v1 v2 나뉘게 해야 할 것 같아요!
| class | ||
| VariableMenuPickAdapter(private val menuList: List<MenuMini>?) : |
Copilot
AI
Sep 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class declaration is unnecessarily split across two lines. This should be on a single line for better readability.
| class | |
| VariableMenuPickAdapter(private val menuList: List<MenuMini>?) : | |
| class VariableMenuPickAdapter(private val menuList: List<MenuMini>?) : |
| likes: Long, | ||
| photoAttached: Boolean, | ||
| ) { | ||
| firebaseAnalytics.logEvent("complete_review_v1") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇ코파일럿 코멘트처럼 추후에 completeReviewV2 함수에서 왜 complete_review_v1를 호출하는지 헷갈릴 수도 있을 것 같아요.
백엔드와 협업해서 complete_review 이름 자체를 공통되게 하나로 변경하거나, 이름을 v1 v2 나뉘게 해야 할 것 같아요!
app/src/main/java/com/eatssu/android/presentation/login/IntroActivity.kt
Show resolved
Hide resolved
PeraSite
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니당!!
kangyuri1114
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오타가 있는 것 같아 리뷰 남깁니다..!
| FilterType.All -> viewModel.loadPartnerships() | ||
| FilterType.All -> { | ||
| viewModel.loadPartnerships() | ||
| EventLogger.clickMap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 이벤트가 "‘지도’ 클릭 - 제휴지도 이용 빈도"으로 노션에 써져있던데
토글 선택 시에만 이벤트가 수집되는 것 같아서요
바텀 바에서 지도 클릭 시에도 디폴트가 전체 제휴 지도 화면이 보여지니까 포함되어야 하지 않을까요??
아니면 그냥 '내제휴 / 전체제휴' 이용 빈도를 보기 위한 이벤트 일까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 결과, 토글 클릭 때에만 수집되지 않고, 바텀네비로 진입시에도 click_map이 호출됩니당~!
Summary
파이어베이스의 event_log를 기록합니다
Describe your changes
https://www.notion.so/eat-ssu/1d2eeef75a16813ca490db6ddf528c26?source=copy_link
위 노션 문서에서 정의한 Firebase Event Logging에 대한 작업을 하였습니다.
참고자료
https://kwongdevelop.tistory.com/60
todaywhat/TodayWhat-Aos#67
Issue
To reviewers
순수 이번 작업에 해당하는 커밋은 하기와 같습니다.