BIDMCDigitalPsychiatry / LAMP-platform

The LAMP Platform (issues and documentation).
https://docs.lamp.digital/
Other
12 stars 10 forks source link

Mismatch in login events #626

Closed lukeoftheshire closed 2 years ago

lukeoftheshire commented 2 years ago

Events fire with different keys depending on whether you log-in from a phone or internet:

 {'data': {'action': 'login',
    'device_token': 'fK7crcaWR4KcCRMuzrS42F:APA91bFJnaFxNo-Im3lawubt7aQ4pAtqgdm1V6K22Ionvj-6Cizb19Ip_OYSGmaCqIG0sFjc-AeDl_6RK7Hrkb7ijpcK5DkuENyRuFIjZD_vN95_epK3GAIahycC1fT20IFa8o5L5S6w',
    'device_type': 'Android',
    'user_agent': 'NativeCore 2022.4.25; Android 10; OnePlus; HD1905'},
   'sensor': 'lamp.analytics',
   'timestamp': 1651158007854},
  {'timestamp': 1651158007808,
   'sensor': 'lamp.analytics',
   'data': {'type': 'login',
    'device_type': 'Dashboard',
    'user_agent': 'LAMP-dashboard/2022.4.23 Mozilla/5.0 (Linux; Android 10; HD1905 Build/QKQ1.190716.003; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/100.0.4896.127 Mobile Safari/537.36'}}]}

Notice that the login via the web fires with type:login, where the login via the app fires with action:login

These should be consistent across all ways to access LAMP - ~I'd favor changing both to action~ see my comment below - I actually now think type might be more appropriate

avaidyam commented 2 years ago

@lukeoftheshire We'll also have to make a list of these inconsistencies and eventually write a database-fixer script. We can't and shouldn't be handling everything in Cortex, that's bad practice.

sarithapillai8 commented 2 years ago

@lukeoftheshire @avaidyam https://github.com/BIDMCDigitalPsychiatry/LAMP-platform/issues/506#issuecomment-1046890343 @avaidyam It was asked to use 'type' key for this. We have all the sensor events(open_page) from dashboard with type key. So could you please confirm changing this?

lukeoftheshire commented 2 years ago

Just gonna chime in to say that we've refactored the code that led to us discovering the issue -- I actually now think it may be better to use type to minimize the number of keywords used across LAMP

jijopulikkottil commented 2 years ago

in iOS and Android, for all lamp.analytics, we are using following events as action "login" "notification" "logout" "log" "lowpowermode"

So should we update all as type rather than action ?

jijopulikkottil commented 2 years ago

@avaidyam Could you confirm my above comment ?

avaidyam commented 2 years ago

@sarithapillai8 @jijopulikkottil Let's stick to type as @lukeoftheshire says. Please replace action as well, but please make sure LAMP-worker is able to use either, for backwards compatibility reasons.

Linoy339 commented 2 years ago

@avaidyam . We will be having some changes in LAMP-worker.

saranya-sajeev commented 2 years ago

@avaidyam Renamed action to type in Android & iOS and given for QA testing.

divyav2020 commented 2 years ago

We have updated in Staging

saranya-sajeev commented 2 years ago

@avaidyam We have pushed the android build to beta and it is in review status.

Tuna9129 commented 1 year ago

This seems to be fixed in staging!