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.

Sign up for GitHub

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

Jump to bottom

Files: Extend search to also cover tags #26813

Merged
merged 1 commit into from
May 18, 2022
Merged

Files: Extend search to also cover tags #26813

merged 1 commit into from
May 18, 2022

Conversation

marcelklehr
Copy link
Member

@marcelklehr marcelklehr commented Apr 28, 2021 •
edited
Loading

fixes #326

This is my first real server PR so please bear with me :)

@marcelklehr marcelklehr force-pushed the feature/files-search-tags branch 2 times, most recently from 1eaa8a4 to 82f8737 Compare April 28, 2021 17:48
@Spartachetto
Copy link

@marcelklehr thank you!!!!!!!!!!
That is a feature request from 2016!

Beyond that, I'd like to remember two other issues:

This just because maybe it could be useful to review this code keeping in mind the requirements of those FR.
Thanks again.

@marcelklehr marcelklehr force-pushed the feature/files-search-tags branch 5 times, most recently from 7665381 to 9995706 Compare April 29, 2021 08:31
@marcelklehr marcelklehr marked this pull request as ready for review April 29, 2021 09:49
@marcelklehr
Copy link
Member Author 

Failed scenarios:
 /drone/src/tests/acceptance/features/app-files-tags.feature:22

But I can't replicate that manually.

@icewind1991
Copy link
Member

I'm a bit concerned about the performance impact of joining the tag tables for every query

@marcelklehr
Copy link
Member Author

I'm a bit concerned about the performance impact of joining the tag tables for every query

The systemtag and vcategory tables are only joined when the query involves the fields tagname or systemtag. Which for the FilesSearchProvider is indeed every time.

Can we extract something actionable from this concern? Should I run a benchmark? Should we cc someone else to weigh in? Should we close the feature request as wontfix due to performance reasons?

@szaimen szaimen added this to the Nextcloud 23 milestone Jun 18, 2021
@marcelklehr marcelklehr force-pushed the feature/files-search-tags branch 2 times, most recently from fdd9bd1 to 1d320fc Compare June 22, 2021 16:25
@dmuensterer dmuensterer mentioned this pull request Aug 25, 2021
Open
@dmuensterer
Copy link

Any update on this?

@szaimen
Copy link
Contributor

szaimen commented Sep 13, 2021

/rebase

@marcelklehr marcelklehr mentioned this pull request Sep 28, 2021
Closed
@marcelklehr
Copy link
Member Author

Done :)

@szaimen
Copy link
Contributor

szaimen commented May 17, 2022

Done :)

Thanks! I guess squashing would also be good, then I merge :)

@marcelklehr
Files: Extend search to also cover tags
f755ee0 
fixes #326

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
Copy link
Member Author

Done :)

@bonswouar
Copy link

Thank you so much @marcelklehr for this, and Nextcloud's maintainers for your feedbacks and accepting to merge it! This feature will make a big difference, love it!
(I know this isn't a very useful comment but I had to communicate my appreciation ;))

@szaimen
Copy link
Contributor

szaimen commented May 18, 2022

Thanks! CI failure unrelated

@szaimen szaimen merged commit 18dd460 into master May 18, 2022
@szaimen szaimen deleted the feature/files-search-tags branch May 18, 2022 08:58
@BloodyIron
Copy link

yay! \o/

@darguez
Copy link

darguez commented Aug 2, 2022 •
edited
Loading

Is it posible to select the found files (searched by tag) to perform any action with them? (f.i. copy them to a folder to share them). Why the search view does not show the checkboxes? (btw right click shows Select option but it does nothing) I have NC23. Thanks and great work!

@szaimen szaimen mentioned this pull request Sep 16, 2022
Closed
@nkeilar
Copy link

nkeilar commented Sep 28, 2022

Looks like a great improvement - good work. Just one small issue - it doesn't seem working for me.

We are running 24.0.5 but when I search for folder with tags, no results are showing. Does anything need to be done to enable this functionality?

Additionally, we actually really want to be able to use an API to query NC for a list of files/folders with a given tag. This is for integration with a third parry app we are building. Is their a search API that a client could use to search NC by tags?

Thanks in advance for any assistance - we are a bit stuck!

@tcitworld
Copy link
Member

We are running 24.0.5 but when I search for folder with tags, no results are showing. Does anything need to be done to enable this functionality?

As per the milestone set on the right sidebar of this page, this is a Nextcloud 25 feature.

kesselb added a commit that referenced this pull request Sep 14, 2023
@kesselb
fix: don't use davtags for tag search
da226d8 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb mentioned this pull request Sep 14, 2023
Merged
5 tasks
kesselb added a commit that referenced this pull request Sep 15, 2023
@kesselb
fix: don't use davtags for tag search
8395eb7 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit that referenced this pull request Sep 15, 2023
@kesselb
fix: don't use davtags for tag search
4c5e79d 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
backportbot-nextcloud bot pushed a commit that referenced this pull request Sep 15, 2023
@kesselb @backportbot-nextcloud
fix: don't use davtags for tag search
562c472 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
backportbot-nextcloud bot pushed a commit that referenced this pull request Sep 15, 2023
@kesselb @backportbot-nextcloud
fix: don't use davtags for tag search
5a2ab7b 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
backportbot-nextcloud bot pushed a commit that referenced this pull request Sep 15, 2023
@kesselb @backportbot-nextcloud
fix: don't use davtags for tag search
cc047d8 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit that referenced this pull request Sep 18, 2023
@kesselb
fix: don't use davtags for tag search
27dc783 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb added a commit that referenced this pull request Sep 18, 2023
@kesselb
fix: don't use davtags for tag search
a75d3e5 
1. #26813 Initial implementation with support for systemtags and davtags (vcategory)
2. #39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@ChristophWurst

This comment was marked as off-topic.

Sign in to view
zak39 pushed a commit to arawa/server that referenced this pull request Oct 11, 2023
@kesselb @zak39
fix: don't use davtags for tag search
770f4ab 
1. nextcloud#26813 Initial implementation with support for systemtags and davtags (vcategory)
2. nextcloud#39062 Additional check if the given tag exists, though ISystemTagManager.getAllTags only looks for systemtags

Therefore it's not possible anymore to search for davtags and unnecessary to join the other tables.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@BloodyIron
Copy link

Not seeing this mentioned in the NC 25.0.0 release announcement.. What version did this actually make it into? : https://nextcloud.com/blog/announcing-nextcloud-hub-3-brand-new-design-and-photos-2-0-with-editor-and-ai/

@marcelklehr
Copy link
Member Author

This pull request first appeared in v25.0.0beta1

@BloodyIron v25

@BloodyIron
Copy link

This pull request first appeared in v25.0.0beta1

@BloodyIron v25

I don't see release notes referencing this issue# or commit, and checking the changes between v25.0.0beta1 and v24.0.0 is over 1200 commits. So I don't see a way to explicitly confirm it. And while I suspect you're right, I'm a fan of a bird in the hand. Unless there's a method I'm not seeing? 🤔

@marcelklehr
Copy link
Member Author

@BloodyIron You can view the commit in this PR and github will tell you which tags the commit appears in. The first tag is something with v25

@nextcloud nextcloud locked as resolved and limited conversation to collaborators Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@szaimen szaimen Awaiting requested review from szaimen

@nimishavijay nimishavijay Awaiting requested review from nimishavijay

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement feature: files feature: search feature: tags
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging this pull request may close these issues.

Use the search bar to search for files by tag