Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add trackings for polls #85

Merged
merged 10 commits into from
Sep 12, 2023
Merged

Add trackings for polls #85

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4340860
Add trackings for polls
Aug 25, 2023
2afd104
Restore composer event
Aug 28, 2023
9ddbb8c
Update schemas/Composer.json
julioromano Aug 29, 2023
7e3a9e2
Include output from `yarn build`
Aug 29, 2023
c4afca1
Remove `or edit`
Aug 29, 2023
fdaddea
add generated types
Aug 29, 2023
03a0df7
Refactor
Aug 29, 2023
dc572b9
Put creat/edit into action prop (not eventName).
Aug 29, 2023
922f841
Add dummy props to not crash kotlin type generation
Aug 29, 2023
1ac859c
Switch away from `isLocation` `isPoll` etc. and over to `messageType`.
Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions schemas/Composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
"description": "Whether this message it's a shared location.",
"type": "boolean"
},
"isPoll": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@pixlwave pixlwave Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

"description": "Whether this message iss a poll.",
julioromano marked this conversation as resolved.
Show resolved Hide resolved
"type": "boolean"
},
"locationType": {
"description": "The type of the shared location",
"type": "string",
Expand Down
3 changes: 2 additions & 1 deletion schemas/MobileScreen.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

]
},
"durationMs": {
Expand Down
11 changes: 11 additions & 0 deletions schemas/PollAnswered.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"type": "object",
"description": "Triggered when a poll is answered.",
"properties": {
"eventName": {
"enum": ["PollAnswered"]
}
},
"required": ["eventName"],
"additionalProperties": false
}
19 changes: 19 additions & 0 deletions schemas/PollCreated.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"type": "object",
"description": "Triggered when a poll is created.",
"properties": {
"eventName": {
"enum": ["PollStarted"]
},
"isAnonymous": {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@julioromano julioromano Aug 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"description": "Whether this poll is anonymous.",
"type": "boolean"
},
"numberOfAnswers": {
"description": "Number of answers in the poll.",
"type": "integer"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@julioromano julioromano Aug 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, staying aligned with the spec LGTM anyway 👍🏻

}
},
"required": ["numberOfAnswers", "isAnonymous", "eventName"],
"additionalProperties": false
}
19 changes: 19 additions & 0 deletions schemas/PollEdited.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"type": "object",
"description": "Triggered when a poll is edited.",
"properties": {
"eventName": {
"enum": ["PollEdited"]
},
"isAnonymous": {
"description": "Whether this poll is anonymous.",
"type": "boolean"
},
"numberOfAnswers": {
"description": "Number of answers in the poll.",
"type": "integer"
}
},
"required": ["numberOfAnswers", "isAnonymous", "eventName"],
"additionalProperties": false
}
11 changes: 11 additions & 0 deletions schemas/PollEnded.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"type": "object",
"description": "Triggered when a poll is ended.",
"properties": {
"eventName": {
"enum": ["PollEnded"]
}
},
"required": ["eventName"],
"additionalProperties": false
}
Loading