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

Add FullTextSearch::IssueAnySearchable module for multi-field AND searches #164

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

otegami
Copy link
Contributor

@otegami otegami commented Jan 30, 2025

GitHub: GH-163

This commit introduces the FullTextSearch::IssueAnySearchable module to the IssueQuery class.
The module is designed to support AND-based searches across multiple fields.

This initial implementation only sets up the module and its interface without implementing the actual search logic.
At the following PRs, we will implement the search logic and adding relevant tests.

…earches

GitHub: clear-codeGH-163

This commit introduces the `FullTextSearch::IssueAnySearchable`
module to the `IssueQuery` class. The module is designed to
support AND-based searches across multiple fields.

This initial implementation only sets up the module and its
interface without implementing the actual search logic.

At the following PRs, we will implemente the search logic and
adding relevant tests.
lib/full_text_search/issue_any_searchable.rb Outdated Show resolved Hide resolved
init.rb Outdated Show resolved Hide resolved
@otegami otegami force-pushed the add-issue-any-searchable branch 2 times, most recently from 70264f9 to 21aedb8 Compare January 31, 2025 05:30
@otegami otegami force-pushed the add-issue-any-searchable branch from 6ebccfe to 2dc7600 Compare January 31, 2025 05:46
init.rb Outdated
@@ -91,6 +91,11 @@ class << Setting
FullTextSearch::TagType
FullTextSearch::Type

if Gem::Version.new(Redmine::VERSION) > Gem::Version.new("5.1")
Copy link
Contributor Author

@otegami otegami Jan 31, 2025

Choose a reason for hiding this comment

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

I added this condition because the original Issue#sql_for_any_searchable_fieldis called and causes the error by the time we implement the IssueAnySearchable#sql_for_any_searchable_field.

Copy link
Member

Choose a reason for hiding this comment

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

Can we use feature based check instead of version based check?

BTW, should we use >= here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: 9cfbe32 Thanks, is this implementation satisfied with your feature based check?

@otegami
Copy link
Contributor Author

otegami commented Jan 31, 2025

@kou
Thanks you for reviewing. I've just fixed what you pointed out.

}
}
)
searched_issues = Issue.where(query.statement).order(:id)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that issues page use IssueQuery#issues not Issue.where(query.statement): https://github.com/redmine/redmine/blob/09167d0b3238a4a7e27fded6a4111fe39f1bf22f/app/controllers/issues_controller.rb#L62

Should we use the same API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: 08ef3cc I think using the same API is better. I fixed it.

@otegami
Copy link
Contributor Author

otegami commented Jan 31, 2025

@kou
Thanks you for reviewing. I've just fixed what you pointed out.

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.

2 participants