Add trackings for polls

matrix-org / matrix-analytics-events

Cross-platform definitions of analytics events raised by matrix SDKs

Apache License 2.0

8 stars 7 forks source link

Add trackings for polls #85

Closed julioromano closed 1 year ago

julioromano commented 1 year ago

ADDS: analytics for polls
BREAKING: refactors Composer analytics by condensing several message type boolean properties into a single "messageType" enum. This is a breaking change.

Closes https://github.com/vector-im/element-meta/issues/2014

julioromano commented 1 year ago

I wonder, do we really need poll created/edited/ended as 3 events? The guidelines suggest fewer events are better. We could e.g. have a PollManagement event with an action enum of Created, Edited, Ended.

@langleyd suggested to mimic the existing Call{Ended|Started} events. But if there are better ways I'm open to them.

Thing is the properties are not the same among the three events, so if we merge them into one most of the props won't be able to be "required" anymore, maybe this could be a problem data wise.

julioromano commented 1 year ago

Included output of yarn build

pixlwave commented 1 year ago

Included output of yarn build

I don't see any output in the swift or kotlin2 directories here? Possibly a python/poetry config issue?

pixlwave commented 1 year ago

Thing is the properties are not the same among the three events, so if we merge them into one most of the props won't be able to be "required" anymore, maybe this could be a problem data wise.

Ok fair, I missed that PollEnded wasn't the same here.

Thought: if we're interesting is knowing e.g. the average number of answers/options in a poll, perhaps having those on ended would be useful anyway? E.g. You can't use the number from PollStart because it could be edited, and you can't correlate PollStart to PollEdited events. Whereas if you took the number from PollEnded you know that is correct. Same could be said for the isAnonymous property if that's editable in the spec.

julioromano commented 1 year ago

A few changes for the better:

Separate MobileScreens have been added for create/edit.
PollCreated/PollEdited have been merged and renamed into PollCreation (they are very similar so one single event could cover both).
PollEnded -> PollEnd
PollAnswered -> PollVote (in the spec "answer" refers to the options of the poll, not to the act of voting. That is called "response" in the spec but it can be confused with "replying" so I opted to use "vote" which is also a term frequently used in the spec.)
isAnonymous -> isUndisclosed (to stick with spec terminology)

julioromano commented 1 year ago

Since the python script will produce wrong code when there are 0 properties other than the event name, I had to add some dummy props to work around it. It would be great to fix the script instead but we're rushing for a release so there won't be enough time.

julioromano commented 1 year ago

@pixlwave here's the proposed changes following your suggestion at: https://github.com/matrix-org/matrix-analytics-events/pull/85/files#r1311509508

I'm not sure whether startsThread should also be among the required properties. What do you think?

pixlwave commented 1 year ago

I'm not sure whether startsThread should also be among the required properties. What do you think?

Given we don't support threads in the context of EX, I wouldn't worry about it here.