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

Augment search to consider tags #8842

Merged
merged 1 commit into from
Dec 11, 2022
Merged

Augment search to consider tags #8842

merged 1 commit into from
Dec 11, 2022

Conversation

bradbeattie
Copy link
Contributor

@bradbeattie bradbeattie commented Dec 3, 2022

Changes
This minor change permits users to search for items via tags.

Issues

Commentary

Seems to be a frequently requested feature. I considered searching with a join on the ItemValues table, but figured this direct approach would likely be sufficient.

@Shadowghost
Copy link
Contributor

Please make sure that excludedTags are respected and that the added clause does not clash with them.

@bradbeattie
Copy link
Contributor Author

bradbeattie commented Dec 3, 2022

Please make sure that excludedTags are respected and that the added clause does not clash with them.

I wasn't aware of said functionality in Jellyfin! Checking...

Yes, tested and confirmed things still work as expected. My proposed change only alters the SearchScore. https://github.com/jellyfin/jellyfin/blob/master/Emby.Server.Implementations/Data/SqliteItemRepository.cs#L4523 still excludes content regardless of the score.

@bradbeattie bradbeattie changed the title Changed search to also consider tags Search augmented to consider tags Dec 5, 2022
@bradbeattie bradbeattie changed the title Search augmented to consider tags Augment search to consider tags Dec 5, 2022
Copy link
Contributor

@Shadowghost Shadowghost left a comment

Choose a reason for hiding this comment

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

Haven't tested it but code seems to be fine.
Would be nice if you can squash the commits. 3 commits for a one line change seem a bit much.

@bbeattie-phxlabs
Copy link

I'll squash them tonight when I'm at my home PC. Thanks!

@bradbeattie
Copy link
Contributor Author

Okay, merge squashed. Back over to you!

@bradbeattie
Copy link
Contributor Author

Considering bradbeattie/jellyfin@master...augmented-tag-searching as a second revision of this. I'll test it when I get a chance before submitting a PR.

@chinobis
Copy link

Ok, I'm a retard. How do I implement this?

@bradbeattie
Copy link
Contributor Author

Ok, I'm a retard. How do I implement this?

One method would be to use the unstable docker image, or wait until 10.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants