View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0026447 | mantisbt | filters | public | 2019-12-09 10:16 | 2023-01-07 13:22 |
Reporter | polzin | Assigned To | |||
Priority | high | Severity | feature | Reproducibility | N/A |
Status | acknowledged | Resolution | open | ||
Summary | 0026447: Enhancement of free text filter function: Search for custom fields, summary-only, id-only and mixing AND and OR. | ||||
Description | Attached is a patch for improved free text filter function. Examples: i:123 searches only for the bug id (useful if you have a list of IDs and create a search link with 'xargs -n 1 printf "i:%s OR "') Necessary improvements for incorporation:
| ||||
Tags | No tags attached. | ||||
Attached Files | filter_patch.txt (4,578 bytes)
diff --git a/core/classes/BugFilterQuery.class.php b/core/classes/BugFilterQuery.class.php index 110b5c5..ece5a2c 100644 --- a/core/classes/BugFilterQuery.class.php +++ b/core/classes/BugFilterQuery.class.php @@ -1401,22 +1401,32 @@ class BugFilterQuery extends DbQuery { } # break up search terms by spacing or quoting - preg_match_all( "/-?([^'\"\s]+|\"[^\"]+\"|'[^']+')/", $this->filter[FILTER_PROPERTY_SEARCH], $t_matches, PREG_SET_ORDER ); + # optional "s:, i:, c:" for faster summary only requests + preg_match_all( "/-?([sic]:)?([^'\"\s]+|\"[^\"]+\"|'[^']+')/", $this->filter[FILTER_PROPERTY_SEARCH], $t_matches, PREG_SET_ORDER ); - # organize terms without quoting, paying attention to negation - $t_search_terms = array(); - foreach( $t_matches as $t_match ) { - $t_search_terms[trim( $t_match[1], "\'\"" )] = ( $t_match[0][0] == '-' ); - } + $t_bug_table = $this->helper_table_alias_for_bugnote(); $t_bugnote_table = $this->helper_table_alias_for_bugnote(); # build a big where-clause and param list for all search terms, including negations $t_first = true; - $t_textsearch_where_clause = '( '; - foreach( $t_search_terms as $t_search_term => $t_negate ) { - if( !$t_first ) { - $t_textsearch_where_clause .= ' AND '; + $t_after_or = false; # if we are after an OR + $t_textsearch_where_clause = "( ( "; # second ( for OR + foreach( $t_matches as $t_match ) { + # analyse search term + $t_search_term = trim( $t_match[2], "\'\"" ); + $t_negate = ( $t_match[0][0] == '-' ); + $t_summary_only = ( $t_match[1] && $t_match[1][0] == 's' ); + $t_id_only = ( $t_match[1] && $t_match[1][0] == 'i' ); + $t_custom_field_only = ( $t_match[1] && $t_match[1][0] == 'c' ); + if ( !$t_first && !$t_after_or ) { + if ( $t_search_term == 'OR' ) { + $t_textsearch_where_clause .= ' ) OR ( '; + $t_after_or = true; + continue; + } else { + $t_textsearch_where_clause .= ' AND '; + } } if( $t_negate ) { @@ -1424,25 +1434,53 @@ class BugFilterQuery extends DbQuery { } $c_search = '%' . $t_search_term . '%'; - $t_textsearch_where_clause .= '( ' . $this->sql_like( '{bug}.summary', $c_search ) - . ' OR ' . $this->sql_like( '{bug_text}.description', $c_search ) - . ' OR ' . $this->sql_like( '{bug_text}.steps_to_reproduce', $c_search ) - . ' OR ' . $this->sql_like( '{bug_text}.additional_information', $c_search ) - . ' OR ' . $this->sql_like( '{bugnote_text}.note', $c_search ); + $t_textsearch_where_clause .= '( '; + # search in summary only if not id / custom_field_search + if ( !$t_id_only && !$t_custom_field_only ) { + $t_textsearch_where_clause .= $this->sql_like( '{bug}.summary', $c_search ); + } else { + $t_textsearch_where_clause .= '0'; + } + if ( !$t_summary_only && !$t_id_only && !$t_custom_field_only ) { + $t_textsearch_where_clause .= + ' OR ' . $this->sql_like( '{bug_text}.description', $c_search ) . + ' OR ' . $this->sql_like( '{bug_text}.steps_to_reproduce', $c_search ) . + ' OR ' . $this->sql_like( '{bug_text}.additional_information', $c_search ) . + ' OR ' . $this->sql_like( '{bugnote_text}.note', $c_search ); + } + - if( is_numeric( $t_search_term ) ) { + if( is_numeric( $t_search_term ) && !$t_summary_only && !$t_custom_field_only ) { # Note: no need to test negative values, '-' sign has been removed if( $t_search_term <= DB_MAX_INT ) { $c_search_int = (int)$t_search_term; $t_textsearch_where_clause .= ' OR {bug}.id = ' . $this->param( $c_search_int ); - $t_textsearch_where_clause .= ' OR ' . $t_bugnote_table . '.id = ' . $this->param( $c_search_int ); + # id search only in bugs, not in bugnotes + if ( !$t_id_only ) { + $t_textsearch_where_clause .= ' OR ' . $t_bugnote_table . '.id = ' . $this->param( $c_search_int ); + } } } + if ( $t_custom_field_only ) { + $t_textsearch_where_clause .= + ' OR ' . "({bug}.id IN ( SELECT DISTINCT bug_id from " . db_get_table( 'mantis_custom_field_string_table' ) . " where " . $this->sql_like( "value", $c_search )."))"; + } $t_textsearch_where_clause .= ' )'; $t_first = false; + $t_after_or = false; } - $t_textsearch_where_clause .= ' )'; + if ( $t_after_or ) { + # avoid SQL error with "XXX OR" + $t_textsearch_where_clause .= ' FALSE '; + } + $t_textsearch_where_clause .= ' ) )'; + + + log_event(LOG_FILTERING,'Clause'.$t_textsearch_where_clause); + + + # add text query elements to arrays if( !$t_first ) { | ||||
@polzin thanks for your contribution Would you mind submitting your patch as a pull request on Github, to make it easier to test and review ? |
|
Pull request see https://github.com/mantisbt/mantisbt/pull/1597 (updated) |
|
Thanks ! |
|
Work for me. Tested on 2.22.1 Do you mind if I add a suggestion? |
|
@dregad Do you have a suggestion, how to implement the end-user documentation? The placeholder has probably not enough space for a syntax description. The tooltip describing the syntax would be an easy addition. If that's wanted, I can do it. For the other improvement "possible extension to AND" , I checked the code and noticed that it was a mistake to think there is something missing. AND is the default, therefore no further extension is necessary. I would not use a drop-down, as suggested for @emil-tsl, because this is a different gui concept. |
|
I don't have time for a deeper look, but please consider my warning 0007183:0059789 and the discussion around previous PRs Especially the following comment concerning performance and security |
|
@polzin |
|
@emil-tsl You could do this using "c:TEST OR TEST" |
|
@atrol As the default behaviour is not changed, the performance is not an issue. In fact, for big repositories it is a relevant performance advantage to use "s:XXX" for a search only in the summary. |
|
I updated to pull request, so that it contains only one commit and changed the author.email. |
|
Yes concerning existing functionality. There is still the security aspect I mentioned in the old PR
|
|
@atrol Thanks for the feedback. The security problem is fixed and tested with a user being able to see the issue, but not the custom field. The performance is still okay in a setting with > 60000 issues and > 50 custom fields (where > 15 string custom fields are searched): |
|
I think performance may be relevant |
|
Previously, when this topic has been discussed, the proposals are inthe line of:
your proposal has the potential to do more complex searches, like selectively target different terms in each target: "s:something OR c:otherthing" right? With your scheme, should we have a mapping of the old searches (eg, stored filters), so that the old search for |
|
@cproensa: I am not sure if there is a misunderstanding: The current behaviour is not changed (if not a "[sci]:" or "OR" is used). If a user or a stored filter looked for By the way: Compared to the checkboxes, one very usefull sideeffect for power users is: You can use this browser shortcuts/keywords, such that |
|
ok, sorry, i thought that this searches within custom fields (along all other contexts) by default. so, would it make more sense for this to include custom fields by default?
|
|
@cproensa: Add custom filters by default would bring you: perfomance drops, changes of existing filters, more complex sql by default, all those things you probably don't want to have. :-) |
|
@atrol, @cproensa, @dregad: see 0026447:0063291, I think all security and performance issues are addressed now. How do we proceed? |
|
is it compatible with current version 2.24.1? |
|
@amphetamine |
|
file changed follow by |
|
@amphetamine
If yes, could you try my code and see if it gives the same result? |
|
no idea how to execute your code (filter_patch.txt) under XAMPP |
|
So, you basically took my code from github, without modifications, right.
|
|
problem solved for i:1 and i:10 (the issue can not be hide) found the root cause for c:search not found string custom fields have to add to filterif possible for those fields not add to filter can be searched as well |
|
I find it more consistant to search only those fields that are mentioned in filtered. But if this is does not suite you, you may remove the lines from |
|
Does this code still work? |
|
@materwm I use the code, but not with the newest version. Unfortunately, it was not merged into the main version. Here are the differences, you may try: https://github.com/mantisbt/mantisbt/pull/1597/files |
|
thank you polzin, in fact I changed the code however I cant see any change.! |
|