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: make full text search default #10626

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Conversation

JammingBen
Copy link
Contributor

Description

Enables full text search as default if the server supports it.

Related Issue

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
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@JammingBen JammingBen self-assigned this Mar 18, 2024
@JammingBen JammingBen force-pushed the make-full-text-search-default branch from 098ba79 to a0d5ccd Compare March 18, 2024 12:20
@JammingBen JammingBen marked this pull request as ready for review March 18, 2024 12:48
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Same logic as in the search result page should also apply for the search preview.

packages/web-app-files/src/components/Search/List.vue Outdated Show resolved Hide resolved
const fullTextParam = useRouteQuery('q_fullText')
const titleOnlyParam = useRouteQuery('q_titleOnly')

const fullTextSearchEnabled = computed(() => capabilityStore.searchContent?.enabled)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using storeToRefs for that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's nested in the capabilities, meaning even if we'd use storeToRefs, we would still need to further narrow it down via a computed.

@JammingBen JammingBen force-pushed the make-full-text-search-default branch 3 times, most recently from c1b28de to f9c97ca Compare March 19, 2024 09:38
@JammingBen
Copy link
Contributor Author

Okay, now it searches for name and content at the same time if supported. Also applies to the search bar.

I decided to remove the generic query building we had before because it didn't really allow edge cases like we have here.

Enables full text search as default if the server supports it.
@JammingBen JammingBen force-pushed the make-full-text-search-default branch from f9c97ca to bc052dd Compare March 19, 2024 10:21
Copy link

sonarcloud bot commented Mar 19, 2024

@JammingBen JammingBen requested a review from kulmann March 19, 2024 11:32
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

🥳

@JammingBen JammingBen merged commit fee4b99 into master Mar 19, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the make-full-text-search-default branch March 19, 2024 13:47
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.

[web] Enable Fulltext Search by default
3 participants