Add trackings for polls #85

julioromano · 2023-08-25T12:18:28Z

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 element-hq/element-meta#2014

alfogrillo · 2023-08-28T14:17:47Z

schemas/PollCreated.json

+    "numberOfAnswers": {
+      "description": "Number of answers in the poll.",
+      "type": "integer"


To me "option" would be a better naming. To me an "answer" is picking a specific option.

In the matrix spec it is called "answer". In the UI it is called "option". Not sure what nomenclature the analytics usually follow.

Idk, staying aligned with the spec LGTM anyway 👍🏻

alfogrillo · 2023-08-28T14:18:42Z

schemas/PollCreated.json

+    "eventName": {
+      "enum": ["PollStarted"]
+    },
+    "isAnonymous": {


This is the disclosed/undisclosed things? Votes are never anonymous. 🤔

In the matrix spec it is called "disclosed"/"undisclosed". In the UI it is called "anonymous" true/false. Not sure what nomenclature the analytics usually follow.

BTW it is not the votes that are anonymous, it is the poll.

I think @alfogrillo wants to point out that the votes can be seen, so even if the UI does not tell who has chosen which option, it's not hard to see that information by inspecting content of (hidden) events in the timeline.

This is well known within the team.
Design/Product decided to use anonymous in the user visible copy though.
I don't know if analytics tend to follow the product wording or than the Matrix Spec wording. Do you have any guidance on this?

We should be consistent in deciding which naming convention we want to use.
To me using the spec one is more robust, since copies in the UI can change much more easily.

schemas/Composer.json

bmarty · 2023-08-29T07:47:36Z

schemas/MobileScreen.json

@@ -64,7 +64,8 @@
        {"const": "Invites", "description": "Room accessed via space bottom sheet list."},
        {"const": "CreateSpace", "description": "The screen shown to create a new space."},
        {"const": "LocationSend", "description": "The screen shown to share location."},
-        {"const": "LocationView", "description": "The screen shown to view a shared location."}
+        {"const": "LocationView", "description": "The screen shown to view a shared location."},
+        {"const": "CreatePollView", "description": "The screen shown to create or edit a poll."}


Just a remark: don't we want to do the diff between creating and editing?

Yes thanks, initially only poll creation will make the cut so a EditPollView will be added later.

bmarty · 2023-08-29T07:52:55Z

schemas/PollCreated.json

+    "eventName": {
+      "enum": ["PollStarted"]
+    },
+    "isAnonymous": {


I think @alfogrillo wants to point out that the votes can be seen, so even if the UI does not tell who has chosen which option, it's not hard to see that information by inspecting content of (hidden) events in the timeline.

pixlwave

Oh, this is nice to see polls included in the analytics 🗳️

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.
The PR needs to include the generated files too, please run yarn and commit these :)

pixlwave · 2023-08-29T08:12:38Z

schemas/Composer.json

@@ -25,6 +25,10 @@
        "description": "Whether this message it's a shared location.",
        "type": "boolean"
    },
+    "isPoll": {


Does this property provide any value given there's a dedicated event for starting a poll?

I asked the same but @VolkerJunginger said they want the composer event as well, I'll post his answer here:

No, it is not redundant from an analytics point of view.
We need the composer Event to count "no of messages send" overall.
We need the poll event to get data for the poll

Ok, that makes sense, thinking out loud here: it feels like we're doing it wrong in that case. having isPoll, locationType as separate properties isn't very scalable. Because it sounds like we also want an isImage, isVideo etc if we want all messages sent.

Perhaps we should have a messageType enum here with initially, Text, Poll, LocationPin and LocationUser and then in the future this could be expanded to include other types of message content?

@VolkerJunginger does that sound reasonable to you (and would you have an issue if this caused any old events with the locationType property to be misaligned)?

Given what's been discussed in the analytics room I'll change this as suggested. Though it'll have to wait for me to be back from vacation (~1week).

Co-authored-by: Benoit Marty <[email protected]>

julioromano · 2023-08-29T08:50:10Z

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 · 2023-08-29T08:55:11Z

Included output of yarn build

pixlwave · 2023-08-29T09:14:11Z

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 · 2023-08-29T09:21:07Z

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 · 2023-08-29T11:49:54Z

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)

NB: This is missing analytics, which will be added once matrix-org/matrix-analytics-events#85 is merged. Closes element-hq/element-meta#2011

julioromano · 2023-08-29T21:21:21Z

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 · 2023-09-12T08:51:33Z

@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?

alfogrillo

LGTM

alfogrillo · 2023-09-12T09:21:49Z

schemas/PollEnd.json

+    "eventName": {
+      "enum": ["PollEnd"]
+    },
+    "doNotUse": {


Should this be removed before we merge?

See: #85 (comment)
We can merge it and then remove this once the script that generates the Kotlin code is fixed.

alfogrillo · 2023-09-12T09:21:55Z

schemas/PollVote.json

+    "eventName": {
+      "enum": ["PollVote"]
+    },
+    "doNotUse": {


Should this be removed before we merge?

Ditto as above.

pixlwave

Thanks, this looks cleaner to me now with the message type property 🙏

pixlwave · 2023-09-12T09:33:17Z

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.

julioromano self-assigned this Aug 25, 2023

julioromano mentioned this pull request Aug 25, 2023

[Story] Create poll element-hq/element-meta#1974

Closed

6 tasks

Add trackings for polls

4340860

julioromano force-pushed the julioromano/polls branch from 585cac7 to 4340860 Compare August 28, 2023 12:05

julioromano changed the title ~~[WIP] Add tracking for polls~~ Add trackings for polls Aug 28, 2023

julioromano marked this pull request as ready for review August 28, 2023 12:06

julioromano requested a review from a team as a code owner August 28, 2023 12:06

julioromano requested review from pixlwave, langleyd, alfogrillo and VolkerJunginger August 28, 2023 12:06

Restore composer event

2afd104

julioromano enabled auto-merge August 28, 2023 13:35

julioromano disabled auto-merge August 28, 2023 13:35

julioromano removed request for langleyd and VolkerJunginger August 28, 2023 13:37

alfogrillo reviewed Aug 28, 2023

View reviewed changes

bmarty reviewed Aug 29, 2023

View reviewed changes

pixlwave reviewed Aug 29, 2023

View reviewed changes

julioromano mentioned this pull request Aug 29, 2023

"Create poll" UI element-hq/element-x-android#1143

Merged

Update schemas/Composer.json

9ddbb8c

Co-authored-by: Benoit Marty <[email protected]>

Include output from yarn build

7e3a9e2

Marco Romano added 3 commits August 29, 2023 11:57

Remove or edit

c4afca1

add generated types

fdaddea

Refactor

03a0df7

julioromano force-pushed the julioromano/polls branch from ed7f0b4 to 03a0df7 Compare August 29, 2023 11:49

Put creat/edit into action prop (not eventName).

dc572b9

julioromano pushed a commit to element-hq/element-x-android that referenced this pull request Aug 29, 2023

"Create poll" UI (#1143)

633d528

NB: This is missing analytics, which will be added once matrix-org/matrix-analytics-events#85 is merged. Closes element-hq/element-meta#2011

Add dummy props to not crash kotlin type generation

922f841

julioromano force-pushed the julioromano/polls branch from 100e6cc to 922f841 Compare August 29, 2023 21:17

julioromano force-pushed the julioromano/polls branch from 0527cd1 to 80dcfeb Compare September 12, 2023 08:53

Switch away from isLocation isPoll etc. and over to messageType.

1ac859c

julioromano force-pushed the julioromano/polls branch from 80dcfeb to 1ac859c Compare September 12, 2023 08:56

alfogrillo approved these changes Sep 12, 2023

View reviewed changes

pixlwave approved these changes Sep 12, 2023

View reviewed changes

julioromano merged commit e9cd9ad into main Sep 12, 2023
3 checks passed

julioromano deleted the julioromano/polls branch September 12, 2023 09:35

julioromano mentioned this pull request Sep 12, 2023

[Tech task] Polls Analytics element-hq/element-meta#2014

Closed

alfogrillo mentioned this pull request Sep 12, 2023

Add polls analytics element-hq/element-x-ios#1691

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add trackings for polls #85

Add trackings for polls #85

julioromano commented Aug 25, 2023 •

edited

Loading

alfogrillo Aug 28, 2023

julioromano Aug 28, 2023 •

edited

Loading

alfogrillo Aug 29, 2023

alfogrillo Aug 28, 2023

julioromano Aug 28, 2023 •

edited

Loading

bmarty Aug 29, 2023

julioromano Aug 29, 2023

alfogrillo Aug 29, 2023

bmarty Aug 29, 2023

julioromano Aug 29, 2023

bmarty Aug 29, 2023

pixlwave left a comment

pixlwave Aug 29, 2023

julioromano Aug 29, 2023

pixlwave Aug 29, 2023 •

edited

Loading

julioromano Aug 31, 2023

julioromano commented Aug 29, 2023

julioromano commented Aug 29, 2023

pixlwave commented Aug 29, 2023

pixlwave commented Aug 29, 2023

julioromano commented Aug 29, 2023

julioromano commented Aug 29, 2023

julioromano commented Sep 12, 2023

alfogrillo left a comment

alfogrillo Sep 12, 2023

julioromano Sep 12, 2023

alfogrillo Sep 12, 2023

julioromano Sep 12, 2023

pixlwave left a comment

pixlwave commented Sep 12, 2023

Add trackings for polls #85

Add trackings for polls #85

Conversation

julioromano commented Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

julioromano Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julioromano Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pixlwave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pixlwave Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julioromano commented Aug 29, 2023

julioromano commented Aug 29, 2023

pixlwave commented Aug 29, 2023

pixlwave commented Aug 29, 2023

julioromano commented Aug 29, 2023

julioromano commented Aug 29, 2023

julioromano commented Sep 12, 2023

alfogrillo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pixlwave left a comment

Choose a reason for hiding this comment

pixlwave commented Sep 12, 2023

julioromano commented Aug 25, 2023 •

edited

Loading

julioromano Aug 28, 2023 •

edited

Loading

julioromano Aug 28, 2023 •

edited

Loading

pixlwave Aug 29, 2023 •

edited

Loading