Redmine

I would love to have this feature incorporated into Redmine. I think it would be very useful!

As a slight addition though: As a search over many objects can be quite expensive, it might be worthwhile to further filter the projects passed to Redmine::Search::Fetcher.new, similar to the rules in Query#project_statement, so that the search results are only fetched from the selected and visible (sub-)projects of the query rather than all (sub-)projects. While this does not change anything functionally (as the actual returned issue query result is further restricted by the actual project filter), it may significantly improve the performance of filtered queries in large Redmines. This would also avoid searching in archived projects or projects not visible to the current user, whose issues we would never return.

This could be implemented as a new method on the IssueQuery model which could look like this:

def projects
  if project
    subprojects = project.descendants.allowed_to(:view_issues)
    if has_filter?("subproject_id")
      case operator_for("subproject_id")
      when '='
        # include the selected subprojects
        [project] + subprojects.where(id: values_for("subproject_id").map(&:to_i))
      when '!'
        # exclude the selected subprojects
        [project] + subprojects.where.not(id: values_for("subproject_id").map(&:to_i))
      when '!*'
        # main project only
        [project]
      else
        # all subprojects
        project_ids = [project] + subprojects
      end
    elsif Setting.display_subprojects_issues?
      [project] + subprojects
    else
      [project]
    end
  else
    projects = Project.allowed_to(:view_issues)
    case operator_for("project_id")
    when '='
      # include the selected subprojects
      projects.where(id: values_for("project_id").map(&:to_i)).to_a
    when '!'
      # exclude the selected subprojects
      projects.where.not(id: values_for("subproject_id").map(&:to_i)).to_a
    else
      # All projects
      nil
    end
  end
end

(Note that I have not fully tested this yet)

What do you think?

Holger Just wrote in #note-2:

As a slight addition though: As a search over many objects can be quite expensive, it might be worthwhile to further filter the projects passed to Redmine::Search::Fetcher.new, similar to the rules in Query#project_statement, so that the search results are only fetched from the selected and visible (sub-)projects of the query rather than all (sub-)projects.

Thank you for reviewing the patches and suggesting an improvement! The improvement makes the code much more efficient.

However, Redmine will have very similar codes in two places after adding the suggested IssueQuery#projects. It would be better if we can somehow use Query#project_statement.

I think that changing 0001-Add-Any-serchable-text-filter-for-issues.patch as follows can avoid the code dupulication and reduce the number of projects to search. What is your opinion?

diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb
index 4db44b6b3..11f3f2adb 100644
--- a/app/models/issue_query.rb
+++ b/app/models/issue_query.rb
@@ -778,7 +778,11 @@ class IssueQuery < Query

   def sql_for_any_searchable_field(field, operator, value)
     question = value.first
-    projects = project&.self_and_descendants
+    if project || operator_for('subproject_id').present?
+      projects = Project.where(project_statement).allowed_to(:view_issues)
+    else
+      projects = nil
+    end
     fetcher = Redmine::Search::Fetcher.new(
       question, User.current, ['issue'], projects, attachments: '0'
     )

Turns out, there were a couple of bugs / typos / copy-and-paste errors in my proposed method. Sorry for that :/ In any case, your proposal would allow for almost the same effect with much less new code so it is indeed much better!

Your proposal however does not handle global issue queries unfortunately. Here, we don't have a subproject_id filter but a separate project_id filter (which is added to the query statement with a generic sql_for_field call). To restrict the searched projects on global issue queries, we should also handle that case.

In my updated proposal below, we use the project_statement in case we are a project-local query as you have proposed. If I have understood the logic in Query#project_statement correctly, it will always return an SQL snippet if there is a project, potentially further restricted according to the subproject_id filter. Thus, I don't think we have to check for the presence of the subproject_id filter in sql_for_any_searchable_field.

For the global case, where we don't have a project for the query, we can restrict projects according to the project_id filter with the sql_for_field method. The project_id filter is defined to have only the = and ! operators (although I don't think this is verified during query generation), so this should generate a simple statement we can use in the global case.

  def sql_for_any_searchable_field(field, operator, value)
    question = value.first

    project_scope = Project.allowed_to(:view_issues)
    if project
      projects = project_scope.where(project_statement)
    elsif has_filter?('project_id')
      projects = project_scope.where(sql_for_field('project_id', operator_for('project_id'), values_for('project_id'), Project.table_name, 'id'))
    else
      projects = nil
    end

    fetcher = Redmine::Search::Fetcher.new(
      question, User.current, ['issue'], projects, attachments: '0'
    )
    # ...

What do you think?

Holger Just wrote in #note-7:

I was just having another look at r22164. Are you sure that the logic is correct for the !~ filter in case we find no issues with the search?

If I'm reading the code correctly, we now restrict the final query result to also return no issues (by returning '1=0') in that case. However, I think we should probably not restrict the result at all in that case?

You are right, "doesn't contains" filter does not work correctly when no issue matches. Thank you for pointing it out.

Committed the fix in #note-7 in r22168.

I found another issue that should be fixed.

The new filter currently performs an OR search when multiple keywords are given, but I found that it should perform an AND search instead. This is because the "contains" operator for other string/text fields performs an AND search.

Looks good to me, thanks!

I'd still have an updated German translation for the new filter if you like :)

diff --git a/config/locales/de.yml b/config/locales/de.yml
index 71c315e487..ad6e4c071d 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -280,6 +280,7 @@ de:
   field_active: Aktiv
   field_activity: Aktivität
   field_admin: Administrator
+  field_any_searchable: Durchsuchbarer Text
   field_assignable: Tickets können dieser Rolle zugewiesen werden
   field_assigned_to: Zugewiesen an
   field_assigned_to_role: Zuständigkeitsrolle
@@ -1439,4 +1440,3 @@ de:
     them is the better solution.
   text_users_bulk_destroy_confirm: To confirm, please enter "%{yes}" below.
   label_auto_watch_on_issue_created: Issues I created
-  field_any_searchable: Any searchable text

Good filter. Thanks.

Holger Just wrote in #note-14:

I'd still have an updated German translation for the new filter if you like :)

Thank you, I committed the update for German translation in r22174.

LGTM, thanks!