Conversation
There was a problem hiding this comment.
eek must have forgotten to turn this into a constant. good find!
There was a problem hiding this comment.
what is the purpose of removing the confirmation_for_phone_change key here? based on the method name it looks like the hash is built specifically for analytics, so it is confusing as a reader to me to remove it?
There was a problem hiding this comment.
On second thought, you are right. I thought we wouldn't be interested in that property for a page visit, but I think we might, and it doesn't hurt to have extra properties. I'll revert this.
There was a problem hiding this comment.
should confirmation be a constant?
There was a problem hiding this comment.
nice! this method name is V clear 🐃
There was a problem hiding this comment.
seeing this string in the specs makes me think that my earlier comment (re: making this a constant) may be correct. I can imagine a typo in the word creating a confusingly failing spec.
There was a problem hiding this comment.
I'm not opposed, but can we do that in a separate PR since this wasn't introduced here?
monfresh
force-pushed
the
mb-cleanup-analytics
branch
2 times, most recently
from
6f54798 to
f174ab5
Compare
**Why**: To better capture phone confirmation events **How**: - Remove the 'User confirmed their phone number' event because that can be queried via the properties of the MULTI_FACTOR_AUTH event. - Track successful phone changes by adding a new property in the MULTI_FACTOR_AUTH event, as opposed to having a separate event
monfresh
force-pushed
the
mb-cleanup-analytics
branch
from
f174ab5 to
a0f2a31
Compare
**Why**: To better capture phone confirmation events **How**: - Remove the 'User confirmed their phone number' event because that can be queried via the properties of the MULTI_FACTOR_AUTH event. - Track successful phone changes by adding a new property in the MULTI_FACTOR_AUTH event, as opposed to having a separate event
**Why**: To better capture phone confirmation events **How**: - Remove the 'User confirmed their phone number' event because that can be queried via the properties of the MULTI_FACTOR_AUTH event. - Track successful phone changes by adding a new property in the MULTI_FACTOR_AUTH event, as opposed to having a separate event
3 participants
Why: To better capture phone confirmation events
How:
be queried via the properties of the MULTI_FACTOR_AUTH event.
MULTI_FACTOR_AUTH event, as opposed to having a separate event