Analytics for device forecast details entry point #214
Conversation
WalkthroughThis pull request focuses on modifying analytics tracking across several UI components in the PresentationLayer. The changes primarily involve removing existing screen tracking calls from various views and introducing a more centralized approach to tracking screen view events in the Changes
Sequence DiagramsequenceDiagram
participant View as StationDetailsContainerView
participant ViewModel as StationDetailsViewModel
participant Analytics as WXMAnalytics
View->>ViewModel: trackScreenViewEvent(for: index)
ViewModel->>Analytics: trackScreen(screen)
Analytics-->>ViewModel: Screen tracked
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wxm-ios/Toolkit/Toolkit/Analytics/WXMAnalytics.swift (1)
63-70: Consider moving the test extension to a test-specific file.The
testStringextension is only used for debugging purposes. Consider moving it to a separate test utilities file to keep the production code clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
PresentationLayer/UIComponents/Screens/LoggedInViews/LoggedInTabViewContainer.swift(0 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Forecast/StationForecastView.swift(0 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift(0 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Rewards/StationRewardsView.swift(0 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/StationDetailsContainerView.swift(2 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/StationDetailsViewModel.swift(2 hunks)wxm-ios/Toolkit/Toolkit/Analytics/WXMAnalytics.swift(2 hunks)
💤 Files with no reviewable changes (4)
- PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Rewards/StationRewardsView.swift
- PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Forecast/StationForecastView.swift
- PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift
- PresentationLayer/UIComponents/Screens/LoggedInViews/LoggedInTabViewContainer.swift
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WeatherXM | Unit tests | Test - iOS
🔇 Additional comments (5)
wxm-ios/Toolkit/Toolkit/Analytics/WXMAnalytics.swift (1)
33-33: Remove debug print statements after testing.These print statements are marked as temporary and should be removed before merging to production.
Also applies to: 38-39
✅ Verification successful
Remove temporary debug print statements in WXMAnalytics.swift
The following temporary debug prints should be removed:
- Line 33:
print("WXMAnalytics screen: \(screen.rawValue)")- Line 38:
print("WXMAnalytics event: \(event.rawValue) parameters: \(parameters?.testString ?? "")")Other print statements in RemoteConfigManager.swift and FirebaseManager.swift are legitimate logging statements and should be retained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other debug prints that might need cleanup rg -l 'print.*TEMP for testing'Length of output: 86
Script:
#!/bin/bash # Check for various debug print patterns echo "=== Checking for print statements ===" rg -l "print\(" 'wxm-ios/Toolkit' echo -e "\n=== Checking for debug comments ===" rg -l "// *(?i)(DEBUG|TEMP|TEMPORARY|TODO|FIXME)" 'wxm-ios/Toolkit'Length of output: 507
Script:
#!/bin/bash echo "=== WXMAnalytics.swift prints ===" rg "print\(" wxm-ios/Toolkit/Toolkit/Analytics/WXMAnalytics.swift -A 1 echo -e "\n=== RemoteConfigManager.swift prints ===" rg "print\(" wxm-ios/Toolkit/Toolkit/FirebaseManager/RemoteConfigManager.swift -A 1 echo -e "\n=== FirebaseManager.swift prints ===" rg "print\(" wxm-ios/Toolkit/Toolkit/FirebaseManager/FirebaseManager.swift -A 1Length of output: 1405
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/StationDetailsContainerView.swift (2)
178-183: LGTM! Robust implementation of screen tracking.The screen tracking is now properly handled when tabs change, fixing the original issue with force swipes.
196-198: LGTM! Initial screen view tracking.Good addition to track the initial screen view when the view appears.
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/StationDetailsViewModel.swift (2)
143-149: LGTM! Safe implementation of screen tracking.The method safely handles index bounds and delegates to the analytics service.
175-184: Consider adding a test for the analytics screen mapping.The mapping between tabs and analytics screens is critical for accurate tracking. Consider adding unit tests to verify this mapping.
✅ Verification successful
The review comment is valid - no tests found for analytics screen mapping
The codebase search confirms there are no existing tests for the StationDetailsViewModel or its analytics screen mapping. Adding tests would help ensure the reliability of analytics tracking.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if tests exist for the Tab enum rg -l "TestStationDetailsViewModel.*Tab"Length of output: 42
Script:
#!/bin/bash # Search for StationDetailsViewModel tests with various patterns echo "=== Searching for StationDetailsViewModel tests ===" rg -l "StationDetailsViewModel.*Test|Test.*StationDetailsViewModel" echo -e "\n=== Searching for Screen enum usage in tests ===" rg -l "Screen.*case|enum.*Screen" --type swift "*Test*.swift" echo -e "\n=== Searching for analytics-related test files ===" rg -l "analytics|Analytics" "*Test*.swift"Length of output: 627
Script:
#!/bin/bash # Find all Swift test files echo "=== Finding Swift test files ===" fd -e swift -g "*Test*.swift" echo -e "\n=== Searching for StationDetailsViewModel in test files ===" fd -e swift -g "*Test*.swift" -x rg -l "StationDetailsViewModel" echo -e "\n=== Searching for Screen enum and analytics in test files ===" fd -e swift -g "*Test*.swift" -x rg -l "Screen|analytics|Analytics"Length of output: 772
pantelisss
deleted the
feature/fe-1529-analytics-for-device-forecast-details-entry-point
branch
Why?
Fix the inconsistency of screen tracking in station details.
The issue occurred when the user swipes with more "power" from the first tab (Overview) to the second (Forecast). So because of the scroller bounce effect, a small part of the third tab (Rewards) appears and triggers the
onAppearcallbackHow?
Now we track the screen event from the container (
StationDetailsContainerView) on everyselectedIndexchangeTesting
There are some debug prints in
WXMAnalyticsfile which shows in the console every screen and event track.Just filter the console output with
WXMAnalytics.This code will be removed once the review is over
Additional context
fixes fe-1529
Summary by CodeRabbit
Analytics
StationDetailsViewModelto handle screen view tracking more dynamicallyRefactor