bayang / jelu

Self hosted read and to-read list book tracker
MIT License
369 stars 14 forks source link

Implement "Random" as a sorting option, add standalone "Random" page #109

Closed DarthNerdus closed 7 months ago

DarthNerdus commented 7 months ago

Inspired by the "Random" page segment in Calibre-Web, I really enjoy rediscovering items in my list randomly. To that end, this PR implements "Random" sort.

bayang commented 7 months ago

Thank you ! That looks cool. Give me a few days to review it please.

DarthNerdus commented 7 months ago

Awesome! I just pushed a small change to remove the pagination from the Random page--a single set is all that's needed as it randomly regenerates every reload.

DarthNerdus commented 7 months ago

I further reverted the PageRequest changes as the paginator is suppressed on the Random page anyways. This cleans up the changes.

bayang commented 7 months ago

We will need tests for the backend here as well.

DarthNerdus commented 7 months ago

I'm curious -- how would you propose that? We'd need to populate the test harness DB with multiple books and can query them, but there's always the risk that two concurrent requests return the same result set/order (especially if there's a small number of results).

I'm happy to implement something, but testing a Random sortby will require a lot more standup and there is always that chance. I can't think of anything that wouldn't be mitigating a low chance that the test fails -- which will always be there.

bayang commented 7 months ago

For this use case I would go for the sanity check rather than testing that we actually get random results. We want to make sure we get a list of results even when we use the random function. This is to make sure that if one day a breaking change happens in the Exposed framework for example we have a test that will fail. That would be enough for me in this situation. What do you think of that ?

DarthNerdus commented 7 months ago

That's reasonable -- I'll look to mock that in tonight.

DarthNerdus commented 7 months ago

Alright, tests are up!

bayang commented 7 months ago

:tada: This PR is included in version 0.56.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: