-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
feat: recordings ordering #24794
feat: recordings ordering #24794
Changes from 41 commits
fd6ea28
5183a11
77a7ceb
cc482df
108d492
91653a7
874d3cc
f34c534
6e92bf6
10c4a74
841d907
e5f62bb
635ef6f
dd6baf2
5629032
971b590
e79b621
16c9a56
93c56af
347e0ea
43c5e2b
baea08a
a9ff478
2391d38
0f25271
04cc08b
34dcb3c
434c5b3
6965f88
3b363aa
66e3c98
03ccf64
947bbfb
45cb534
0ff53f8
f2b86a9
7560042
cc32efd
d885e8a
288488e
437d1e0
b8cfa22
dbd6b65
8287d70
5fa2c25
b8dae38
bf04127
bcf3aee
1971815
c0db65b
c920e67
4c4470b
2452f83
b637a33
aabb1eb
ed64528
47a93f1
d79ae5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
import { LemonSwitch } from '@posthog/lemon-ui' | ||
import { LemonSelect, LemonSwitch } from '@posthog/lemon-ui' | ||
import { useActions, useValues } from 'kea' | ||
import { DurationTypeSelect } from 'scenes/session-recordings/filters/DurationTypeSelect' | ||
|
||
import { playerSettingsLogic } from '../player/playerSettingsLogic' | ||
import { sessionRecordingsPlaylistLogic } from './sessionRecordingsPlaylistLogic' | ||
|
||
export function SessionRecordingsPlaylistSettings(): JSX.Element { | ||
const { durationTypeToShow, hideViewedRecordings } = useValues(playerSettingsLogic) | ||
const { setDurationTypeToShow, setHideViewedRecordings } = useActions(playerSettingsLogic) | ||
const { orderBy } = useValues(sessionRecordingsPlaylistLogic) | ||
const { hideViewedRecordings } = useValues(playerSettingsLogic) | ||
const { setHideViewedRecordings } = useActions(playerSettingsLogic) | ||
const { orderBy, randomSample } = useValues(sessionRecordingsPlaylistLogic) | ||
const { setOrderBy, setRandomSample } = useActions(sessionRecordingsPlaylistLogic) | ||
|
||
return ( | ||
<div className="relative flex flex-col gap-2 p-3 border-b"> | ||
|
@@ -20,16 +20,71 @@ export function SessionRecordingsPlaylistSettings(): JSX.Element { | |
onChange={() => setHideViewedRecordings(!hideViewedRecordings)} | ||
/> | ||
</div> | ||
{orderBy === 'start_time' && ( | ||
<div className="flex flex-row items-center justify-between space-x-2"> | ||
<span className="text-black font-medium">Show</span> | ||
<DurationTypeSelect | ||
value={durationTypeToShow} | ||
onChange={(value) => setDurationTypeToShow(value)} | ||
onChangeEventDescription="session recording list duration type to show selected" | ||
/> | ||
</div> | ||
)} | ||
<div className="flex flex-row items-center justify-between space-x-2"> | ||
<span className="text-black font-medium">Random sample</span> | ||
<LemonSwitch | ||
aria-label="Random sample" | ||
checked={randomSample} | ||
onChange={() => setRandomSample(!randomSample)} | ||
tooltip="Chooses a random sample of recordings matching the filters" | ||
/> | ||
</div> | ||
<div className="flex flex-row items-center justify-between space-x-2"> | ||
<span className="text-black font-medium">Order by</span> | ||
<LemonSelect | ||
options={[ | ||
{ | ||
value: 'start_time', | ||
label: 'Latest', | ||
}, | ||
{ | ||
label: 'Longest', | ||
options: [ | ||
{ | ||
value: 'duration', | ||
label: 'Total duration', | ||
}, | ||
{ | ||
value: 'active_seconds', | ||
label: 'Active duration', | ||
}, | ||
{ | ||
value: 'inactive_seconds', | ||
label: 'Inactive duration', | ||
}, | ||
], | ||
}, | ||
{ | ||
label: 'Most active', | ||
options: [ | ||
{ | ||
value: 'click_count', | ||
label: 'Clicks', | ||
}, | ||
{ | ||
value: 'keypress_count', | ||
label: 'Key presses', | ||
}, | ||
{ | ||
value: 'mouse_activity_count', | ||
label: 'Mouse activity', | ||
}, | ||
], | ||
}, | ||
{ | ||
value: 'console_error_count', | ||
label: 'Most errors', | ||
}, | ||
{ | ||
value: 'random_sample', | ||
label: 'Random sample', | ||
}, | ||
]} | ||
size="small" | ||
value={orderBy as string} | ||
onChange={setOrderBy} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could probably avoid this cast by properly typing the options but they're nested and i'm lazy 🤣 |
||
/> | ||
</div> | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,28 @@ import { | |
|
||
describe('sessionRecordingsPlaylistLogic', () => { | ||
let logic: ReturnType<typeof sessionRecordingsPlaylistLogic.build> | ||
const aRecording = { id: 'abc', viewed: false, recording_duration: 10, console_error_count: 50 } | ||
const bRecording = { id: 'def', viewed: false, recording_duration: 10, console_error_count: 100 } | ||
const aRecording = { | ||
id: 'abc', | ||
viewed: false, | ||
recording_duration: 10, | ||
start_time: '2023-10-12T16:55:36.404000Z', | ||
console_error_count: 50, | ||
} | ||
const bRecording = { | ||
id: 'def', | ||
viewed: false, | ||
recording_duration: 10, | ||
start_time: '2023-05-12T16:55:36.404000Z', | ||
console_error_count: 100, | ||
} | ||
const listOfSessionRecordings = [aRecording, bRecording] | ||
const offsetRecording = { | ||
id: `recording_offset_by_${listOfSessionRecordings.length}`, | ||
viewed: false, | ||
recording_duration: 10, | ||
start_time: '2023-08-12T16:55:36.404000Z', | ||
console_error_count: 75, | ||
} | ||
|
||
beforeEach(() => { | ||
useMocks({ | ||
|
@@ -54,7 +73,7 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
return [ | ||
200, | ||
{ | ||
results: [`List of recordings offset by ${listOfSessionRecordings.length}`], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
results: [offsetRecording], | ||
}, | ||
] | ||
} else if ( | ||
|
@@ -167,6 +186,11 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
}) | ||
|
||
describe('ordering', () => { | ||
afterEach(() => { | ||
logic.actions.setOrderBy('start_time') | ||
logic.actions.loadSessionRecordings() | ||
}) | ||
|
||
it('is set by setOrderBy, loads filtered results and orders the non pinned recordings', async () => { | ||
await expectLogic(logic, () => { | ||
logic.actions.setOrderBy('console_error_count') | ||
|
@@ -179,21 +203,22 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
expect(logic.values.otherRecordings.map((r) => r.console_error_count)).toEqual([100, 50]) | ||
}) | ||
|
||
it('adds an offset when not using latest ordering', async () => { | ||
it('adds an offset', async () => { | ||
await expectLogic(logic, () => { | ||
logic.actions.setOrderBy('console_error_count') | ||
logic.actions.loadSessionRecordings() | ||
}) | ||
.toDispatchActionsInAnyOrder(['loadSessionRecordingsSuccess']) | ||
.toDispatchActions(['loadSessionRecordingsSuccess']) | ||
.toMatchValues({ | ||
sessionRecordings: listOfSessionRecordings, | ||
}) | ||
|
||
await expectLogic(logic, () => { | ||
logic.actions.maybeLoadSessionRecordings('newer') | ||
logic.actions.loadSessionRecordings('older') | ||
}) | ||
.toDispatchActions(['loadSessionRecordingsSuccess']) | ||
.toMatchValues({ | ||
sessionRecordings: [...listOfSessionRecordings, 'List of recordings offset by 2'], | ||
// reorganises recordings based on start_time | ||
sessionRecordings: [aRecording, offsetRecording, bRecording], | ||
}) | ||
}) | ||
}) | ||
|
@@ -306,6 +331,7 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
expect(router.values.searchParams.filters).toHaveProperty('date_to', '2021-10-20') | ||
}) | ||
}) | ||
|
||
describe('duration filter', () => { | ||
it('is set by setFilters and fetches results from server and sets the url', async () => { | ||
await expectLogic(logic, () => { | ||
|
@@ -370,8 +396,9 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
.toFinishAllListeners() | ||
.toMatchValues({ | ||
sessionRecordingsResponse: { | ||
results: listOfSessionRecordings, | ||
order: 'start_time', | ||
has_next: undefined, | ||
results: listOfSessionRecordings, | ||
}, | ||
sessionRecordings: listOfSessionRecordings, | ||
}) | ||
|
@@ -382,6 +409,8 @@ describe('sessionRecordingsPlaylistLogic', () => { | |
.toFinishAllListeners() | ||
.toMatchValues({ | ||
sessionRecordingsResponse: { | ||
has_next: undefined, | ||
order: 'start_time', | ||
results: [ | ||
{ | ||
...aRecording, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commenting here so we can thread if need be
high quality photoshop warning
should it be here (but not beige)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh... and we have a random sample toggle, and a random ordering in the menu... so i can choose random random 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hesitant to put something on that level in case it ends up getting muddled with a large amount of filters. Going to go with something like this instead:
Experimented with an even more natural language based approach but I don't think it quite works because of our button paddings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