feat(data-exploration): person and cohort tables #13136
Conversation
|
Actually ready for a look! |
There was a problem hiding this comment.
Great! Only one real issue that I ran into: The delete button for persons in cohorts does not show up for me.
Besides that some UX details regarding exports on the persons page, and a few inline comments. Nothing that needs to change, just observations:
-
The exported CSV for "selected columns" contains a
person.$deletecolumn, which is probably unnecessary. -
The exported CSV for "all columns" contains a
value_at_data_pointcolumn, which is probably unnecessary. -
Not related to this PR, but we might be able to improve this with data exploration - The exported CSV contains columns
idanddistinct_ids.0, but I can't filter for these columns (unclear what property I should use). -
The export button does not reflect the number of selected columns this any more:
Before
After
| @@ -193,7 +198,7 @@ export function CohortEdit({ id }: CohortLogicProps): JSX.Element { | |||
| </> | |||
| )} | |||
|
|
|||
| {!isNewCohort && ( | |||
| {typeof cohort.id === 'number' && ( | |||
There was a problem hiding this comment.
How does this differ from !isNewCohort? Might want to extract to a variable for better understanding.
There was a problem hiding this comment.
Unfortunately this is needed for typing below when passing the cohort id to the query :/
There was a problem hiding this comment.
I think a comment would be great in that case :)
|
Thanks for the review! Fixed all the feedback, I hope. Would love to get this in to help move along work on the two stacked PRs on top of this, plus the event table regressions...
Removed!
This is there also on the current live persons table export. I propose we deal with it later 🙃
Hopefully HogQL will help here
Fixed! |
| @@ -193,7 +198,7 @@ export function CohortEdit({ id }: CohortLogicProps): JSX.Element { | |||
| </> | |||
| )} | |||
|
|
|||
| {!isNewCohort && ( | |||
| {typeof cohort.id === 'number' && ( | |||
There was a problem hiding this comment.
I think a comment would be great in that case :)
Problem
Follow up to #13116
Changes
Replaces persons and cohort tables with data exploration versions.
I had to do some annoying refactors to get the delete button properly extracted, apologies for all the noise around this in the PR:
I also cleaned up the storybook query stories somewhat.
How did you test this code?
Clicking around in the interface. Hopefully soon with visual regression tests.