mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 00:32:42 +01:00
Custom integer fields: fix search by array of possible values
Summary: This minimal changes seems the natural expansion of this search function, that "seems" designed to only search with a single value, but the backend is designed to receive an array of possible values. Look at the same method in PhabricatorStandardCustomFieldLink that already works also for array inputs. To get extra confidence look at the method withApplicationSearchContainsConstraint() that works perfectly with arrays (to be honest it only works with arrays in mind...) This feature allows to avoid crashes when extension developers tries to run "IN" queries because for example they try to follow our performance guidelines: https://we.phorge.it/book/contrib/article/n_plus_one/ Closes T15752 Test Plan: Have a nice custom integer field, like this one for Maniphest: { "mycompany.estimated-hours": { "name": "Estimated Hours", "type": "int", "caption": "Estimated number of hours this will take.", "search": true } } You can reproduce T15752 before the change. It just works after the change. Or, just trust your instincts by looking at the same method in PhabricatorStandardCustomFieldLink. Also, use the "normal" frontend search page. Still works as usual. Reviewers: O1 Blessed Committers, 20after4 Reviewed By: O1 Blessed Committers, 20after4 Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15752 Differential Revision: https://we.phorge.it/D25554
This commit is contained in:
parent
48e121c485
commit
5263d5d590
1 changed files with 17 additions and 4 deletions
|
@ -48,15 +48,28 @@ final class PhabricatorStandardCustomFieldInt
|
||||||
return $request->getStr($this->getFieldKey());
|
return $request->getStr($this->getFieldKey());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Apply an application search constraint to a query.
|
||||||
|
* If you have a field of type integer, and a value (or an array of values),
|
||||||
|
* the result set will be limited to the rows with these values.
|
||||||
|
* @param PhabricatorApplicationSearchEngine $engine
|
||||||
|
* @param PhabricatorCursorPagedPolicyAwareQuery $query
|
||||||
|
* @param mixed $value Single value or array of values (IN query).
|
||||||
|
*/
|
||||||
public function applyApplicationSearchConstraintToQuery(
|
public function applyApplicationSearchConstraintToQuery(
|
||||||
PhabricatorApplicationSearchEngine $engine,
|
PhabricatorApplicationSearchEngine $engine,
|
||||||
PhabricatorCursorPagedPolicyAwareQuery $query,
|
PhabricatorCursorPagedPolicyAwareQuery $query,
|
||||||
$value) {
|
$value) {
|
||||||
|
|
||||||
if (phutil_nonempty_scalar($value)) {
|
// The basic use case is with a single value.
|
||||||
$query->withApplicationSearchContainsConstraint(
|
// The backend really works with an array. So also array allowed.
|
||||||
$this->newNumericIndex(null),
|
if (is_array($value) || phutil_nonempty_scalar($value)) {
|
||||||
$value);
|
$value = (array)$value;
|
||||||
|
if ($value) {
|
||||||
|
$query->withApplicationSearchContainsConstraint(
|
||||||
|
$this->newNumericIndex(null),
|
||||||
|
$value);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue