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

Implement fulltext filter on search result page #9059

Merged
merged 3 commits into from
May 23, 2023
Merged

Implement fulltext filter on search result page #9059

merged 3 commits into from
May 23, 2023

Conversation

JammingBen
Copy link
Collaborator

@JammingBen JammingBen commented May 17, 2023 •
edited
Loading

Description

Adds a fulltext filter toggle to the search result page.

You need apache tika to enable fulltext search on your local machine. This can be achieved by adding the following to your docker-compose.override.yml. Note that the tike service needs to be started and initialized before starting ocis.

services:
  ocis:
    environment:
      FRONTEND_FULL_TEXT_SEARCH_ENABLED: "true"
      SEARCH_EXTRACTOR_TYPE: "tika"
      SEARCH_EXTRACTOR_TIKA_TIKA_URL: "http://host.docker.internal:9998"
  tika:
    image: apache/tika:latest
    ports:
      - 9998:9998

Also note that combining multiple filters does not work optimal yet. I plan to do that in another PR once owncloud/ocis#6343 is resolved.

Related Issue

Screenshots

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

@JammingBen JammingBen self-assigned this May 17, 2023
@JammingBen JammingBen force-pushed the search-in-file-content branch 2 times, most recently from d9f986d to 143ffec Compare May 17, 2023 11:43
@owncloud owncloud deleted a comment from update-docs bot May 17, 2023
@JammingBen JammingBen force-pushed the search-in-file-content branch 4 times, most recently from c6bc31c to faa1469 Compare May 17, 2023 14:08
@delete-merged-branch delete-merged-branch bot deleted the branch master May 17, 2023 14:57
@JammingBen JammingBen changed the base branch from search-tag-filter to master May 17, 2023 14:57
@JammingBen JammingBen marked this pull request as ready for review May 17, 2023 14:57
@AlexAndBear
Copy link
Contributor

Didn't have a look yet, but shouldn't the toggle be the oc-switch element? having it looking like the filter chip is confusing me in the screenshot

@JammingBen
Copy link
Collaborator Author

Didn't have a look yet, but shouldn't the toggle be the oc-switch element? having it looking like the filter chip is confusing me in the screenshot

That's intended as it should match the other filter chips. I evaluated a few options together with @tbsbdr and we finally decided on this for look and feel 🙂

AlexAndBear
AlexAndBear reviewed May 23, 2023
View reviewed changes
packages/web-app-files/src/components/Search/List.vue Outdated
Comment on lines 30 to 34
<item-filter-toggle
:filter-label="$gettext('Search in file content')"
filter-name="fulltext"
class="files-search-filter-fulltext oc-mr-s"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Tika seems to required for full-text search, I am wondering if we shouldn't rely on a capability then to render this toggle only if capability is enabned

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've created owncloud/ocis#6367, Julian will tackle it as soon as he finds the time.

Copy link
Member

Choose a reason for hiding this comment

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

guys! love it 😍

@AlexAndBear
Copy link
Contributor

Minor nitpick, but shouldn't be fullText instead of fulltext?

@ownclouders
Copy link
Contributor

Results for e2e-tests oC10 https://drone.owncloud.com/owncloud/web/35919/11/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/35919/tracing/public-link-alice-2023-5-23-11-45-18.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/35919/tracing/public-link-anonymous-2023-5-23-11-45-39.zip

💥 The e2e-oc10 tests pipeline failed. The build has been cancelled.

@sonarcloud
Copy link

sonarcloud bot commented May 23, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

56.7% 56.7% Coverage
0.0% 0.0% Duplication

@JammingBen
Copy link
Collaborator Author

I addressed your comments. The capability as well as the ability to combine filters will be follow-ups because I don't know when the server part is done.

@JammingBen JammingBen requested a review from lookacat May 23, 2023 12:24
@JammingBen JammingBen merged commit ce1d270 into master May 23, 2023
@delete-merged-branch delete-merged-branch bot deleted the search-in-file-content branch May 23, 2023 12:43
@JammingBen JammingBen mentioned this pull request May 23, 2023
Merged
@micbar micbar mentioned this pull request Jul 24, 2023
Closed
68 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexAndBear AlexAndBear AlexAndBear approved these changes

@kulmann kulmann Awaiting requested review from kulmann

@lookacat lookacat Awaiting requested review from lookacat

Assignees

@JammingBen JammingBen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[web] Add "Search in file content" filter Chip
5 participants
@JammingBen @AlexAndBear @ownclouders @lookacat @kulmann