Fix: Searching for multiple terms on assets (#5860)

* Give advancedTextSearch all search terms at one

The additional conditions for assets had some problems, since they were joining tables for the additional attributes. The method was called once for every search term, so the join was added multiple times if the user entered multiple search terms.

* Allows search to handle multiple search terms better

The search now better handles multiple search terms, adding additional orWhere clauses, instead of duplicating all queries.

* Fixing typo
This commit is contained in:
Till Deeke 2018-07-17 02:44:31 +02:00 committed by snipe
parent 638a7b2d91
commit b5de5ac19c
3 changed files with 92 additions and 73 deletions

View file

@ -621,10 +621,12 @@ class Asset extends Depreciable
* Run additional, advanced searches.
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term The search term
* @param array $terms The search terms
* @return Illuminate\Database\Eloquent\Builder
*/
public function advancedTextSearch(Builder $query, string $term) {
public function advancedTextSearch(Builder $query, array $terms) {
/**
* Assigned user
*/
@ -632,31 +634,44 @@ class Asset extends Depreciable
$leftJoin->on("assets_users.id", "=", "assets.assigned_to")
->where("assets.assigned_type", "=", User::class);
});
$query = $query
->orWhere('assets_users.first_name', 'LIKE', '%'.$term.'%')
->orWhere('assets_users.last_name', 'LIKE', '%'.$term.'%')
->orWhere('assets_users.username', 'LIKE', '%'.$term.'%')
->orWhereRaw('CONCAT('.DB::getTablePrefix().'assets_users.first_name," ",'.DB::getTablePrefix().'assets_users.last_name) LIKE ?', ["%$term%", "%$term%"]);
foreach($terms as $term) {
$query = $query
->orWhere('assets_users.first_name', 'LIKE', '%'.$term.'%')
->orWhere('assets_users.last_name', 'LIKE', '%'.$term.'%')
->orWhere('assets_users.username', 'LIKE', '%'.$term.'%')
->orWhereRaw('CONCAT('.DB::getTablePrefix().'assets_users.first_name," ",'.DB::getTablePrefix().'assets_users.last_name) LIKE ?', ["%$term%", "%$term%"]);
}
/**
* Assigned location
*/
*/
$query = $query->leftJoin('locations as assets_locations',function ($leftJoin) {
$leftJoin->on("assets_locations.id","=","assets.assigned_to")
->where("assets.assigned_type","=",Location::class);
});
$query = $query->orWhere('assets_locations.name', 'LIKE', '%'.$term.'%');
foreach($terms as $term) {
$query = $query->orWhere('assets_locations.name', 'LIKE', '%'.$term.'%');
}
/**
* Assigned assets
*/
*/
$query = $query->leftJoin('assets as assigned_assets',function ($leftJoin) {
$leftJoin->on('assigned_assets.id', '=', 'assets.assigned_to')
->where('assets.assigned_type', '=', Asset::class);
});
$query = $query->orWhere('assigned_assets.name', 'LIKE', '%'.$term.'%');
foreach($terms as $term) {
$query = $query->orWhere('assigned_assets.name', 'LIKE', '%'.$term.'%');
}
return $query;
}

View file

