[iOS] Analytics Screen
Closed this issue · 14 comments
Description
On Beagle 1.10.1, the implementation for the analytics screen record is returning the remote url of the screen instead of the view controller identifier. This implementation is different from android where beagle is returning the identifier.
Steps To Reproduce
-
Create a remote screen. Example: BeagleScreenViewController(.remote(.init(url: "http://localhost:8080/timeline")))
-
Set the analyticsProvider on Beagle.dependencies.analyticsProvider
-
Create record will be called with the screen variable as the remote url, instead of the screen identifier.
Expected Results
Create record should be called with the screen identifier on the screen variable.
Code example, screenshot, or link to a repository:
func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
let dependencies = BeagleDependencies()
dependencies.analyticsProvider = Analytics()
dependencies.urlBuilder = UrlBuilder(baseUrl: baseURL)
dependencies.networkClient = Network()
Beagle.dependencies = dependencies
let beagleController = BeagleScreenViewController(.remote(.init(url: "http://localhost:8080/timeline")))
window = UIWindow(frame: UIScreen.main.bounds)
window?.rootViewController = beagleController
window?.backgroundColor = UIColor.white
window?.makeKeyAndVisible()
return true
}
class Analytics: AnalyticsProvider {
func createRecord(_ record: AnalyticsRecord) {
switch record.type {
case .action(let action):
return
case .screen:
if let screenName = record.screen {
// REMOTE URL COMES AS THE SCREEN NAME
}
@unknown default:
return
}
}
}👋 @mateusforgi
Thank you for raising an issue. We will investigate into the matter and get back to you as soon as possible.
Please make sure you have given us as much context as possible and that you have followed our contributing guidelines.
We will review it as soon as possible.
Hi @mateusforgi,
Thank you for contributing with Beagle and for the thorough explanation on the issue. We will be on that asap.
this is a fact that could be discussed with the beagle core and explained better in the documentation since there are scenarios that use the identifier and others the url, it would be better to define a pattern and both platforms follow
scenarios that all this gets complicated and it's good to be discussed: if you use in the analytics provider the identifier of the view and not the URL, when I use the navigation should it be the URL or identifier to go back to the back screen?
What is best for the end developer? it would be nice to study and adjust both platforms
The correct behavior of this features is:
- Check whether the route is local or remote.
- If it's local, then use
route.screen.identifieras the route. - Otherwise, use
route.urlas the route.
I just checked the behavior of Web and Flutter and they're both implemented just like iOS. Android is actually the one with the unexpected behavior.
If the screen id is important to you, we can add a new field to the ScreenRecord named screenId. screenId would always have the value of screen.identifier if there is a screen with an id at the root node, otherwise, the key wouldn't exist in the record.
@mateusforgi Is it important for you to have the screen.identifier in the ScreenRecord if it exists?
@uziassantosferreira the screen identifier would be the best solution for us. I am afraid that returning the remote URL can be described as a vulnerability as some mobile apps use encrypted URLs to avoid leaking the real URL.
@Tiagoperes the screenId would solve the issue for us, as it would give more control to our backend do the analytics without the need of a map from URL to page name on the frontend. However, I think it should be considered the fact that returning the URL might be a vulnerability.
@mateusforgi if the URL is encrypted in the backend and decrypted in your HttpClient, this won't be a problem, because only the encrypted URL would be exposed.
In any case, you shouldn't base any security in the URL, no matter how much you try to hide it, it will always be visible for anyone inspecting the connection. For instance, any Android rooted device can see all the network traffic, just like you can check the network tab in a web browser.
@Tiagoperes When the navigation happens through a Beagle action it exposes the path not encrypted, right? I haven't done this test yet, but I noticed beagle only returns the path as the remote URL in that case to analytics.
Anyway, can you give a direction if the screen id or the screen identifier will be returned to analytics? We will move forward in case it is not possible.
@mateusforgi I suggest we follow up with Tiago's idea, I have changed our web core to show you what the final payload would look like:
What's new:
- Added
rootIdproperty to screen records. This property will return the value of the id of the current screen if it exists, otherwise it won't be added.
Does it cover the case for your? If so, I will add it to our backlog to be worked on all platforms.
@hectorcustodiozup the rootId solves the issue! We really appreciate it if you guys can add this field to the next release.
We are going to add this in the next release @mateusforgi.
If there's any field in the final record you don't want to use, remember it goes through your analytics provider method createRecord. You can just remove the fields you don't want before sending it to the backend.
@carlossteinzup @hectorcustodiozup can you guys implement this feature in iOS and Android?
@Tiagoperes Awesome! Thank you!
@Tiagoperes @mateusforgi. Last week I could not work on this, but this one I will try to finish Web and add the changes for both iOS and Android.