Skip to content
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(server): sort assets randomly from the API 'api/search/metadata' endpoint by including 'order': 'rand' in the API call. #12741

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

jschwalbe
Copy link
Contributor

@jschwalbe jschwalbe commented Sep 17, 2024

Warning

GET /api/assets/random is now deprecated in favor of POST /api/search/random

Goal is to simplify getting random photos for a kiosk. The new API call works as such:

curl -L "http://localhost:2283/api/search/random" \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json' \
  -H "x-api-key: $API_KEY"  \
  -d  '{ "size": 100 }'

@jschwalbe jschwalbe changed the title Add ability to sort assets randomly from the API 'api/search/metadata endpoint by including 'order': 'rand' in the API call. feat(server): sort assets randomly from the API 'api/search/metadata' endpoint by including 'order': 'rand' in the API call. Sep 17, 2024
@jrasm91 jrasm91 requested a review from mertalev September 23, 2024 13:44
builder.orderBy('asset.fileCreatedAt', options.orderDirection ?? 'DESC');

if (options.random) {
mode = PaginationMode.LIMIT_OFFSET;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think limit/offset is safe here because there are possible joins with one-to-many or many-to-many relations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A thought: you could make a separate /search/random endpoint that takes the same options as the smart search endpoint but without the query field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The other pagination throws an error I did not know how to fix. If you have time can you take a look?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I think I figured it out.

@jrasm91 jrasm91 force-pushed the api-with-random branch 2 times, most recently from fec9f98 to af077b1 Compare September 23, 2024 15:23
Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jrasm91 jrasm91 merged commit 9f8a7e0 into immich-app:main Sep 23, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants