search

wordpress-mobile / WordPress-Login-Flow-Android

Pluggable WordPress login flow for Android
GNU General Public License v2.0
14 stars 3 forks source link

Fix tracks connected info property naming #26

Closed AmandaRiu closed 4 years ago

AmandaRiu commented 4 years ago

This PR merges the changes defined in this WCAndroid PR. The changes are minor, I just needed to fix the property names of the _login_connected_site_info_succeeded event so the properties would get properly saved to the event.

This Draft PR can be used for testing these changes in WPAndroid.

malinajirka commented 4 years ago

Thanks @AmandaRiu! The changes look good. My only concerns is if it's a good idea to rename the properties. In general changing names of events or properties isn't a good idea as it will break already existing funnels/queries. This is especially true when the events are related to login/signup/sitecreation as we have some cross-platform funnels which we regularly check. I'm not saying we shouldn't make this change, I just wanted to raise this point. I want to be 100% this change is safe to do. Wdyt?

malinajirka commented 4 years ago

ok, just ignore my comment:D. I forgot you actually checked the previous properties were being completely ignored. I'll double check the subtree and merge this PR ;).

AmandaRiu commented 4 years ago

thanks @malinajirka !! 💃