Feature #44464 » 0001-Show-a-warning-listing-the-search-terms-actually-use.patch
| app/helpers/queries_helper.rb | ||
|---|---|---|
| 66 | 66 |
s |
| 67 | 67 |
end |
| 68 | 68 | |
| 69 |
# Renders the warnings about the filters of the given query |
|
| 70 |
def render_query_filter_warnings(query) |
|
| 71 |
safe_join(query.filter_warnings.map {|warning| content_tag('p', warning, :class => 'warning')})
|
|
| 72 |
end |
|
| 73 | ||
| 69 | 74 |
def query_filters_hidden_tags(query) |
| 70 | 75 |
tags = ''.html_safe |
| 71 | 76 |
query.filters.each do |field, options| |
| app/models/query.rb | ||
|---|---|---|
| 348 | 348 |
:tree => ["=", "~", "!*", "*"] |
| 349 | 349 |
} |
| 350 | 350 | |
| 351 |
# Filter types and operators whose value is split into tokens |
|
| 352 |
# by Redmine::Search::Tokenizer |
|
| 353 |
TOKENIZED_FILTER_TYPES = [:string, :text, :search].freeze |
|
| 354 |
TOKENIZED_FILTER_OPERATORS = ["~", "*~", "!~", "^", "$"].freeze |
|
| 355 | ||
| 351 | 356 |
class_attribute :available_columns |
| 352 | 357 |
self.available_columns = [] |
| 353 | 358 | |
| ... | ... | |
| 547 | 552 |
errors.add(:base, m) |
| 548 | 553 |
end |
| 549 | 554 | |
| 555 |
# Returns warning messages about filters that are applied only partially. |
|
| 556 |
# Unlike errors, warnings do not prevent the query from being run |
|
| 557 |
def filter_warnings |
|
| 558 |
return [] unless filters |
|
| 559 | ||
| 560 |
filters.keys.filter_map do |field| |
|
| 561 |
next unless TOKENIZED_FILTER_TYPES.include?(type_for(field)) && |
|
| 562 |
TOKENIZED_FILTER_OPERATORS.include?(operator_for(field)) |
|
| 563 | ||
| 564 |
tokenizer = Redmine::Search::Tokenizer.new(values_for(field)&.first) |
|
| 565 |
next unless tokenizer.truncated? |
|
| 566 | ||
| 567 |
connector = l(:'support.array.words_connector') |
|
| 568 |
tokens = tokenizer.tokens.map {|token| %("#{token}")}.join(connector)
|
|
| 569 |
message = l(:notice_search_tokens_truncated, |
|
| 570 |
:max => Redmine::Search::Tokenizer::MAX_TOKENS, |
|
| 571 |
:tokens => tokens.truncate(100, :separator => connector)) |
|
| 572 |
"#{label_for(field)}: #{message}"
|
|
| 573 |
end |
|
| 574 |
end |
|
| 575 | ||
| 550 | 576 |
def editable_by?(user) |
| 551 | 577 |
return false unless user |
| 552 | 578 | |
| app/views/calendars/show.html.erb | ||
|---|---|---|
| 51 | 51 |
<% end %> |
| 52 | 52 | |
| 53 | 53 |
<%= error_messages_for 'query' %> |
| 54 |
<%= render_query_filter_warnings @query %> |
|
| 54 | 55 |
<% if @query.valid? %> |
| 55 | 56 |
<%= render :partial => 'common/calendar', :locals => {:calendar => @calendar} %>
|
| 56 | 57 | |
| app/views/gantts/show.html.erb | ||
|---|---|---|
| 10 | 10 | |
| 11 | 11 |
<%= render partial: 'query_form', locals: {project: @project, query: @query, gantt: @gantt} %>
|
| 12 | 12 |
<%= error_messages_for 'query' %> |
| 13 |
<%= render_query_filter_warnings @query %> |
|
| 13 | 14 | |
| 14 | 15 |
<% if @query.valid? %> |
| 15 | 16 |
<%= render partial: 'chart', locals: {gantt: @gantt, query: @query} %>
|
| app/views/queries/_query_form.html.erb | ||
|---|---|---|
| 77 | 77 |
</div> |
| 78 | 78 | |
| 79 | 79 |
<%= error_messages_for @query %> |
| 80 |
<%= render_query_filter_warnings @query %> |
|
| 80 | 81 | |
| 81 | 82 |
<%= javascript_tag do %> |
| 82 | 83 |
$(function ($) {
|
| config/locales/en.yml | ||
|---|---|---|
| 188 | 188 |
notice_unable_delete_time_entry: Unable to delete time log entry. |
| 189 | 189 |
notice_issue_done_ratios_updated: Issue done ratios updated. |
| 190 | 190 |
notice_gantt_chart_truncated: "The chart was truncated because it exceeds the maximum number of items that can be displayed (%{max})"
|
| 191 |
notice_search_tokens_truncated: "A maximum of %{max} search terms can be used. Only the following terms were used: %{tokens}"
|
|
| 191 | 192 |
notice_issue_successful_create: "Issue %{id} created."
|
| 192 | 193 |
notice_issue_update_conflict: "The issue has been updated by an other user while you were editing it." |
| 193 | 194 |
notice_account_deleted: "Your account has been permanently deleted." |
| lib/redmine/search.rb | ||
|---|---|---|
| 127 | 127 |
end |
| 128 | 128 | |
| 129 | 129 |
class Tokenizer |
| 130 |
# Maximum number of tokens to search for |
|
| 131 |
MAX_TOKENS = 5 |
|
| 132 | ||
| 130 | 133 |
def initialize(question) |
| 131 | 134 |
@question = question.to_s |
| 132 | 135 |
end |
| 133 | 136 | |
| 137 |
# Returns the tokens to search for (no more than MAX_TOKENS) |
|
| 134 | 138 |
def tokens |
| 135 |
# extract tokens from the question |
|
| 136 |
# eg. hello "bye bye" => ["hello", "bye bye"] |
|
| 137 |
tokens = @question.scan(/"[^"]+"|[^\p{Zs}]+/).map do |token|
|
|
| 138 |
# Remove quotes from quoted tokens, strip surrounding whitespace |
|
| 139 |
# e.g. "\" foo bar \"" => "foo bar" |
|
| 140 |
token.gsub(/\A"\p{Zs}*|\p{Zs}*"\Z/, '')
|
|
| 139 |
all_tokens.first(MAX_TOKENS) |
|
| 140 |
end |
|
| 141 | ||
| 142 |
# Returns true if some tokens are ignored because the question |
|
| 143 |
# contains more than MAX_TOKENS tokens |
|
| 144 |
def truncated? |
|
| 145 |
all_tokens.size > MAX_TOKENS |
|
| 146 |
end |
|
| 147 | ||
| 148 |
private |
|
| 149 | ||
| 150 |
def all_tokens |
|
| 151 |
@all_tokens ||= begin |
|
| 152 |
# extract tokens from the question |
|
| 153 |
# eg. hello "bye bye" => ["hello", "bye bye"] |
|
| 154 |
tokens = @question.scan(/"[^"]+"|[^\p{Zs}]+/).map do |token|
|
|
| 155 |
# Remove quotes from quoted tokens, strip surrounding whitespace |
|
| 156 |
# e.g. "\" foo bar \"" => "foo bar" |
|
| 157 |
token.gsub(/\A"\p{Zs}*|\p{Zs}*"\Z/, '')
|
|
| 158 |
end |
|
| 159 |
# tokens must be at least 2 characters long |
|
| 160 |
# but for Chinese characters (Chinese HANZI/Japanese KANJI), tokens can be one character |
|
| 161 |
tokens.uniq.select{|w| w.length > 1 || w =~ /\p{Han}/}
|
|
| 141 | 162 |
end |
| 142 |
# tokens must be at least 2 characters long |
|
| 143 |
# but for Chinese characters (Chinese HANZI/Japanese KANJI), tokens can be one character |
|
| 144 |
# no more than 5 tokens to search for |
|
| 145 |
tokens.uniq.select{|w| w.length > 1 || w =~ /\p{Han}/}.first 5
|
|
| 146 | 163 |
end |
| 147 | 164 |
end |
| 148 | 165 | |
| test/functional/issues_controller_test.rb | ||
|---|---|---|
| 180 | 180 |
assert_query_filters [['tracker_id', '=', '1']] |
| 181 | 181 |
end |
| 182 | 182 | |
| 183 |
def test_index_with_too_many_tokens_in_filter_should_display_warning_and_results |
|
| 184 |
get( |
|
| 185 |
:index, |
|
| 186 |
:params => {
|
|
| 187 |
:project_id => 1, |
|
| 188 |
:set_filter => 1, |
|
| 189 |
:f => ['subject'], |
|
| 190 |
:op => {'subject' => '*~'},
|
|
| 191 |
:v => {'subject' => ['one two print <script>alert(1)</script> three four']}
|
|
| 192 |
} |
|
| 193 |
) |
|
| 194 |
assert_response :success |
|
| 195 | ||
| 196 |
assert_select '#errorExplanation', 0 |
|
| 197 |
assert_select 'p.warning', :count => 1, |
|
| 198 |
:text => 'Subject: A maximum of 5 search terms can be used. Only the following terms were used: ' \ |
|
| 199 |
'"one", "two", "print", "<script>alert(1)</script>", "three"' |
|
| 200 |
assert_not_include '<script>alert(1)</script>', response.body |
|
| 201 |
# Issues are still displayed using the first 5 tokens |
|
| 202 |
assert_select 'table.issues tr.issue' |
|
| 203 |
end |
|
| 204 | ||
| 183 | 205 |
def test_index_with_short_filters |
| 184 | 206 |
to_test = {
|
| 185 | 207 |
'status_id' => {
|
| test/unit/lib/redmine/search_test.rb | ||
|---|---|---|
| 35 | 35 |
value = '"phrase one" "phrase two"' |
| 36 | 36 |
assert_equal ["phrase one", "phrase two"], Redmine::Search::Tokenizer.new(value).tokens |
| 37 | 37 |
end |
| 38 | ||
| 39 |
def test_tokenize_should_return_no_more_than_max_tokens |
|
| 40 |
tokenizer = Redmine::Search::Tokenizer.new('one two three four five six seven')
|
|
| 41 |
assert_equal %w[one two three four five], tokenizer.tokens |
|
| 42 |
assert tokenizer.truncated? |
|
| 43 | ||
| 44 |
assert_not Redmine::Search::Tokenizer.new('one two three four five').truncated?
|
|
| 45 |
end |
|
| 38 | 46 |
end |
| test/unit/query_test.rb | ||
|---|---|---|
| 796 | 796 |
result.each {|issue| assert issue.subject =~ /(close|block)/i}
|
| 797 | 797 |
end |
| 798 | 798 | |
| 799 |
def test_filter_warnings_should_include_used_tokens |
|
| 800 |
query = IssueQuery.new( |
|
| 801 |
:name => '_', |
|
| 802 |
:filters => {
|
|
| 803 |
'subject' => {:operator => '*~', :values => ['one two three four five six seven']},
|
|
| 804 |
'description' => {:operator => '~', :values => ['one two three four five']},
|
|
| 805 |
'any_searchable' => {:operator => '!~', :values => ['a one "two three" one four five six seven']}
|
|
| 806 |
} |
|
| 807 |
) |
|
| 808 |
assert query.valid? |
|
| 809 |
assert_equal( |
|
| 810 |
[ |
|
| 811 |
'Subject: A maximum of 5 search terms can be used. Only the following terms were used: "one", "two", "three", "four", "five"', |
|
| 812 |
'Any searchable text: A maximum of 5 search terms can be used. Only the following terms were used: "one", "two three", "four", "five", "six"' |
|
| 813 |
], |
|
| 814 |
query.filter_warnings |
|
| 815 |
) |
|
| 816 |
end |
|
| 817 | ||
| 818 |
def test_filter_warnings_should_ignore_operators_and_filters_that_do_not_tokenize_the_value |
|
| 819 |
query = IssueQuery.new( |
|
| 820 |
:name => '_', |
|
| 821 |
:filters => {
|
|
| 822 |
'subject' => {:operator => '*', :values => ['']},
|
|
| 823 |
'status_id' => {:operator => 'o', :values => ['']},
|
|
| 824 |
'parent_id' => {:operator => '~', :values => ['1,2,3,4,5,6,7']}
|
|
| 825 |
} |
|
| 826 |
) |
|
| 827 |
assert_equal [], query.filter_warnings |
|
| 828 | ||
| 829 |
user_query = UserQuery.new( |
|
| 830 |
:name => '_', |
|
| 831 |
:filters => {'login' => {:operator => '=', :values => ['one two three four five six']}}
|
|
| 832 |
) |
|
| 833 |
assert_equal [], user_query.filter_warnings |
|
| 834 |
end |
|
| 835 | ||
| 799 | 836 |
def test_operator_contains_any_of_with_any_searchable_text |
| 800 | 837 |
User.current = User.find(1) |
| 801 | 838 |
query = IssueQuery.new( |
- « Previous
- 1
- 2
- Next »