-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Merged
Merged
feat: recordings ordering #24794
Changes from 19 commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
fd6ea28
include ordering clause
daibhin 5183a11
remove durationToShow and rely on ordering
daibhin 77a7ceb
remove earliest and correct order when merging
daibhin cc482df
cleanup function
daibhin 108d492
Merge branch 'master' into dn-feat/replay-filtering
daibhin 91653a7
inline header actions
daibhin 874d3cc
incomparible filters
daibhin f34c534
Merge branch 'master' into dn-feat/replay-filtering
daibhin 6e92bf6
add test
daibhin 10c4a74
Merge branch 'master' into dn-feat/replay-filtering
daibhin 841d907
feat: random sample recordings order
daibhin e5f62bb
Merge branch 'master' into dn-feat/replay-filtering
daibhin 635ef6f
rename latest to start_time
daibhin dd6baf2
much cleaner tests
daibhin 5629032
fix name
daibhin 971b590
remove comment
daibhin e79b621
Update query snapshots
github-actions[bot] 16c9a56
Update query snapshots
github-actions[bot] 93c56af
Update query snapshots
github-actions[bot] 347e0ea
Update query snapshots
github-actions[bot] 43c5e2b
Update query snapshots
github-actions[bot] baea08a
Update query snapshots
github-actions[bot] a9ff478
Update query snapshots
github-actions[bot] 2391d38
Update query snapshots
github-actions[bot] 0f25271
Update query snapshots
github-actions[bot] 04cc08b
Update query snapshots
github-actions[bot] 34dcb3c
Update query snapshots
github-actions[bot] 434c5b3
Update query snapshots
github-actions[bot] 6965f88
Merge branch 'master' into dn-feat/replay-filtering
daibhin 3b363aa
random sample toggle
daibhin 66e3c98
Update query snapshots
github-actions[bot] 03ccf64
Update query snapshots
github-actions[bot] 947bbfb
Update query snapshots
github-actions[bot] 45cb534
Update query snapshots
github-actions[bot] 0ff53f8
Update query snapshots
github-actions[bot] f2b86a9
Update query snapshots
github-actions[bot] 7560042
Update query snapshots
github-actions[bot] cc32efd
make table qualifier optional when replacing session id in snapshots
pauldambra d885e8a
Update query snapshots
github-actions[bot] 288488e
Update query snapshots
github-actions[bot] 437d1e0
Fangle types
pauldambra b8cfa22
Merge branch 'master' into dn-feat/replay-filtering
daibhin dbd6b65
remove random sampling
daibhin 8287d70
Update query snapshots
github-actions[bot] 5fa2c25
move ordering
daibhin b8dae38
Merge branch 'dn-feat/replay-filtering' of github.com:PostHog/posthog…
daibhin bf04127
update snapshots
daibhin bcf3aee
Update query snapshots
github-actions[bot] 1971815
Update UI snapshots for `chromium` (1)
github-actions[bot] c0db65b
Update UI snapshots for `chromium` (2)
github-actions[bot] c920e67
Merge branch 'master' into dn-feat/replay-filtering
daibhin 4c4470b
Update UI snapshots for `chromium` (1)
github-actions[bot] 2452f83
Merge branch 'master' into dn-feat/replay-filtering
daibhin b637a33
reset snapshots
daibhin aabb1eb
Update UI snapshots for `chromium` (1)
github-actions[bot] ed64528
Update UI snapshots for `chromium` (2)
github-actions[bot] 47a93f1
Merge branch 'master' into dn-feat/replay-filtering
daibhin d79ae5c
Update UI snapshots for `chromium` (1)
github-actions[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
344 changes: 172 additions & 172 deletions
344
...ssion_recordings/queries/test/__snapshots__/test_session_recording_list_from_filters.ambr
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🙌