diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8539271141..e6eaec9be0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2349,6 +2349,8 @@ phutil_register_library_map(array( 'PhabricatorHovercardView' => 'view/widget/hovercard/PhabricatorHovercardView.php', 'PhabricatorHunksManagementMigrateWorkflow' => 'applications/differential/management/PhabricatorHunksManagementMigrateWorkflow.php', 'PhabricatorHunksManagementWorkflow' => 'applications/differential/management/PhabricatorHunksManagementWorkflow.php', + 'PhabricatorIDsSearchEngineExtension' => 'applications/search/engineextension/PhabricatorIDsSearchEngineExtension.php', + 'PhabricatorIDsSearchField' => 'applications/search/field/PhabricatorIDsSearchField.php', 'PhabricatorIRCProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php', 'PhabricatorIconRemarkupRule' => 'applications/macro/markup/PhabricatorIconRemarkupRule.php', 'PhabricatorImageMacroRemarkupRule' => 'applications/macro/markup/PhabricatorImageMacroRemarkupRule.php', @@ -2625,6 +2627,7 @@ phutil_register_library_map(array( 'PhabricatorPHIDResolver' => 'applications/phid/resolver/PhabricatorPHIDResolver.php', 'PhabricatorPHIDType' => 'applications/phid/type/PhabricatorPHIDType.php', 'PhabricatorPHIDTypeTestCase' => 'applications/phid/type/__tests__/PhabricatorPHIDTypeTestCase.php', + 'PhabricatorPHIDsSearchField' => 'applications/search/field/PhabricatorPHIDsSearchField.php', 'PhabricatorPHPASTApplication' => 'applications/phpast/application/PhabricatorPHPASTApplication.php', 'PhabricatorPHPConfigSetupCheck' => 'applications/config/check/PhabricatorPHPConfigSetupCheck.php', 'PhabricatorPHPMailerConfigOptions' => 'applications/config/option/PhabricatorPHPMailerConfigOptions.php', @@ -6539,6 +6542,8 @@ phutil_register_library_map(array( 'PhabricatorHovercardView' => 'AphrontView', 'PhabricatorHunksManagementMigrateWorkflow' => 'PhabricatorHunksManagementWorkflow', 'PhabricatorHunksManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'PhabricatorIDsSearchEngineExtension' => 'PhabricatorSearchEngineExtension', + 'PhabricatorIDsSearchField' => 'PhabricatorSearchField', 'PhabricatorIRCProtocolAdapter' => 'PhabricatorProtocolAdapter', 'PhabricatorIconRemarkupRule' => 'PhutilRemarkupRule', 'PhabricatorImageMacroRemarkupRule' => 'PhutilRemarkupRule', @@ -6847,6 +6852,7 @@ phutil_register_library_map(array( 'PhabricatorPHIDResolver' => 'Phobject', 'PhabricatorPHIDType' => 'Phobject', 'PhabricatorPHIDTypeTestCase' => 'PhutilTestCase', + 'PhabricatorPHIDsSearchField' => 'PhabricatorSearchField', 'PhabricatorPHPASTApplication' => 'PhabricatorApplication', 'PhabricatorPHPConfigSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorPHPMailerConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/conduit/parametertype/ConduitIntListParameterType.php b/src/applications/conduit/parametertype/ConduitIntListParameterType.php index a2f4c95534..07c87dcd8a 100644 --- a/src/applications/conduit/parametertype/ConduitIntListParameterType.php +++ b/src/applications/conduit/parametertype/ConduitIntListParameterType.php @@ -18,7 +18,7 @@ final class ConduitIntListParameterType } } - return $this->validateIntList($request, $key, $list); + return $list; } protected function getParameterTypeName() { diff --git a/src/applications/conduit/parametertype/ConduitParameterType.php b/src/applications/conduit/parametertype/ConduitParameterType.php index 2c67886f3e..8f02057c0f 100644 --- a/src/applications/conduit/parametertype/ConduitParameterType.php +++ b/src/applications/conduit/parametertype/ConduitParameterType.php @@ -26,7 +26,7 @@ abstract class ConduitParameterType extends Phobject { final public function getExists(array $request, $key) { - return $this->getValueExists($request, $key); + return $this->getParameterExists($request, $key); } diff --git a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php index 70aa1891c7..f3ba5d1694 100644 --- a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php +++ b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php @@ -10,7 +10,7 @@ final class ConduitResultSearchEngineExtension } public function getExtensionOrder() { - return 1000; + return 1500; } public function getExtensionName() { diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index 2165111e3d..8355e8947e 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -94,9 +94,6 @@ final class ManiphestTaskSearchEngine ->setLabel(pht('Group By')) ->setKey('group') ->setOptions($this->getGroupOptions()), - id(new PhabricatorSearchStringListField()) - ->setLabel(pht('Task IDs')) - ->setKey('ids'), id(new PhabricatorSearchDateField()) ->setLabel(pht('Created After')) ->setKey('createdStart'), diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index e8d115d9a2..67caaa1035 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -1070,27 +1070,9 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { // These are handled separately for Conduit, so don't show them as // supported. - unset($fields['ids']); - unset($fields['phids']); unset($fields['order']); unset($fields['limit']); - // TODO: Clean these up, shortly. - $fields = array( - 'ids' => id(new PhabricatorSearchDatasourceField()) - ->setKey('ids') - ->setLabel(pht('IDs')) - ->setDescription( - pht('Search for objects with specific IDs.')) - ->setConduitParameterType(new ConduitIntListParameterType()), - 'phids' => id(new PhabricatorSearchDatasourceField()) - ->setKey('phids') - ->setLabel(pht('PHIDs')) - ->setDescription( - pht('Search for objects with specific PHIDs.')) - ->setConduitParameterType(new ConduitPHIDListParameterType()), - ) + $fields; - $viewer = $this->requireViewer(); foreach ($fields as $key => $field) { $field->setViewer($viewer); @@ -1145,7 +1127,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $query = $this->buildQueryFromSavedQuery($saved_query); $pager = $this->newPagerForSavedQuery($saved_query); - $this->setAutomaticConstraintsForConduit($query, $request, $constraints); $this->setQueryOrderForConduit($query, $request); $this->setPagerLimitForConduit($pager, $request); $this->setPagerOffsetsForConduit($pager, $request); @@ -1222,22 +1203,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { return $extensions; } - private function setAutomaticConstraintsForConduit( - $query, - ConduitAPIRequest $request, - array $constraints) { - - $with_ids = idx($constraints, 'ids'); - if ($with_ids) { - $query->withIDs($with_ids); - } - - $with_phids = idx($constraints, 'phids'); - if ($with_phids) { - $query->withPHIDs($with_phids); - } - } - private function setQueryOrderForConduit($query, ConduitAPIRequest $request) { $order = $request->getValue('order'); if ($order === null) { diff --git a/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php b/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php index df46325079..051061d34c 100644 --- a/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php +++ b/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php @@ -163,6 +163,13 @@ EOTEXT $fields = $engine->getSearchFieldsForConduit(); + // As a convenience, put these fields at the very top, even if the engine + // specifies and alternate display order for the web UI. These fields are + // very important in the API and nearly useless in the web UI. + $fields = array_select_keys( + $fields, + array('ids', 'phids')) + $fields; + $rows = array(); foreach ($fields as $field) { $key = $field->getConduitKey(); diff --git a/src/applications/search/engineextension/PhabricatorIDsSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorIDsSearchEngineExtension.php new file mode 100644 index 0000000000..6247ba2825 --- /dev/null +++ b/src/applications/search/engineextension/PhabricatorIDsSearchEngineExtension.php @@ -0,0 +1,55 @@ +setKey('ids') + ->setLabel(pht('IDs')) + ->setDescription( + pht('Search for objects with specific IDs.')), + id(new PhabricatorPHIDsSearchField()) + ->setKey('phids') + ->setLabel(pht('PHIDs')) + ->setDescription( + pht('Search for objects with specific PHIDs.')), + ); + } + + public function applyConstraintsToQuery( + $object, + $query, + PhabricatorSavedQuery $saved, + array $map) { + + if ($map['ids']) { + $query->withIDs($map['ids']); + } + + if ($map['phids']) { + $query->withPHIDs($map['phids']); + } + + } + +} diff --git a/src/applications/search/field/PhabricatorIDsSearchField.php b/src/applications/search/field/PhabricatorIDsSearchField.php new file mode 100644 index 0000000000..909cce9c4a --- /dev/null +++ b/src/applications/search/field/PhabricatorIDsSearchField.php @@ -0,0 +1,30 @@ +getStrList($key); + } + + protected function newControl() { + if (strlen($this->getValueForControl())) { + return new AphrontFormTextControl(); + } else { + return null; + } + } + + protected function getValueForControl() { + return implode(', ', parent::getValueForControl()); + } + + protected function newConduitParameterType() { + return new ConduitIntListParameterType(); + } + +} diff --git a/src/applications/search/field/PhabricatorPHIDsSearchField.php b/src/applications/search/field/PhabricatorPHIDsSearchField.php new file mode 100644 index 0000000000..459233c472 --- /dev/null +++ b/src/applications/search/field/PhabricatorPHIDsSearchField.php @@ -0,0 +1,30 @@ +getStrList($key); + } + + protected function newControl() { + if (strlen($this->getValueForControl())) { + return new AphrontFormTextControl(); + } else { + return null; + } + } + + protected function getValueForControl() { + return implode(', ', parent::getValueForControl()); + } + + protected function newConduitParameterType() { + return new ConduitPHIDListParameterType(); + } + +} diff --git a/src/applications/search/field/PhabricatorSearchField.php b/src/applications/search/field/PhabricatorSearchField.php index 380b574f17..9624fe22f9 100644 --- a/src/applications/search/field/PhabricatorSearchField.php +++ b/src/applications/search/field/PhabricatorSearchField.php @@ -273,17 +273,26 @@ abstract class PhabricatorSearchField extends Phobject { protected function renderControl() { + $control = $this->newControl(); + + if (!$control) { + return null; + } + // TODO: We should `setError($this->getShortError())` here, but it looks // terrible in the form layout. - return $this->newControl() + return $control ->setValue($this->getValueForControl()) ->setName($this->getKey()) ->setLabel($this->getLabel()); } public function appendToForm(AphrontFormView $form) { - $form->appendControl($this->renderControl()); + $control = $this->renderControl(); + if ($control !== null) { + $form->appendControl($this->renderControl()); + } return $this; }