diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 37794cfb81..d1233b3684 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '5a4a5010', + 'core.pkg.css' => 'c69171e6', 'core.pkg.js' => '73a06a9f', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '0b037a4f', @@ -155,7 +155,7 @@ return array( 'rsrc/css/phui/phui-form-view.css' => '01b796c0', 'rsrc/css/phui/phui-form.css' => '159e2d9c', 'rsrc/css/phui/phui-head-thing.css' => 'd7f293df', - 'rsrc/css/phui/phui-header-view.css' => '285c9139', + 'rsrc/css/phui/phui-header-view.css' => 'b500eeea', 'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0', 'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec', 'rsrc/css/phui/phui-icon.css' => '4cbc684a', @@ -168,6 +168,7 @@ return array( 'rsrc/css/phui/phui-object-box.css' => 'f434b6be', 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', + 'rsrc/css/phui/phui-policy-section-view.css' => '139fdc64', 'rsrc/css/phui/phui-property-list-view.css' => 'cad62236', 'rsrc/css/phui/phui-remarkup-preview.css' => '91767007', 'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370', @@ -178,7 +179,7 @@ return array( 'rsrc/css/phui/phui-two-column-view.css' => '01e6991e', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308', 'rsrc/css/phui/workboards/phui-workboard.css' => '74fc9d98', - 'rsrc/css/phui/workboards/phui-workcard.css' => '9e9eb0df', + 'rsrc/css/phui/workboards/phui-workcard.css' => '913441b6', 'rsrc/css/phui/workboards/phui-workpanel.css' => '3ae89b20', 'rsrc/css/sprite-login.css' => '18b368a6', 'rsrc/css/sprite-tokens.css' => 'f1896dc5', @@ -396,7 +397,7 @@ return array( 'rsrc/js/application/files/behavior-icon-composer.js' => '38a6cedb', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => 'a17b84f1', 'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => 'b347a301', - 'rsrc/js/application/herald/HeraldRuleEditor.js' => '27daef73', + 'rsrc/js/application/herald/HeraldRuleEditor.js' => '2633bef7', 'rsrc/js/application/herald/PathTypeahead.js' => 'ad486db3', 'rsrc/js/application/herald/herald-rule-editor.js' => '0922e81d', 'rsrc/js/application/maniphest/behavior-batch-selector.js' => '139ef688', @@ -570,7 +571,7 @@ return array( 'global-drag-and-drop-css' => '1d2713a4', 'harbormaster-css' => '8dfe16b2', 'herald-css' => '648d39e2', - 'herald-rule-editor' => '27daef73', + 'herald-rule-editor' => '2633bef7', 'herald-test-css' => 'e004176f', 'inline-comment-summary-css' => '81eb368d', 'javelin-aphlict' => '022516b4', @@ -842,7 +843,7 @@ return array( 'phui-form-css' => '159e2d9c', 'phui-form-view-css' => '01b796c0', 'phui-head-thing-view-css' => 'd7f293df', - 'phui-header-view-css' => '285c9139', + 'phui-header-view-css' => 'b500eeea', 'phui-hovercard' => '074f0783', 'phui-hovercard-view-css' => '6ca90fa0', 'phui-icon-set-selector-css' => '7aa5f3ec', @@ -863,6 +864,7 @@ return array( 'phui-oi-simple-ui-css' => '6a30fa46', 'phui-pager-css' => 'd022c7ad', 'phui-pinboard-view-css' => '1f08f5d8', + 'phui-policy-section-view-css' => '139fdc64', 'phui-property-list-view-css' => 'cad62236', 'phui-remarkup-preview-css' => '91767007', 'phui-segment-bar-view-css' => '5166b370', @@ -874,7 +876,7 @@ return array( 'phui-two-column-view-css' => '01e6991e', 'phui-workboard-color-css' => 'e86de308', 'phui-workboard-view-css' => '74fc9d98', - 'phui-workcard-view-css' => '9e9eb0df', + 'phui-workcard-view-css' => '913441b6', 'phui-workpanel-view-css' => '3ae89b20', 'phuix-action-list-view' => 'c68f183f', 'phuix-action-view' => 'aaa08f3b', @@ -1115,7 +1117,7 @@ return array( 'javelin-json', 'phabricator-draggable-list', ), - '27daef73' => array( + '2633bef7' => array( 'multirow-row-manager', 'javelin-install', 'javelin-util', diff --git a/resources/sql/autopatches/20190909.herald.01.rebuild.php b/resources/sql/autopatches/20190909.herald.01.rebuild.php new file mode 100644 index 0000000000..a29b7d2f45 --- /dev/null +++ b/resources/sql/autopatches/20190909.herald.01.rebuild.php @@ -0,0 +1,3 @@ + 'applications/settings/panel/PhabricatorEditEngineSettingsPanel.php', 'PhabricatorEditEngineStaticCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineStaticCommentAction.php', 'PhabricatorEditEngineSubtype' => 'applications/transactions/editengine/PhabricatorEditEngineSubtype.php', + 'PhabricatorEditEngineSubtypeHeraldField' => 'applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php', 'PhabricatorEditEngineSubtypeInterface' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeInterface.php', 'PhabricatorEditEngineSubtypeMap' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php', 'PhabricatorEditEngineSubtypeTestCase' => 'applications/transactions/editengine/__tests__/PhabricatorEditEngineSubtypeTestCase.php', @@ -3438,8 +3439,10 @@ phutil_register_library_map(array( 'PhabricatorFlagDeleteController' => 'applications/flag/controller/PhabricatorFlagDeleteController.php', 'PhabricatorFlagDestructionEngineExtension' => 'applications/flag/engineextension/PhabricatorFlagDestructionEngineExtension.php', 'PhabricatorFlagEditController' => 'applications/flag/controller/PhabricatorFlagEditController.php', + 'PhabricatorFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagHeraldAction.php', 'PhabricatorFlagListController' => 'applications/flag/controller/PhabricatorFlagListController.php', 'PhabricatorFlagQuery' => 'applications/flag/query/PhabricatorFlagQuery.php', + 'PhabricatorFlagRemoveFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php', 'PhabricatorFlagSearchEngine' => 'applications/flag/query/PhabricatorFlagSearchEngine.php', 'PhabricatorFlagSelectControl' => 'applications/flag/view/PhabricatorFlagSelectControl.php', 'PhabricatorFlaggableInterface' => 'applications/flag/interface/PhabricatorFlaggableInterface.php', @@ -4193,8 +4196,10 @@ phutil_register_library_map(array( 'PhabricatorPolicyManagementWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementWorkflow.php', 'PhabricatorPolicyPHIDTypePolicy' => 'applications/policy/phid/PhabricatorPolicyPHIDTypePolicy.php', 'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php', + 'PhabricatorPolicyRef' => 'applications/policy/view/PhabricatorPolicyRef.php', 'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php', 'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php', + 'PhabricatorPolicyRulesView' => 'applications/policy/view/PhabricatorPolicyRulesView.php', 'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php', 'PhabricatorPolicyStrengthConstants' => 'applications/policy/constants/PhabricatorPolicyStrengthConstants.php', 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php', @@ -9546,6 +9551,7 @@ phutil_register_library_map(array( 'PhabricatorEditEngineSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorEditEngineStaticCommentAction' => 'PhabricatorEditEngineCommentAction', 'PhabricatorEditEngineSubtype' => 'Phobject', + 'PhabricatorEditEngineSubtypeHeraldField' => 'HeraldField', 'PhabricatorEditEngineSubtypeMap' => 'Phobject', 'PhabricatorEditEngineSubtypeTestCase' => 'PhabricatorTestCase', 'PhabricatorEditEngineSubtypeTransaction' => 'PhabricatorEditEngineTransactionType', @@ -9799,7 +9805,7 @@ phutil_register_library_map(array( 'PhabricatorFlagDAO', 'PhabricatorPolicyInterface', ), - 'PhabricatorFlagAddFlagHeraldAction' => 'HeraldAction', + 'PhabricatorFlagAddFlagHeraldAction' => 'PhabricatorFlagHeraldAction', 'PhabricatorFlagColor' => 'PhabricatorFlagConstants', 'PhabricatorFlagConstants' => 'Phobject', 'PhabricatorFlagController' => 'PhabricatorController', @@ -9807,8 +9813,10 @@ phutil_register_library_map(array( 'PhabricatorFlagDeleteController' => 'PhabricatorFlagController', 'PhabricatorFlagDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', 'PhabricatorFlagEditController' => 'PhabricatorFlagController', + 'PhabricatorFlagHeraldAction' => 'HeraldAction', 'PhabricatorFlagListController' => 'PhabricatorFlagController', 'PhabricatorFlagQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorFlagRemoveFlagHeraldAction' => 'PhabricatorFlagHeraldAction', 'PhabricatorFlagSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorFlagSelectControl' => 'AphrontFormControl', 'PhabricatorFlaggableInterface' => 'PhabricatorPHIDInterface', @@ -10671,8 +10679,10 @@ phutil_register_library_map(array( 'PhabricatorPolicyManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType', 'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorPolicyRef' => 'Phobject', 'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorPolicyRule' => 'Phobject', + 'PhabricatorPolicyRulesView' => 'AphrontView', 'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension', 'PhabricatorPolicyStrengthConstants' => 'PhabricatorPolicyConstants', 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php index 2cbcb3cee0..de9a9e8b54 100644 --- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php +++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php @@ -50,30 +50,47 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck { if (!in_array('STRICT_ALL_TABLES', $modes)) { $summary = pht( 'MySQL is not in strict mode (on host "%s"), but using strict mode '. - 'is strongly encouraged.', + 'is recommended.', $host_name); $message = pht( - "On database host \"%s\", the global %s is not set to %s. ". - "It is strongly encouraged that you enable this mode when running ". - "Phabricator.\n\n". - "By default MySQL will silently ignore some types of errors, which ". - "can cause data loss and raise security concerns. Enabling strict ". - "mode makes MySQL raise an explicit error instead, and prevents this ". - "entire class of problems from doing any damage.\n\n". - "You can find more information about this mode (and how to configure ". - "it) in the MySQL manual. Usually, it is sufficient to add this to ". - "your %s file (in the %s section) and then restart %s:\n\n". - "%s\n". - "(Note that if you run other applications against the same database, ". - "they may not work in strict mode. Be careful about enabling it in ". - "these cases.)", + 'On database host "%s", the global "sql_mode" setting does not '. + 'include the "STRICT_ALL_TABLES" mode. Enabling this mode is '. + 'recommended to generally improve how MySQL handles certain errors.'. + "\n\n". + 'Without this mode enabled, MySQL will silently ignore some error '. + 'conditions, including inserts which attempt to store more data in '. + 'a column than actually fits. This behavior is usually undesirable '. + 'and can lead to data corruption (by truncating multibyte characters '. + 'in the middle), data loss (by discarding the data which does not '. + 'fit into the column), or security concerns (for example, by '. + 'truncating keys or credentials).'. + "\n\n". + 'Phabricator is developed and tested in "STRICT_ALL_TABLES" mode so '. + 'you should normally never encounter these situations, but may run '. + 'into them if you interact with the database directly, run '. + 'third-party code, develop extensions, or just encounter a bug in '. + 'the software.'. + "\n\n". + 'Enabling "STRICT_ALL_TABLES" makes MySQL raise an explicit error '. + 'if one of these unusual situations does occur. This is a safer '. + 'behavior and prevents these situations from causing secret, subtle, '. + 'and potentially serious issues later on.'. + "\n\n". + 'You can find more information about this mode (and how to configure '. + 'it) in the MySQL manual. Usually, it is sufficient to add this to '. + 'your "my.cnf" file (in the "[mysqld]" section) and then '. + 'restart "mysqld":'. + "\n\n". + '%s'. + "\n". + 'Note that if you run other applications against the same database, '. + 'they may not work in strict mode.'. + "\n\n". + 'If you can not or do not want to enable "STRICT_ALL_TABLES", you '. + 'can safely ignore this warning. Phabricator will work correctly '. + 'with this mode enabled or disabled.', $host_name, - phutil_tag('tt', array(), 'sql_mode'), - phutil_tag('tt', array(), 'STRICT_ALL_TABLES'), - phutil_tag('tt', array(), 'my.cnf'), - phutil_tag('tt', array(), '[mysqld]'), - phutil_tag('tt', array(), 'mysqld'), phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES')); $this->newIssue('sql_mode.strict') diff --git a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php index e4ae03915f..57772ecb5f 100644 --- a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php +++ b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php @@ -1,6 +1,7 @@ getAdapter()->getPHID(); $rule = $effect->getRule(); diff --git a/src/applications/flag/herald/PhabricatorFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php new file mode 100644 index 0000000000..6f87a4ee6c --- /dev/null +++ b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php @@ -0,0 +1,18 @@ +getAdapter()->getPHID(); + $rule = $effect->getRule(); + $author = $rule->getAuthor(); + + $flag = PhabricatorFlagQuery::loadUserFlag($author, $phid); + if (!$flag) { + $this->logEffect(self::DO_IGNORE_UNFLAG, null); + return; + } + + if ($flag->getColor() !== $effect->getTarget()) { + $this->logEffect(self::DO_IGNORE_UNFLAG, $flag->getColor()); + return; + } + + $flag->delete(); + + $this->logEffect(self::DO_UNFLAG, $flag->getColor()); + } + + public function getHeraldActionValueType() { + return id(new HeraldSelectFieldValue()) + ->setKey('flag.color') + ->setOptions(PhabricatorFlagColor::getColorNameMap()) + ->setDefault(PhabricatorFlagColor::COLOR_BLUE); + } + + protected function getActionEffectMap() { + return array( + self::DO_IGNORE_UNFLAG => array( + 'icon' => 'fa-times', + 'color' => 'grey', + 'name' => pht('Did Not Remove Flag'), + ), + self::DO_UNFLAG => array( + 'icon' => 'fa-flag', + 'name' => pht('Removed Flag'), + ), + ); + } + + public function renderActionDescription($value) { + $color = PhabricatorFlagColor::getColorName($value); + return pht('Remove %s flag.', $color); + } + + protected function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_IGNORE_UNFLAG: + if (!$data) { + return pht('Not marked with any flag.'); + } else { + return pht( + 'Marked with flag of the wrong color ("%s").', + PhabricatorFlagColor::getColorName($data)); + } + case self::DO_UNFLAG: + return pht( + 'Removed "%s" flag.', + PhabricatorFlagColor::getColorName($data)); + } + } + +} diff --git a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php index 9fc053e8ae..6934a2e3d9 100644 --- a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php +++ b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php @@ -96,4 +96,8 @@ final class HarbormasterRunBuildPlansHeraldAction return $record->getTarget(); } + public function isActionAvailable() { + return id(new PhabricatorHarbormasterApplication())->isInstalled(); + } + } diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php index a9740d1736..d914c97a1c 100644 --- a/src/applications/herald/action/HeraldAction.php +++ b/src/applications/herald/action/HeraldAction.php @@ -405,4 +405,8 @@ abstract class HeraldAction extends Phobject { return array(); } + public function isActionAvailable() { + return true; + } + } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index cb8545b71d..2832c6d9f4 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -373,6 +373,16 @@ abstract class HeraldAdapter extends Phobject { return $field->getFieldGroupKey(); } + public function isFieldAvailable($field_key) { + $field = $this->getFieldImplementation($field_key); + + if (!$field) { + return null; + } + + return $field->isFieldAvailable(); + } + /* -( Conditions )--------------------------------------------------------- */ @@ -765,6 +775,16 @@ abstract class HeraldAdapter extends Phobject { return $action->getActionGroupKey(); } + public function isActionAvailable($action_key) { + $action = $this->getActionImplementation($action_key); + + if (!$action) { + return null; + } + + return $action->isActionAvailable(); + } + public function getActions($rule_type) { $actions = array(); foreach ($this->getActionsForRuleType($rule_type) as $key => $action) { diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index d05ed2d525..5e13522b6e 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -404,8 +404,8 @@ final class HeraldRuleController extends HeraldController { HeraldAdapter $adapter) { $all_rules = $this->loadRulesThisRuleMayDependUpon($rule); - $all_rules = mpull($all_rules, 'getName', 'getPHID'); - asort($all_rules); + $all_rules = msortv($all_rules, 'getEditorSortVector'); + $all_rules = mpull($all_rules, 'getEditorDisplayName', 'getPHID'); $all_fields = $adapter->getFieldNameMap(); $all_conditions = $adapter->getConditionNameMap(); @@ -674,15 +674,6 @@ final class HeraldRuleController extends HeraldController { ->execute(); } - // mark disabled rules as disabled since they are not useful as such; - // don't filter though to keep edit cases sane / expected - foreach ($all_rules as $current_rule) { - if ($current_rule->getIsDisabled()) { - $current_rule->makeEphemeral(); - $current_rule->setName($rule->getName().' '.pht('(Disabled)')); - } - } - // A rule can not depend upon itself. unset($all_rules[$rule->getID()]); @@ -693,7 +684,10 @@ final class HeraldRuleController extends HeraldController { $group_map = array(); foreach ($field_map as $field_key => $field_name) { $group_key = $adapter->getFieldGroupKey($field_key); - $group_map[$group_key][$field_key] = $field_name; + $group_map[$group_key][$field_key] = array( + 'name' => $field_name, + 'available' => $adapter->isFieldAvailable($field_key), + ); } return $this->getGroups( @@ -705,7 +699,10 @@ final class HeraldRuleController extends HeraldController { $group_map = array(); foreach ($action_map as $action_key => $action_name) { $group_key = $adapter->getActionGroupKey($action_key); - $group_map[$group_key][$action_key] = $action_name; + $group_map[$group_key][$action_key] = array( + 'name' => $action_name, + 'available' => $adapter->isActionAvailable($action_key), + ); } return $this->getGroups( diff --git a/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php b/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php index 5df229ccd6..ca28116d4c 100644 --- a/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php +++ b/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php @@ -37,6 +37,20 @@ final class HeraldRuleIndexEngineExtension $phids = array(); + $fields = HeraldField::getAllFields(); + foreach ($rule->getConditions() as $condition_record) { + $field = idx($fields, $condition_record->getFieldName()); + + if (!$field) { + continue; + } + + $affected_phids = $field->getPHIDsAffectedByCondition($condition_record); + foreach ($affected_phids as $phid) { + $phids[] = $phid; + } + } + $actions = HeraldAction::getAllActions(); foreach ($rule->getActions() as $action_record) { $action = idx($actions, $action_record->getAction()); diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index 611ea2cdb3..2a1e89d558 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -176,6 +176,29 @@ abstract class HeraldField extends Phobject { return $value_type->renderEditorValue($value); } + public function getPHIDsAffectedByCondition(HeraldCondition $condition) { + try { + $standard_type = $this->getHeraldFieldStandardType(); + } catch (PhutilMethodNotImplementedException $ex) { + $standard_type = null; + } + + switch ($standard_type) { + case self::STANDARD_PHID: + case self::STANDARD_PHID_NULLABLE: + case self::STANDARD_PHID_LIST: + $phids = $condition->getValue(); + + if (!is_array($phids)) { + $phids = array(); + } + + return $phids; + } + + return array(); + } + final public function setAdapter(HeraldAdapter $adapter) { $this->adapter = $adapter; return $this; @@ -218,4 +241,8 @@ abstract class HeraldField extends Phobject { return false; } + public function isFieldAvailable() { + return true; + } + } diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php index a9c131e717..07be991bda 100644 --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -259,6 +259,22 @@ final class HeraldRule extends HeraldDAO return '/'.$this->getMonogram(); } + public function getEditorSortVector() { + return id(new PhutilSortVector()) + ->addInt($this->getIsDisabled() ? 1 : 0) + ->addString($this->getName()); + } + + public function getEditorDisplayName() { + $name = pht('%s %s', $this->getMonogram(), $this->getName()); + + if ($this->getIsDisabled()) { + $name = pht('%s (Disabled)', $name); + } + + return $name; + } + /* -( Repetition Policies )------------------------------------------------ */ diff --git a/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php index 7ff69d37d5..b477c2ce35 100644 --- a/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php +++ b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php @@ -130,4 +130,9 @@ final class LegalpadRequireSignatureHeraldAction 'Require document signatures: %s.', $this->renderHandleList($value)); } + + public function isActionAvailable() { + return id(new PhabricatorLegalpadApplication())->isInstalled(); + } + } diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index 077fc511ce..60a41a26ca 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -344,6 +344,8 @@ dictionary with these keys: - `children` //Optional map.// Configure options shown to the user when they "Create Subtask". See below. - `fields` //Optional map.// Configure field behaviors. See below. + - `mutations` //Optional list.// Configure which subtypes this subtype + can easily be converted to by using the "Change Subtype" action. See below. Each subtype must have a unique key, and you must define a subtype with the key "%s", which is used as a default subtype. @@ -404,15 +406,15 @@ The `fields` key can configure the behavior of custom fields on specific task subtypes. For example: ``` -{ - ... - "fields": { - "custom.some-field": { - "disabled": true + { + ... + "fields": { + "custom.some-field": { + "disabled": true + } } + ... } - ... -} ``` Each field supports these options: @@ -421,6 +423,31 @@ Each field supports these options: subtypes. - `name` //Optional string.// Custom name of this field for the subtype. + +The `mutations` key allows you to control the behavior of the "Change Subtype" +action above the comment area. By default, this action allows users to change +the task subtype into any other subtype. + +If you'd prefer to make it more difficult to change subtypes or offer only a +subset of subtypes, you can specify the list of subtypes that "Change Subtypes" +offers. For example, if you have several similar subtypes and want to allow +tasks to be converted between them but not easily converted to other types, +you can make the "Change Subtypes" control show only these options like this: + +``` + { + ... + "mutations": ["bug", "issue", "defect"] + ... + } +``` + +If you specify an empty list, the "Change Subtypes" action will be completely +hidden. + +This mutation list is advisory and only configures the UI. Tasks may still be +converted across subtypes freely by using the Bulk Editor or API. + EOTEXT , $subtype_default_key)); diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index d2700895ce..c56d8fe57a 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -564,7 +564,8 @@ final class ManiphestTask extends ManiphestDAO public function newEditEngineSubtypeMap() { $config = PhabricatorEnv::getEnvConfig('maniphest.subtypes'); - return PhabricatorEditEngineSubtype::newSubtypeMap($config); + return PhabricatorEditEngineSubtype::newSubtypeMap($config) + ->setDatasource(new ManiphestTaskSubtypeDatasource()); } diff --git a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php index 147aa9635f..16d8dd9315 100644 --- a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php +++ b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php @@ -26,7 +26,17 @@ final class PhabricatorDatasourceURIEngineExtension ->setProtocol(null) ->setPort(null); - return phutil_string_cast($uri); + $uri = phutil_string_cast($uri); + + // See T13412. If the URI was in the form "http://dev.example.com" with + // no trailing slash, there may be no path. Redirecting to the empty + // string is considered an error by safety checks during redirection, + // so treat this like the user entered the URI with a trailing slash. + if (!strlen($uri)) { + $uri = '/'; + } + + return $uri; } return null; diff --git a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php index 5364a3d4fa..94e1cc495d 100644 --- a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php +++ b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php @@ -52,28 +52,22 @@ final class PhabricatorApplicationPolicyChangeTransaction } public function getTitle() { - $old = $this->renderApplicationPolicy($this->getOldValue()); - $new = $this->renderApplicationPolicy($this->getNewValue()); - return pht( - '%s changed the "%s" policy from "%s" to "%s".', + '%s changed the %s policy from %s to %s.', $this->renderAuthor(), $this->renderCapability(), - $old, - $new); + $this->renderOldPolicy(), + $this->renderNewPolicy()); } public function getTitleForFeed() { - $old = $this->renderApplicationPolicy($this->getOldValue()); - $new = $this->renderApplicationPolicy($this->getNewValue()); - return pht( - '%s changed the "%s" policy for application %s from "%s" to "%s".', + '%s changed the %s policy for application %s from %s to %s.', $this->renderAuthor(), $this->renderCapability(), $this->renderObject(), - $old, - $new); + $this->renderOldPolicy(), + $this->renderNewPolicy()); } public function validateTransactions($object, array $xactions) { @@ -165,38 +159,11 @@ final class PhabricatorApplicationPolicyChangeTransaction return $errors; } - private function renderApplicationPolicy($name) { - $policies = $this->getAllPolicies(); - if (empty($policies[$name])) { - // Not a standard policy, check for a custom policy. - $policy = id(new PhabricatorPolicyQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs(array($name)) - ->executeOne(); - $policies[$name] = $policy; - } - - $policy = idx($policies, $name); - return $this->renderValue($policy->getFullName()); - } - - private function getAllPolicies() { - if (!$this->policies) { - $viewer = $this->getViewer(); - $application = $this->getObject(); - $this->policies = id(new PhabricatorPolicyQuery()) - ->setViewer($viewer) - ->setObject($application) - ->execute(); - } - - return $this->policies; - } - private function renderCapability() { $application = $this->getObject(); $capability = $this->getCapabilityName(); - return $application->getCapabilityLabel($capability); + $label = $application->getCapabilityLabel($capability); + return $this->renderValue($label); } private function getCapabilityName() { diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index aabb3821e0..6688bef285 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -190,14 +190,6 @@ final class PassphraseCredentialViewController extends PassphraseController { pht('Credential Type'), $type->getCredentialTypeName()); - $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $viewer, - $credential); - - $properties->addProperty( - pht('Editable By'), - $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - if ($type->shouldRequireUsername()) { $properties->addProperty( pht('Username'), diff --git a/src/applications/phame/controller/blog/PhameBlogManageController.php b/src/applications/phame/controller/blog/PhameBlogManageController.php index 2bdcbf7635..65378d91cb 100644 --- a/src/applications/phame/controller/blog/PhameBlogManageController.php +++ b/src/applications/phame/controller/blog/PhameBlogManageController.php @@ -143,14 +143,6 @@ final class PhameBlogManageController extends PhameBlogController { ), $feed_uri)); - $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $viewer, - $blog); - - $properties->addProperty( - pht('Editable By'), - $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - $engine = id(new PhabricatorMarkupEngine()) ->setViewer($viewer) ->addObject($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION) diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index bf7282b35a..5d30b5a90b 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -293,37 +293,6 @@ final class PhrictionDocumentController } else { throw new Exception(pht("Unknown document status '%s'!", $doc_status)); } - - $move_notice = null; - if ($current_status == PhrictionChangeType::CHANGE_MOVE_HERE) { - $from_doc_id = $content->getChangeRef(); - - $slug_uri = null; - - // If the old document exists and is visible, provide a link to it. - $from_docs = id(new PhrictionDocumentQuery()) - ->setViewer($viewer) - ->withIDs(array($from_doc_id)) - ->execute(); - if ($from_docs) { - $from_doc = head($from_docs); - $slug_uri = PhrictionDocument::getSlugURI($from_doc->getSlug()); - } - - $move_notice = id(new PHUIInfoView()) - ->setSeverity(PHUIInfoView::SEVERITY_NOTICE); - - if ($slug_uri) { - $move_notice->appendChild( - pht( - 'This document was moved from %s.', - phutil_tag('a', array('href' => $slug_uri), $slug_uri))); - } else { - // Render this for consistency, even though it's a bit silly. - $move_notice->appendChild( - pht('This document was moved from elsewhere.')); - } - } } $children = $this->renderDocumentChildren($slug); diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php index d18c436cd0..078e9a0130 100644 --- a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php @@ -49,7 +49,7 @@ final class PhrictionDocumentMoveToTransaction $new = $this->getNewValue(); return pht( - '%s moved this document from %s', + '%s moved this document from %s.', $this->renderAuthor(), $this->renderHandle($new['phid'])); } diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php index a4ba355b16..cb9bd20088 100644 --- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php +++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php @@ -318,7 +318,7 @@ final class PhabricatorPolicyExplainController ->setViewer($viewer) ->setIcon($handle->getIcon().' bluegrey') ->setHeader(pht('Object Policy')) - ->appendList( + ->appendParagraph( array( array( phutil_tag('strong', array(), pht('%s:', $capability_name)), @@ -333,10 +333,17 @@ final class PhabricatorPolicyExplainController ->appendList( array( PhabricatorPolicy::getPolicyExplanation( - $viewer, - $policy->getPHID()), + $viewer, + $policy->getPHID()), )); + if ($policy->isCustomPolicy()) { + $rules_view = id(new PhabricatorPolicyRulesView()) + ->setViewer($viewer) + ->setPolicy($policy); + $object_section->appendRulesView($rules_view); + } + $strength = $this->getStrengthInformation($object, $policy, $capability); if ($strength) { $object_section->appendHint($strength); diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index 4ea7ce1549..d8f239e51a 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -602,12 +602,13 @@ final class PhabricatorPolicyFilter extends Phobject { PhabricatorPolicyInterface $object, $policy, $capability) { + $viewer = $this->viewer; if (!$this->raisePolicyExceptions) { return; } - if ($this->viewer->isOmnipotent()) { + if ($viewer->isOmnipotent()) { // Never raise policy exceptions for the omnipotent viewer. Although we // will never normally issue a policy rejection for the omnipotent // viewer, we can end up here when queries blanket reject objects that @@ -634,9 +635,60 @@ final class PhabricatorPolicyFilter extends Phobject { $capability); } - $more = PhabricatorPolicy::getPolicyExplanation($this->viewer, $policy); - $more = (array)$more; - $more = array_filter($more); + // See T13411. If you receive a policy exception because you can't view + // an object, we also want to avoid disclosing too many details about the + // actual policy (for example, the names of projects in the policy). + + // If you failed a "CAN_VIEW" check, or failed some other check and don't + // have "CAN_VIEW" on the object, we give you an "opaque" explanation. + // Otherwise, we give you a more detailed explanation. + + $view_capability = PhabricatorPolicyCapability::CAN_VIEW; + if ($capability === $view_capability) { + $show_details = false; + } else { + $show_details = self::hasCapability( + $viewer, + $object, + $view_capability); + } + + // TODO: This is a bit clumsy. We're producing HTML and text versions of + // this message, but can't render the full policy rules in text today. + // Users almost never get a text-only version of this exception anyway. + + $head = null; + $more = null; + + if ($show_details) { + $head = PhabricatorPolicy::getPolicyExplanation($viewer, $policy); + + $policy_type = PhabricatorPolicyPHIDTypePolicy::TYPECONST; + $is_custom = (phid_get_type($policy) === $policy_type); + if ($is_custom) { + $policy_map = PhabricatorPolicyQuery::loadPolicies( + $viewer, + $object); + if (isset($policy_map[$capability])) { + require_celerity_resource('phui-policy-section-view-css'); + + $more = id(new PhabricatorPolicyRulesView()) + ->setViewer($viewer) + ->setPolicy($policy_map[$capability]); + + $more = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-rules', + ), + $more); + } + } + } else { + $head = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy); + } + + $head = (array)$head; $exceptions = PhabricatorPolicy::getSpecialRules( $object, @@ -644,7 +696,10 @@ final class PhabricatorPolicyFilter extends Phobject { $capability, true); - $details = array_filter(array_merge($more, $exceptions)); + $text_details = array_filter(array_merge($head, $exceptions)); + $text_details = implode(' ', $text_details); + + $html_details = array($head, $more, $exceptions); $access_denied = $this->renderAccessDenied($object); @@ -653,7 +708,7 @@ final class PhabricatorPolicyFilter extends Phobject { $access_denied, $capability_name, $rejection, - implode(' ', $details)); + $text_details); $exception = id(new PhabricatorPolicyException($full_message)) ->setTitle($access_denied) @@ -661,7 +716,7 @@ final class PhabricatorPolicyFilter extends Phobject { ->setRejection($rejection) ->setCapability($capability) ->setCapabilityName($capability_name) - ->setMoreInfo($details); + ->setMoreInfo($html_details); throw $exception; } diff --git a/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php b/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php index 208f1ae964..a378ecc076 100644 --- a/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php +++ b/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php @@ -60,8 +60,10 @@ final class PhabricatorPolicyManagementShowWorkflow $console->writeOut("__%s__\n\n", pht('CAPABILITIES')); foreach ($policies as $capability => $policy) { + $ref = $policy->newRef($viewer); + $console->writeOut(" **%s**\n", $capability); - $console->writeOut(" %s\n", $policy->renderDescription()); + $console->writeOut(" %s\n", $ref->getPolicyDisplayName()); $console->writeOut(" %s\n", PhabricatorPolicy::getPolicyExplanation($viewer, $policy->getPHID())); $console->writeOut("\n"); diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php index e51b2ca401..018007db28 100644 --- a/src/applications/policy/query/PhabricatorPolicyQuery.php +++ b/src/applications/policy/query/PhabricatorPolicyQuery.php @@ -43,13 +43,13 @@ final class PhabricatorPolicyQuery public static function renderPolicyDescriptions( PhabricatorUser $viewer, - PhabricatorPolicyInterface $object, - $icon = false) { + PhabricatorPolicyInterface $object) { $policies = self::loadPolicies($viewer, $object); foreach ($policies as $capability => $policy) { - $policies[$capability] = $policy->renderDescription($icon); + $policies[$capability] = $policy->newRef($viewer) + ->newCapabilityLink($object, $capability); } return $policies; diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index 2df8fdf6a0..66a7d9e3be 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -85,8 +85,10 @@ final class PhabricatorPolicy $phid_type = phid_get_type($policy_identifier); switch ($phid_type) { case PhabricatorProjectProjectPHIDType::TYPECONST: - $policy->setType(PhabricatorPolicyType::TYPE_PROJECT); - $policy->setName($handle->getName()); + $policy + ->setType(PhabricatorPolicyType::TYPE_PROJECT) + ->setName($handle->getName()) + ->setIcon($handle->getIcon()); break; case PhabricatorPeopleUserPHIDType::TYPECONST: $policy->setType(PhabricatorPolicyType::TYPE_USER); @@ -218,6 +220,25 @@ final class PhabricatorPolicy PhabricatorUser $viewer, $policy) { + $type = phid_get_type($policy); + if ($type === PhabricatorProjectProjectPHIDType::TYPECONST) { + $handle = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($policy)) + ->executeOne(); + + return pht( + 'Members of the project "%s" can take this action.', + $handle->getFullName()); + } + + return self::getOpaquePolicyExplanation($viewer, $policy); + } + + public static function getOpaquePolicyExplanation( + PhabricatorUser $viewer, + $policy) { + $rule = PhabricatorPolicyQuery::getObjectPolicyRule($policy); if ($rule) { return $rule->getPolicyExplanation(); @@ -243,7 +264,9 @@ final class PhabricatorPolicy $type = phid_get_type($policy); if ($type == PhabricatorProjectProjectPHIDType::TYPECONST) { return pht( - 'Members of the project "%s" can take this action.', + 'Members of a particular project can take this action. (You '. + 'can not see this object, so the name of this project is '. + 'restricted.)', $handle->getFullName()); } else if ($type == PhabricatorPeopleUserPHIDType::TYPECONST) { return pht( @@ -274,45 +297,22 @@ final class PhabricatorPolicy } } - public function renderDescription($icon = false) { - $img = null; - if ($icon) { - $img = id(new PHUIIconView()) - ->setIcon($this->getIcon()); - } + public function newRef(PhabricatorUser $viewer) { + return id(new PhabricatorPolicyRef()) + ->setViewer($viewer) + ->setPolicy($this); + } - if ($this->getHref()) { - $desc = javelin_tag( - 'a', - array( - 'href' => $this->getHref(), - 'class' => 'policy-link', - 'sigil' => $this->getWorkflow() ? 'workflow' : null, - ), - array( - $img, - $this->getName(), - )); - } else { - if ($img) { - $desc = array($img, $this->getName()); - } else { - $desc = $this->getName(); - } - } + public function isProjectPolicy() { + return ($this->getType() === PhabricatorPolicyType::TYPE_PROJECT); + } - switch ($this->getType()) { - case PhabricatorPolicyType::TYPE_PROJECT: - return pht('%s (Project)', $desc); - case PhabricatorPolicyType::TYPE_CUSTOM: - return $desc; - case PhabricatorPolicyType::TYPE_MASKED: - return pht( - '%s (You do not have permission to view policy details.)', - $desc); - default: - return $desc; - } + public function isCustomPolicy() { + return ($this->getType() === PhabricatorPolicyType::TYPE_CUSTOM); + } + + public function isMaskedPolicy() { + return ($this->getType() === PhabricatorPolicyType::TYPE_MASKED); } /** diff --git a/src/applications/policy/view/PHUIPolicySectionView.php b/src/applications/policy/view/PHUIPolicySectionView.php index 471e5035fb..14d97fb17e 100644 --- a/src/applications/policy/view/PHUIPolicySectionView.php +++ b/src/applications/policy/view/PHUIPolicySectionView.php @@ -93,6 +93,16 @@ final class PHUIPolicySectionView return $this->appendChild(phutil_tag('p', array(), $content)); } + public function appendRulesView(PhabricatorPolicyRulesView $rules_view) { + return $this->appendChild( + phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-rules', + ), + $rules_view)); + } + protected function getTagAttributes() { return array( 'class' => 'phui-policy-section-view', @@ -100,7 +110,7 @@ final class PHUIPolicySectionView } protected function getTagContent() { - require_celerity_resource('phui-header-view-css'); + require_celerity_resource('phui-policy-section-view-css'); $icon_view = null; $icon = $this->getIcon(); diff --git a/src/applications/policy/view/PhabricatorPolicyRef.php b/src/applications/policy/view/PhabricatorPolicyRef.php new file mode 100644 index 0000000000..605d59ee45 --- /dev/null +++ b/src/applications/policy/view/PhabricatorPolicyRef.php @@ -0,0 +1,99 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function setPolicy(PhabricatorPolicy $policy) { + $this->policy = $policy; + return $this; + } + + public function getPolicy() { + return $this->policy; + } + + public function getPolicyDisplayName() { + $policy = $this->getPolicy(); + return $policy->getFullName(); + } + + public function newTransactionLink( + $mode, + PhabricatorApplicationTransaction $xaction) { + + $policy = $this->getPolicy(); + + if ($policy->isCustomPolicy()) { + $uri = urisprintf( + '/transactions/%s/%s/', + $mode, + $xaction->getPHID()); + $workflow = true; + } else { + $uri = $policy->getHref(); + $workflow = false; + } + + return $this->newLink($uri, $workflow); + } + + public function newCapabilityLink($object, $capability) { + $policy = $this->getPolicy(); + + $uri = urisprintf( + '/policy/explain/%s/%s/', + $object->getPHID(), + $capability); + + return $this->newLink($uri, true); + } + + private function newLink($uri, $workflow) { + $policy = $this->getPolicy(); + $name = $policy->getName(); + + if ($uri !== null) { + $name = javelin_tag( + 'a', + array( + 'href' => $uri, + 'sigil' => ($workflow ? 'workflow' : null), + ), + $name); + } + + $hint = $this->getPolicyTypeHint(); + if ($hint !== null) { + $name = pht('%s (%s)', $name, $hint); + } + + return $name; + } + + private function getPolicyTypeHint() { + $policy = $this->getPolicy(); + + if ($policy->isProjectPolicy()) { + return pht('Project'); + } + + if ($policy->isMaskedPolicy()) { + return pht('You do not have permission to view policy details.'); + } + + return null; + } + +} diff --git a/src/applications/policy/view/PhabricatorPolicyRulesView.php b/src/applications/policy/view/PhabricatorPolicyRulesView.php new file mode 100644 index 0000000000..657b612f16 --- /dev/null +++ b/src/applications/policy/view/PhabricatorPolicyRulesView.php @@ -0,0 +1,84 @@ +policy = $policy; + return $this; + } + + public function getPolicy() { + return $this->policy; + } + + public function render() { + $policy = $this->getPolicy(); + + require_celerity_resource('policy-transaction-detail-css'); + + $rule_objects = array(); + foreach ($policy->getCustomRuleClasses() as $class) { + $rule_objects[$class] = newv($class, array()); + } + + $policy = clone $policy; + $policy->attachRuleObjects($rule_objects); + + $details = array(); + $details[] = phutil_tag( + 'p', + array( + 'class' => 'policy-transaction-detail-intro', + ), + pht('These rules are processed in order:')); + + foreach ($policy->getRules() as $index => $rule) { + $rule_object = $rule_objects[$rule['rule']]; + if ($rule['action'] == 'allow') { + $icon = 'fa-check-circle green'; + } else { + $icon = 'fa-minus-circle red'; + } + $icon = id(new PHUIIconView()) + ->setIcon($icon) + ->setText( + ucfirst($rule['action']).' '.$rule_object->getRuleDescription()); + + $handle_phids = $rule_object->getRequiredHandlePHIDsForSummary( + $rule['value']); + if ($handle_phids) { + $value = $this->getViewer() + ->renderHandleList($handle_phids) + ->setAsInline(true); + } else { + $value = $rule['value']; + } + + $details[] = phutil_tag('div', + array( + 'class' => 'policy-transaction-detail-row', + ), + array( + $icon, + $value, + )); + } + + $details[] = phutil_tag( + 'p', + array( + 'class' => 'policy-transaction-detail-end', + ), + pht( + 'If no rules match, %s all other users.', + phutil_tag('b', + array(), + $policy->getDefaultAction()))); + + return $details; + } + +} diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php index 54267829d3..1d234d4285 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -904,7 +904,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO public function newEditEngineSubtypeMap() { $config = PhabricatorEnv::getEnvConfig('projects.subtypes'); - return PhabricatorEditEngineSubtype::newSubtypeMap($config); + return PhabricatorEditEngineSubtype::newSubtypeMap($config) + ->setDatasource(new PhabricatorProjectSubtypeDatasource()); } public function newSubtypeObject() { diff --git a/src/applications/spaces/controller/PhabricatorSpacesViewController.php b/src/applications/spaces/controller/PhabricatorSpacesViewController.php index 5fa1c01143..ba55ba90a7 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesViewController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesViewController.php @@ -80,14 +80,6 @@ final class PhabricatorSpacesViewController ? pht('Yes') : pht('No')); - $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $viewer, - $space); - - $list->addProperty( - pht('Editable By'), - $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - $description = $space->getDescription(); if (strlen($description)) { $description = new PHUIRemarkupView($viewer, $description); diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php index bef6fef5a8..ef5e168898 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php @@ -33,6 +33,7 @@ final class PhabricatorApplicationTransactionValueController case PhabricatorTransactions::TYPE_EDIT_POLICY: case PhabricatorTransactions::TYPE_JOIN_POLICY: case PhabricatorRepositoryPushPolicyTransaction::TRANSACTIONTYPE: + case PhabricatorApplicationPolicyChangeTransaction::TRANSACTIONTYPE: break; default: return new Aphront404Response(); @@ -57,89 +58,16 @@ final class PhabricatorApplicationTransactionValueController return new Aphront404Response(); } - $rule_objects = array(); - foreach ($policy->getCustomRuleClasses() as $class) { - $rule_objects[$class] = newv($class, array()); - } - $policy->attachRuleObjects($rule_objects); + $rules_view = id(new PhabricatorPolicyRulesView()) + ->setViewer($viewer) + ->setPolicy($policy); - $this->requireResource('policy-transaction-detail-css'); $cancel_uri = $this->guessCancelURI($viewer, $xaction); return $this->newDialog() ->setTitle($policy->getFullName()) ->setWidth(AphrontDialogView::WIDTH_FORM) - ->appendChild($this->renderPolicyDetails($policy, $rule_objects)) + ->appendChild($rules_view) ->addCancelButton($cancel_uri, pht('Close')); } - - private function extractPHIDs( - PhabricatorPolicy $policy, - array $rule_objects) { - - $phids = array(); - foreach ($policy->getRules() as $rule) { - $rule_object = $rule_objects[$rule['rule']]; - $phids[] = - $rule_object->getRequiredHandlePHIDsForSummary($rule['value']); - } - return array_filter(array_mergev($phids)); - } - - private function renderPolicyDetails( - PhabricatorPolicy $policy, - array $rule_objects) { - $details = array(); - $details[] = phutil_tag( - 'p', - array( - 'class' => 'policy-transaction-detail-intro', - ), - pht('These rules are processed in order:')); - - foreach ($policy->getRules() as $index => $rule) { - $rule_object = $rule_objects[$rule['rule']]; - if ($rule['action'] == 'allow') { - $icon = 'fa-check-circle green'; - } else { - $icon = 'fa-minus-circle red'; - } - $icon = id(new PHUIIconView()) - ->setIcon($icon) - ->setText( - ucfirst($rule['action']).' '.$rule_object->getRuleDescription()); - - $handle_phids = $rule_object->getRequiredHandlePHIDsForSummary( - $rule['value']); - if ($handle_phids) { - $value = $this->getViewer() - ->renderHandleList($handle_phids) - ->setAsInline(true); - } else { - $value = $rule['value']; - } - - $details[] = phutil_tag('div', - array( - 'class' => 'policy-transaction-detail-row', - ), - array( - $icon, - $value, - )); - } - - $details[] = phutil_tag( - 'p', - array( - 'class' => 'policy-transaction-detail-end', - ), - pht( - 'If no rules match, %s all other users.', - phutil_tag('b', - array(), - $policy->getDefaultAction()))); - return $details; - } - } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php index 6e1d1de115..f471fcd92f 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php @@ -14,6 +14,7 @@ final class PhabricatorEditEngineSubtype private $childSubtypes = array(); private $childIdentifiers = array(); private $fieldConfiguration = array(); + private $mutations; public function setKey($key) { $this->key = $key; @@ -78,6 +79,15 @@ final class PhabricatorEditEngineSubtype return $this->childIdentifiers; } + public function setMutations($mutations) { + $this->mutations = $mutations; + return $this; + } + + public function getMutations() { + return $this->mutations; + } + public function hasTagView() { return (bool)strlen($this->getTagText()); } @@ -152,6 +162,7 @@ final class PhabricatorEditEngineSubtype 'icon' => 'optional string', 'children' => 'optional map', 'fields' => 'optional map', + 'mutations' => 'optional list', )); $key = $value['key']; @@ -217,6 +228,28 @@ final class PhabricatorEditEngineSubtype 'with key "%s". This subtype is required and must be defined.', self::SUBTYPE_DEFAULT)); } + + foreach ($config as $value) { + $key = idx($value, 'key'); + + $mutations = idx($value, 'mutations'); + if (!$mutations) { + continue; + } + + foreach ($mutations as $mutation) { + if (!isset($map[$mutation])) { + throw new Exception( + pht( + 'Subtype configuration is invalid: subtype with key "%s" '. + 'specifies that it can mutate into subtype "%s", but that is '. + 'not a valid subtype.', + $key, + $mutation)); + } + } + } + } public static function newSubtypeMap(array $config) { @@ -267,6 +300,8 @@ final class PhabricatorEditEngineSubtype } } + $subtype->setMutations(idx($entry, 'mutations')); + $map[$key] = $subtype; } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php index 638b665184..edf2d2045a 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php @@ -5,6 +5,7 @@ final class PhabricatorEditEngineSubtypeMap extends Phobject { private $subtypes; + private $datasource; public function __construct(array $subtypes) { assert_instances_of($subtypes, 'PhabricatorEditEngineSubtype'); @@ -39,6 +40,57 @@ final class PhabricatorEditEngineSubtypeMap return $this->subtypes[$subtype_key]; } + public function setDatasource(PhabricatorTypeaheadDatasource $datasource) { + $this->datasource = $datasource; + return $this; + } + + public function newDatasource() { + if (!$this->datasource) { + throw new PhutilInvalidStateException('setDatasource'); + } + + return clone($this->datasource); + } + + public function getMutationMap($source_key) { + return mpull($this->getMutations($source_key), 'getName'); + } + + public function getMutations($source_key) { + $mutations = $this->subtypes; + + $subtype = idx($this->subtypes, $source_key); + if ($subtype) { + $map = $subtype->getMutations(); + if ($map !== null) { + $map = array_fuse($map); + foreach ($mutations as $key => $mutation) { + if ($key === $source_key) { + // This is the current subtype, so we always want to show it. + continue; + } + + if (isset($map[$key])) { + // This is an allowed mutation, so keep it. + continue; + } + + // Discard other subtypes as mutation options. + unset($mutations[$key]); + } + } + } + + // If the only available mutation is the current subtype, treat this like + // no mutations are available. + if (array_keys($mutations) === array($source_key)) { + $mutations = array(); + } + + return $mutations; + } + public function getCreateFormsForSubtype( PhabricatorEditEngine $edit_engine, PhabricatorEditEngineSubtypeInterface $object) { diff --git a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php index d0b6d017f3..e73d476d74 100644 --- a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php +++ b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php @@ -29,9 +29,17 @@ final class PhabricatorSubtypeEditEngineExtension PhabricatorApplicationTransactionInterface $object) { $subtype_type = PhabricatorTransactions::TYPE_SUBTYPE; + $subtype_value = $object->getEditEngineSubtype(); $map = $object->newEditEngineSubtypeMap(); - $options = $map->getDisplayMap(); + + if ($object->getID()) { + $options = $map->getMutationMap($subtype_value); + } else { + // NOTE: This is a crude proxy for "are we in the bulk edit workflow". + // We want to allow any mutation. + $options = $map->getDisplayMap(); + } $subtype_field = id(new PhabricatorSelectEditField()) ->setKey(self::EDITKEY) @@ -40,12 +48,12 @@ final class PhabricatorSubtypeEditEngineExtension ->setTransactionType($subtype_type) ->setConduitDescription(pht('Change the object subtype.')) ->setConduitTypeDescription(pht('New object subtype key.')) - ->setValue($object->getEditEngineSubtype()) + ->setValue($subtype_value) ->setOptions($options); - // If subtypes are configured, enable changing them from the bulk editor - // and comment action stack. - if ($map->getCount() > 1) { + // If subtypes are configured, enable changing them from the bulk editor. + // Bulk editor + if ($options) { $subtype_field ->setBulkEditLabel(pht('Change subtype to')) ->setCommentActionLabel(pht('Change Subtype')) diff --git a/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php b/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php new file mode 100644 index 0000000000..be15540fc9 --- /dev/null +++ b/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php @@ -0,0 +1,52 @@ +getEditEngineSubtype(); + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_PHID; + } + + protected function getDatasource() { + $object = $this->getAdapter()->getObject(); + $map = $object->newEditEngineSubtypeMap(); + return $map->newDatasource(); + } + + protected function getDatasourceValueMap() { + $object = $this->getAdapter()->getObject(); + $map = $object->newEditEngineSubtypeMap(); + + $result = array(); + foreach ($map->getSubtypes() as $subtype) { + $result[$subtype->getKey()] = $subtype->getName(); + } + + return $result; + } + + public function isFieldAvailable() { + $object = $this->getAdapter()->getObject(); + $map = $object->newEditEngineSubtypeMap(); + return ($map->getCount() > 1); + } + +} diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 7c327393f8..7e65ac0a09 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -445,19 +445,15 @@ abstract class PhabricatorApplicationTransaction $policy = PhabricatorPolicy::newFromPolicyAndHandle( $phid, $this->getHandleIfExists($phid)); + + $ref = $policy->newRef($this->getViewer()); + if ($this->renderingTarget == self::TARGET_HTML) { - switch ($policy->getType()) { - case PhabricatorPolicyType::TYPE_CUSTOM: - $policy->setHref('/transactions/'.$state.'/'.$this->getPHID().'/'); - $policy->setWorkflow(true); - break; - default: - break; - } - $output = $policy->renderDescription(); + $output = $ref->newTransactionLink($state, $this); } else { - $output = hsprintf('%s', $policy->getFullName()); + $output = $ref->getPolicyDisplayName(); } + return $output; } @@ -775,6 +771,13 @@ abstract class PhabricatorApplicationTransaction case PhabricatorTransactions::TYPE_TOKEN: case PhabricatorTransactions::TYPE_MFA: return true; + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + // See T8952. When an application (usually Herald) modifies + // subscribers, this tends to be very uninteresting. + if ($this->isApplicationAuthor()) { + return true; + } + break; case PhabricatorTransactions::TYPE_EDGE: $edge_type = $this->getMetadataValue('edge:type'); switch ($edge_type) { @@ -1387,12 +1390,6 @@ abstract class PhabricatorApplicationTransaction return 25; } - if ($this->isApplicationAuthor()) { - // When applications (most often: Herald) change subscriptions it - // is very uninteresting. - return 1; - } - // In other cases, subscriptions are more interesting than comments // (which are shown anyway) but less interesting than any other type of // transaction. diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index 2d0cb8e7c1..7d5e3c533e 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -215,17 +215,16 @@ abstract class PhabricatorModularTransactionType $phid, $handles[$phid]); + $ref = $policy->newRef($viewer); + if ($this->isTextMode()) { - return $this->renderValue($policy->getFullName()); + $name = $ref->getPolicyDisplayName(); + } else { + $storage = $this->getStorage(); + $name = $ref->newTransactionLink($mode, $storage); } - $storage = $this->getStorage(); - if ($policy->getType() == PhabricatorPolicyType::TYPE_CUSTOM) { - $policy->setHref('/transactions/'.$mode.'/'.$storage->getPHID().'/'); - $policy->setWorkflow(true); - } - - return $this->renderValue($policy->renderDescription()); + return $this->renderValue($name); } final protected function renderHandleList(array $phids) { diff --git a/src/docs/user/userguide/unlocking.diviner b/src/docs/user/userguide/unlocking.diviner index 7dc29f69bd..456655a393 100644 --- a/src/docs/user/userguide/unlocking.diviner +++ b/src/docs/user/userguide/unlocking.diviner @@ -63,16 +63,23 @@ For detailed help on managing and stripping MFA, see the instructions in Unlocking Objects ================= -If you aren't sure who owns an object, or no user account has access to an -object, you can directly change object policies from the CLI: +If you aren't sure who owns an object, you can inspect the policies from the +CLI: + +``` +$ ./bin/policy show +``` + +To identify the object you want to examine, you can specify an object +name (like `T123`) or a PHID as the `` parameter. + +If examining the policy isn't helpful, or no user account has access to an +object, you can then directly change object policies from the CLI: ``` $ ./bin/policy unlock [--view ...] [--edit ...] [--owner ...] ``` -To identify the object you want to unlock, you can specify an object name (like -`T123`) or a PHID as the `` parameter. - Use the `--view` and `--edit` flags (and, for some objects, the `--owner` flag) to specify new policies for the object. diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index 6a096af76d..1d851f04ee 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -354,45 +354,3 @@ body .phui-header-shell.phui-bleed-header .phui-header-view .phui-tag-indigo a { color: {$sh-indigotext}; } - -.phui-policy-section-view { - margin-bottom: 24px; -} - -.phui-policy-section-view-header { - background: {$bluebackground}; - border-bottom: 1px solid {$lightblueborder}; - padding: 4px 8px; - color: {$darkbluetext}; - margin-bottom: 8px; -} - -.phui-policy-section-view-header-text { - font-weight: bold; -} - -.phui-policy-section-view-header .phui-icon-view { - margin-right: 8px; -} - -.phui-policy-section-view-link { - float: right; -} - -.phui-policy-section-view-link .phui-icon-view { - color: {$bluetext}; -} - -.phui-policy-section-view-hint { - color: {$greytext}; - background: {$lightbluebackground}; - padding: 8px; -} - -.phui-policy-section-view-body { - padding: 0 12px; -} - -.phui-policy-section-view-inactive-rule { - color: {$greytext}; -} diff --git a/webroot/rsrc/css/phui/phui-policy-section-view.css b/webroot/rsrc/css/phui/phui-policy-section-view.css new file mode 100644 index 0000000000..4325b34867 --- /dev/null +++ b/webroot/rsrc/css/phui/phui-policy-section-view.css @@ -0,0 +1,62 @@ +/** + * @provides phui-policy-section-view-css + */ + +.phui-policy-section-view { + margin-bottom: 24px; +} + +.phui-policy-section-view-header { + background: {$bluebackground}; + border-bottom: 1px solid {$lightblueborder}; + padding: 4px 8px; + color: {$darkbluetext}; + margin-bottom: 8px; +} + +.phui-policy-section-view-header-text { + font-weight: bold; +} + +.phui-policy-section-view-header .phui-icon-view { + margin-right: 8px; +} + +.phui-policy-section-view-link { + float: right; +} + +.phui-policy-section-view-link .phui-icon-view { + color: {$bluetext}; +} + +.phui-policy-section-view-hint { + color: {$greytext}; + background: {$lightbluebackground}; + padding: 8px; +} + +.phui-policy-section-view-body { + padding: 0 12px; +} + +.phui-policy-section-view-inactive-rule { + color: {$greytext}; +} + +.phui-policy-section-view-rules { + margin: 8px 0; + padding: 8px; + background: {$lightbluebackground}; + border: 1px solid {$lightblueborder}; +} + +.phui-policy-section-view .phui-policy-section-view-body ul { + margin: 8px 0; + padding: 0 16px 0 24px; + list-style: disc; +} + +.phui-policy-section-view .phui-policy-section-view-body p + p { + margin-top: 8px; +} diff --git a/webroot/rsrc/css/phui/workboards/phui-workcard.css b/webroot/rsrc/css/phui/workboards/phui-workcard.css index 3c6a798fc8..d9acef12ef 100644 --- a/webroot/rsrc/css/phui/workboards/phui-workcard.css +++ b/webroot/rsrc/css/phui/workboards/phui-workcard.css @@ -36,6 +36,10 @@ .phui-workcard .phui-oi-link { white-space: normal; + + /* See T13413. This works around a Chrome 77 rendering engine freeze. */ + word-wrap: normal; + font-weight: normal; color: {$blacktext}; margin-left: 2px; diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 443fe9811e..254061533a 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -350,8 +350,10 @@ JX.install('HeraldRuleEditor', { sigil: 'field-select' }; - var field_select = this._renderGroupSelect(groups, attrs); - field_select.value = this._config.conditions[row_id][0]; + var field_select = this._renderGroupSelect( + groups, + attrs, + this._config.conditions[row_id][0]); var field_cell = JX.$N('td', {sigil: 'field-cell'}, field_select); @@ -367,18 +369,38 @@ JX.install('HeraldRuleEditor', { } }, - _renderGroupSelect: function(groups, attrs) { + _renderGroupSelect: function(groups, attrs, value) { var optgroups = []; for (var ii = 0; ii < groups.length; ii++) { var group = groups[ii]; var options = []; for (var k in group.options) { - options.push(JX.$N('option', {value: k}, group.options[k])); + var option = group.options[k]; + + var name = option.name; + var available = option.available; + + // See T7961. If the option is not marked as "available", we only + // include it in the dropdown if the dropdown already has it as a + // value. We want to hide options provided by applications which are + // not installed, but do not want to break existing rules. + + if (available || (k === value)) { + options.push(JX.$N('option', {value: k}, name)); + } + } + if (options.length) { + optgroups.push(JX.$N('optgroup', {label: group.label}, options)); } - optgroups.push(JX.$N('optgroup', {label: group.label}, options)); } - return JX.$N('select', attrs, optgroups); + var select = JX.$N('select', attrs, optgroups); + + if (value !== undefined) { + select.value = value; + } + + return select; }, _newAction : function(data) { @@ -402,8 +424,10 @@ JX.install('HeraldRuleEditor', { sigil: 'action-select' }; - var action_select = this._renderGroupSelect(groups, attrs); - action_select.value = action[0]; + var action_select = this._renderGroupSelect( + groups, + attrs, + action[0]); var action_cell = JX.$N('td', {sigil: 'action-cell'}, action_select);