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

fix(server): don't return archived assets by default #7278

Merged
merged 5 commits into from Feb 21, 2024

Conversation

mertalev
Copy link
Contributor

Description

The search endpoint doesn't explicitly set withArchived right now, so it won't be part of the query unless the caller sets it to false themselves. This makes it default to false if not provided.

Fixes the behavior described in this comment

Copy link

cloudflare-pages bot commented Feb 21, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3117ad6
Status: ✅  Deploy successful!
Preview URL: https://df46f7b8.immich.pages.dev
Branch Preview URL: https://fix-server-search-archive.immich.pages.dev

View logs

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Would it be better to just default it in the dto instead? Also, why did you add another asset to the e2e? There was already an archived asset you should have been able to use.

@mertalev
Copy link
Contributor Author

Would it be better to just default it in the dto instead? Also, why did you add another asset to the e2e? There was already an archived asset you should have been able to use.

There were too many tests failing with this change because they all reference asset3 and it's archived so it doesn't show by default now. I removed the isArchived flag from it and made a separate asset for this.

@alextran1502 alextran1502 enabled auto-merge (squash) February 21, 2024 03:55
@alextran1502 alextran1502 merged commit eb73f66 into main Feb 21, 2024
24 of 25 checks passed
@alextran1502 alextran1502 deleted the fix/server-search-archive branch February 21, 2024 03:59
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.

None yet

3 participants