@ -25,28 +25,25 @@ trait Searchable {
{
$terms = $this->prepeareSearchTerms($search);
foreach($terms as $term) {
/**
* Search the attributes of this model
*/
$query = $this->searchAttributes($query, $terms);
/**
* Search the attributes of this model
*/
$query = $this->searchAttributes($query, $term);
/**
* Search through the custom fields of the model
*/
$query = $this->searchCustomFields($query, $terms);
/**
* Search through the custom fields of the model
*/
$query = $this->searchCustomFields($query, $term);
/**
* Search through the relations of the model
*/
$query = $this->searchRelations($query, $terms);
/**
* Search through the relations of the model
*/
$query = $this->searchRelations($query, $term);
/**
* Search for additional attributes defined by the model
*/
$query = $this->advancedTextSearch($query, $term);
}
/**
* Search for additional attributes defined by the model
*/
$query = $this->advancedTextSearch($query, $terms);
return $query;
}
@ -61,41 +58,43 @@ trait Searchable {
}
/**
* Searches the models attributes for the search term
* Searches the models attributes for the search terms
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term
* @param array $terms
* @return Illuminate\Database\Eloquent\Builder
*/
private function searchAttributes(Builder $query, string $term) {
private function searchAttributes(Builder $query, array $terms) {
$table = $this->getTable();
foreach($this->getSearchableAttributes() as $column) {
/**
* Making sure to only search in date columns if the search term consists of characters that can make up a MySQL timestamp!
*
* @see https://github.com/snipe/snipe-it/issues/4590
*/
if (!preg_match('/^[0-9 :-]++$/', $term) && in_array($column, $this->getDates())) {
continue;
foreach($terms as $term) {
/**
* Making sure to only search in date columns if the search term consists of characters that can make up a MySQL timestamp!
*
* @see https://github.com/snipe/snipe-it/issues/4590
*/
if (!preg_match('/^[0-9 :-]++$/', $term) && in_array($column, $this->getDates())) {
continue;
}
$query = $query->orWhere($table . '.' . $column, 'LIKE', '%'.$term.'%');
}
$table = $this->getTable();
$query = $query->orWhere($table . '.' . $column, 'LIKE', '%'.$term.'%');
}
return $query;
}
/**
* Searches the models custom fields for the search term
* Searches the models custom fields for the search terms
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term
* @param array $terms
* @return Illuminate\Database\Eloquent\Builder
*/
private function searchCustomFields(Builder $query, string $term) {
private function searchCustomFields(Builder $query, array $terms) {
/**
* If we are searching on something other that an asset, skip custom fields.
@ -104,51 +103,53 @@ trait Searchable {
return $query;
}
foreach (CustomField::all() as $field) {
$query->orWhere($this->getTable() . '.'. $field->db_column_name(), 'LIKE', '%'.$term.'%');
$customFields = CustomField::all();
foreach ($customFields as $field) {
foreach($terms as $term) {
$query->orWhere($this->getTable() . '.'. $field->db_column_name(), 'LIKE', '%'.$term.'%');
}
}
return $query;
}
/**
* Searches the models relations for the search term
* Searches the models relations for the search terms
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term
* @param array $terms
* @return Illuminate\Database\Eloquent\Builder
*/
private function searchRelations(Builder $query, string $term) {
private function searchRelations(Builder $query, array $terms) {
foreach($this->getSearchableRelations() as $relation => $columns) {
/**
* Make the columns into a collection,
* for easier handling further down
*
* @var Illuminate\Support\Collection
*/
$columns = collect($columns);
$query = $query->orWhereHas($relation, function($query) use ($relation, $columns, $term) {
$query = $query->orWhereHas($relation, function($query) use ($relation, $columns, $terms) {
$table = $this->getRelationTable($relation);
/**
* We need to form the query properly, starting with a "where",
* otherwise the generated nested select is wrong.
*
* We can just choose the last column, since they all get "and where"d in the end.
* (And because using pop saves us from handling the removal of the first element)
* @todo This does the job, but is inelegant and fragile
*/
$last = $columns->pop();
$query->where($table . '.' . $last, 'LIKE', '%'.$term.'%');
$firstConditionAdded = false;
foreach($columns as $column) {
$query->orWhere($table . '.' . $column, 'LIKE', '%'.$term.'%');
foreach($terms as $term) {
if (!$firstConditionAdded) {
$query->where($table . '.' . $column, 'LIKE', '%'.$term.'%');
$firstConditionAdded = true;
continue;
}
$query->orWhere($table . '.' . $column, 'LIKE', '%'.$term.'%');
}
}
});
}
return $query;
@ -160,12 +161,12 @@ trait Searchable {
* This is a noop in this trait, but can be overridden in the implementing model, to allow more advanced searches
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term The search term
* @param array $terms The search terms
* @return Illuminate\Database\Eloquent\Builder
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function advancedTextSearch(Builder $query, string $term) {
public function advancedTextSearch(Builder $query, array $terms) {
return $query;
}

View file

@ -440,12 +440,15 @@ class User extends SnipeModel implements AuthenticatableContract, CanResetPasswo
* Run additional, advanced searches.
*
* @param Illuminate\Database\Eloquent\Builder $query
* @param string $term The search term
* @param array $term The search terms
* @return Illuminate\Database\Eloquent\Builder
*/
public function advancedTextSearch(Builder $query, string $term) {
$query = $query->orWhereRaw('CONCAT('.DB::getTablePrefix().'users.first_name," ",'.DB::getTablePrefix().'users.last_name) LIKE ?', ["%$term%", "%$term%"]);
public function advancedTextSearch(Builder $query, array $terms) {
foreach($terms as $term) {
$query = $query->orWhereRaw('CONCAT('.DB::getTablePrefix().'users.first_name," ",'.DB::getTablePrefix().'users.last_name) LIKE ?', ["%$term%", "%$term%"]);
}
return $query;
}