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): provide the ability to search archived photos #6332

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

stevenwcarter
Copy link
Contributor

@stevenwcarter stevenwcarter commented Jan 11, 2024

Description

Adds a query parameter (withArchived) to the search URL parameters to allow the results to contain archived photos.

I commented on #5806 that it might be useful if we could optionally search archived photos, and this PR adds that ability. I'm not really a UI developer, so I've only added the ability to optionally perform this search with a query parameter for now. Eventually we might want to add radio options or magic search terms to enable searching archived/non-visible photos.

How has this been tested

  • Import photos
  • Perform search that returns some of those photos.
  • Archive some of the returned photos.
  • Repeat search and confirm the archived photos are not returned.
  • Add query parameter to URL &withArchived=true
  • Confirm that the previously archived photos are now returned as part of the search.

@stevenwcarter stevenwcarter changed the title Feat: provide the ability to search archived photos feat(server): provide the ability to search archived photos Jan 11, 2024
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.

Where is the difference to https://immich.app/docs/api/search-assets and the isArchived parameter?

@stevenwcarter
Copy link
Contributor Author

Hm, fair point, that I might be able to exercise that existing option. The problem is that the changes made in #5806 explicitly set the archived=false check in the query, so no archived results will ever be returned without this change.

@danieldietzler
Copy link
Member

Hm, fair point, that I might be able to exercise that existing option. The problem is that the changes made in #5806 explicitly set the archived=false check in the query, so no archived results will ever be returned without this change.

We will refactor search anyways and then I think the asset search endpoint will be the foundation for that. (it has already lots of good filters)
So I'm not sure "smart search" is even a thing that will be around much longer lol
I'll leave it to the others to decide :)

@danieldietzler
Copy link
Member

Generally, I believe this PR does look good though

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.

Thank you! Great work :)

@stevenwcarter
Copy link
Contributor Author

Do I need to clean up these conflicts in order for this to be merged? Won't most PRs cause a conflict with these build artifacts included in the repository?

@danieldietzler
Copy link
Member

Yes, they need to be cleaned up. Since those are just generated files you should be able to just run make api again (during the rebase)

Adds a query parameter (`searchArchived`) to the search URL parameters
to allow the results to contain archived photos.
@jrasm91
Copy link
Contributor

jrasm91 commented Jan 12, 2024

Yes, they need to be cleaned up. Since those are just generated files you should be able to just run make api again (during the rebase)

make open-api

@danieldietzler
Copy link
Member

Yes, they need to be cleaned up. Since those are just generated files you should be able to just run make api again (during the rebase)

make open-api

Did the open api refactoring change the target?

@jrasm91 jrasm91 enabled auto-merge (squash) January 18, 2024 02:05
@jrasm91 jrasm91 merged commit d4146e3 into immich-app:main Jan 18, 2024
21 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.

3 participants