Translate: Log StringNotFound UX events for experimentation - #4454
Conversation
|
@eemeli I tried to copy the approach we have for the Notifications experiment. Let me know if there's anything to change with that context and your opinion on using that approach. |
| all_projects: allProjects, | ||
| same_project: sameProject, | ||
| same_resource: sameResource, | ||
| filtered_out: filteredOut, |
There was a problem hiding this comment.
The filteredOut value is derived solely from the previous ones, and leaves out the ... && entityLocation.filters.length > 0 that's also applied below.
There was a problem hiding this comment.
There was also a bug in the test (along with this oversight), commented below.
There was a problem hiding this comment.
Dropping filtered_out was the wrong fix here, as it's now not possible to reconstruct from the log data what was rendered.
The filteredOut value should be refactored to incorporate the condition that's included inline in the render method, and it should be included in the log data.
You should also assess whether it's desirable for the logged data to be a bag of booleans, or an enum. I'm not sure which; that depends on how we're intending to use this data.
There was a problem hiding this comment.
incorporate the condition that's included inline in the render method, and it should be included in the log data.
Refactored filteredOut to incorporate && entityLocation.filters.length > 0.
assess whether it's desirable for the logged data to be a bag of booleans, or an enum
Used an enum and added a panel constant. Currently have 3 × 2 = 6 click cells plus 3 render counts as denominators.
|
@eemeli, 2 other changes:
|
eemeli
left a comment
eemeli
left a comment
There was a problem hiding this comment.
See comment reply above.
For future reference, please do not resolve and close my review comments, unless you're using the GitHub website UI to merge a suggestion I've made in a review comment.
In particular for reviews that end up taking multiple rounds, it's much easier for me to assess if my previous feedback has been accounted for by having me resolve and close threads.
eemeli
left a comment
eemeli
left a comment
There was a problem hiding this comment.
Looks decent now; left one more minor question inline that might just need an answer, rather than any code changes.
|
|
||
| const logAction = (action: string) => { | ||
| if (isAuthUser) { | ||
| logUXAction(action, 'String Not Found 1.0', { panel }); |
There was a problem hiding this comment.
If the data is an enum-ish string value, why wrap it inside an object?
There was a problem hiding this comment.
To be honest it's because the only consumption is manual queries. This way just seems more self-describing to me in case we want to track this a lot later.
2 participants
Fix #4360
Log the 2 buttons for all 3 panels:
filters: the active filters the string doesn't matchproject: the project and resource the string lives inresource: only the resource, the string being in the current projectChanges: