From ec9237fe13c54634d9e9edf0da99e891148f6aae Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 14 Apr 2019 13:38:16 -0700 Subject: [PATCH] In repository settings, fold "Autoclose On/Off" into "Publishing On/Off" Summary: Depends on D20423. Ref T13277. Repositories currently have separate toggles for "Autoclose" and "Publishing". Merge the "Autoclose" toggle into the "Publishing" toggle. I'm unaware of any valid use case for enabling one but not the other. (This doesn't fix all the documentation, yet.) Test Plan: Edited a repository, saw only one publishing option. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13277 Differential Revision: https://secure.phabricator.com/D20424 --- src/__phutil_library_map__.php | 6 +-- .../editor/DiffusionCommitEditEngine.php | 4 +- .../editor/DiffusionRepositoryEditEngine.php | 15 +------- ...usionRepositoryBranchesManagementPanel.php | 37 ++++++++++--------- ...onRepositoryPublishingManagementPanel.php} | 32 +++++----------- .../view/DiffusionBranchTableView.php | 6 +-- .../storage/PhabricatorRepository.php | 8 +++- ...bricatorRepositoryAutocloseTransaction.php | 34 ----------------- ...PhabricatorRepositoryNotifyTransaction.php | 4 +- 9 files changed, 46 insertions(+), 100 deletions(-) rename src/applications/diffusion/management/{DiffusionRepositoryActionsManagementPanel.php => DiffusionRepositoryPublishingManagementPanel.php} (60%) delete mode 100644 src/applications/repository/xaction/PhabricatorRepositoryAutocloseTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 36edb62c70..f536c06527 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -935,7 +935,6 @@ phutil_register_library_map(array( 'DiffusionRefTableController' => 'applications/diffusion/controller/DiffusionRefTableController.php', 'DiffusionRefsQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php', 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php', - 'DiffusionRepositoryActionsManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryActionsManagementPanel.php', 'DiffusionRepositoryAutomationManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryAutomationManagementPanel.php', 'DiffusionRepositoryBasicsManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php', 'DiffusionRepositoryBranchesManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php', @@ -970,6 +969,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php', 'DiffusionRepositoryPoliciesManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryPoliciesManagementPanel.php', 'DiffusionRepositoryProfilePictureController' => 'applications/diffusion/controller/DiffusionRepositoryProfilePictureController.php', + 'DiffusionRepositoryPublishingManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryPublishingManagementPanel.php', 'DiffusionRepositoryRef' => 'applications/diffusion/data/DiffusionRepositoryRef.php', 'DiffusionRepositoryRemarkupRule' => 'applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php', 'DiffusionRepositorySearchConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRepositorySearchConduitAPIMethod.php', @@ -4322,7 +4322,6 @@ phutil_register_library_map(array( 'PhabricatorRepositoryActivateTransaction' => 'applications/repository/xaction/PhabricatorRepositoryActivateTransaction.php', 'PhabricatorRepositoryAuditRequest' => 'applications/repository/storage/PhabricatorRepositoryAuditRequest.php', 'PhabricatorRepositoryAutocloseOnlyTransaction' => 'applications/repository/xaction/PhabricatorRepositoryAutocloseOnlyTransaction.php', - 'PhabricatorRepositoryAutocloseTransaction' => 'applications/repository/xaction/PhabricatorRepositoryAutocloseTransaction.php', 'PhabricatorRepositoryBlueprintsTransaction' => 'applications/repository/xaction/PhabricatorRepositoryBlueprintsTransaction.php', 'PhabricatorRepositoryBranch' => 'applications/repository/storage/PhabricatorRepositoryBranch.php', 'PhabricatorRepositoryCallsignTransaction' => 'applications/repository/xaction/PhabricatorRepositoryCallsignTransaction.php', @@ -6588,7 +6587,6 @@ phutil_register_library_map(array( 'DiffusionRefTableController' => 'DiffusionController', 'DiffusionRefsQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionRenameHistoryQuery' => 'Phobject', - 'DiffusionRepositoryActionsManagementPanel' => 'DiffusionRepositoryManagementPanel', 'DiffusionRepositoryAutomationManagementPanel' => 'DiffusionRepositoryManagementPanel', 'DiffusionRepositoryBasicsManagementPanel' => 'DiffusionRepositoryManagementPanel', 'DiffusionRepositoryBranchesManagementPanel' => 'DiffusionRepositoryManagementPanel', @@ -6622,6 +6620,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryPath' => 'Phobject', 'DiffusionRepositoryPoliciesManagementPanel' => 'DiffusionRepositoryManagementPanel', 'DiffusionRepositoryProfilePictureController' => 'DiffusionController', + 'DiffusionRepositoryPublishingManagementPanel' => 'DiffusionRepositoryManagementPanel', 'DiffusionRepositoryRef' => 'Phobject', 'DiffusionRepositoryRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'DiffusionRepositorySearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', @@ -10555,7 +10554,6 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', ), 'PhabricatorRepositoryAutocloseOnlyTransaction' => 'PhabricatorRepositoryTransactionType', - 'PhabricatorRepositoryAutocloseTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryBlueprintsTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryBranch' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryCallsignTransaction' => 'PhabricatorRepositoryTransactionType', diff --git a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php index 667af46550..3cb0a79657 100644 --- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php @@ -120,10 +120,10 @@ final class DiffusionCommitEditEngine $desc = pht('No, Repository Importing'); break; case PhabricatorRepository::BECAUSE_AUTOCLOSE_DISABLED: - $desc = pht('No, Autoclose Disabled'); + $desc = pht('No, Repository Publishing Disabled'); break; case PhabricatorRepository::BECAUSE_NOT_ON_AUTOCLOSE_BRANCH: - $desc = pht('No, Not On Autoclose Branch'); + $desc = pht('No, Not Reachable from Permanent Ref'); break; case PhabricatorRepository::BECAUSE_AUTOCLOSE_FORCED: $desc = pht('Yes, Forced Via bin/repository CLI Tool.'); diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php index cd05b1ca27..65fbe35a8f 100644 --- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php @@ -468,20 +468,7 @@ final class DiffusionRepositoryEditEngine ->setDescription(pht('Configure how changes are published.')) ->setConduitDescription(pht('Change publishing options.')) ->setConduitTypeDescription(pht('New notification setting.')) - ->setValue(!$object->getDetail('herald-disabled')), - id(new PhabricatorBoolEditField()) - ->setKey('autoclose') - ->setLabel(pht('Autoclose')) - ->setTransactionType( - PhabricatorRepositoryAutocloseTransaction::TRANSACTIONTYPE) - ->setIsCopyable(true) - ->setOptions( - pht('Disable Autoclose'), - pht('Enable Autoclose')) - ->setDescription(pht('Stop or resume autoclosing in this repository.')) - ->setConduitDescription(pht('Change autoclose setting.')) - ->setConduitTypeDescription(pht('New autoclose setting.')) - ->setValue(!$object->getDetail('disable-autoclose')), + ->setValue(!$object->isPublishingDisabled()), id(new PhabricatorPolicyEditField()) ->setKey('policy.push') ->setLabel(pht('Push Policy')) diff --git a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php index 80faeb481b..124766e13d 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php @@ -96,24 +96,23 @@ final class DiffusionRepositoryBranchesManagementPanel phutil_tag('em', array(), pht('Track All Branches'))); $view->addProperty(pht('Track Only'), $track_only); - $autoclose_rules = $repository->getAutocloseOnlyRules(); - $autoclose_rules = implode(', ', $autoclose_rules); - $autoclose_only = nonempty( - $autoclose_rules, - phutil_tag('em', array(), pht('All Branches'))); - $autoclose_disabled = false; - if ($repository->getDetail('disable-autoclose')) { - $autoclose_disabled = true; - $autoclose_only = - phutil_tag('em', array(), pht('Autoclose has been disabled')); + $publishing_disabled = $repository->isPublishingDisabled(); + if ($publishing_disabled) { + $permanent_display = + phutil_tag('em', array(), pht('Publishing Disabled')); + } else { + $permanent_rules = $repository->getAutocloseOnlyRules(); + if ($permanent_rules) { + $permanent_display = implode(', ', $permanent_rules); + } else { + $permanent_display = phutil_tag('em', array(), pht('All Branches')); + } } - - $view->addProperty(pht('Permanent Refs'), $autoclose_only); + $view->addProperty(pht('Permanent Refs'), $permanent_display); $content[] = $this->newBox(pht('Branches'), $view); - // Branch Autoclose Table if (!$repository->isImporting()) { $request = $this->getRequest(); $pager = id(new PHUIPagerView()) @@ -153,10 +152,14 @@ final class DiffusionRepositoryBranchesManagementPanel $status = pht('Open'); } - if ($autoclose_disabled) { - $autoclose_status = pht('Disabled (Repository)'); + if ($publishing_disabled) { + $permanent_status = pht('Publishing Disabled'); } else { - $autoclose_status = pht('Not Permanent'); + if ($permanent) { + $permanent_status = pht('Permanent'); + } else { + $permanent_status = pht('Not Permanent'); + } } $rows[] = array( @@ -164,7 +167,7 @@ final class DiffusionRepositoryBranchesManagementPanel $branch_name, $status, $tracking ? pht('Tracking') : pht('Off'), - $permanent ? pht('Permanent') : $autoclose_status, + $permanent_status, ); } $branch_table = new AphrontTableView($rows); diff --git a/src/applications/diffusion/management/DiffusionRepositoryActionsManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryPublishingManagementPanel.php similarity index 60% rename from src/applications/diffusion/management/DiffusionRepositoryActionsManagementPanel.php rename to src/applications/diffusion/management/DiffusionRepositoryPublishingManagementPanel.php index eb1b98dba7..97f51c1c32 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryActionsManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryPublishingManagementPanel.php @@ -1,12 +1,12 @@ getRepository(); - $has_any = - $repository->getDetail('herald-disabled') || - $repository->getDetail('disable-autoclose'); - - // NOTE: Any value here really means something is disabled, so try to - // hint that a little bit with the icon. + $has_any = $repository->isPublishingDisabled(); if ($has_any) { return 'fa-flash'; @@ -33,7 +28,6 @@ final class DiffusionRepositoryActionsManagementPanel protected function getEditEngineFieldKeys() { return array( 'publish', - 'autoclose', ); } @@ -47,13 +41,13 @@ final class DiffusionRepositoryActionsManagementPanel $repository, PhabricatorPolicyCapability::CAN_EDIT); - $actions_uri = $this->getEditPageURI(); + $publishing_uri = $this->getEditPageURI(); $action_list->addAction( id(new PhabricatorActionView()) ->setIcon('fa-pencil') - ->setName(pht('Edit Actions')) - ->setHref($actions_uri) + ->setName(pht('Edit Publishing')) + ->setHref($publishing_uri) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); @@ -68,19 +62,13 @@ final class DiffusionRepositoryActionsManagementPanel $view = id(new PHUIPropertyListView()) ->setViewer($viewer); - $notify = $repository->getDetail('herald-disabled') + $notify = $repository->isPublishingDisabled() ? pht('Off') : pht('On'); $notify = phutil_tag('em', array(), $notify); - $view->addProperty(pht('Publish/Notify'), $notify); + $view->addProperty(pht('Publishing'), $notify); - $autoclose = $repository->getDetail('disable-autoclose') - ? pht('Off') - : pht('On'); - $autoclose = phutil_tag('em', array(), $autoclose); - $view->addProperty(pht('Autoclose'), $autoclose); - - return $this->newBox(pht('Actions'), $view); + return $this->newBox(pht('Publishing'), $view); } } diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php index b75228a0ce..bd539e0a25 100644 --- a/src/applications/diffusion/view/DiffusionBranchTableView.php +++ b/src/applications/diffusion/view/DiffusionBranchTableView.php @@ -60,7 +60,7 @@ final class DiffusionBranchTableView extends DiffusionView { break; case PhabricatorRepository::BECAUSE_AUTOCLOSE_DISABLED: $icon = 'fa-times bluegrey'; - $tip = pht('Repository Autoclose Disabled'); + $tip = pht('Repository Publishing Disabled'); break; case PhabricatorRepository::BECAUSE_BRANCH_UNTRACKED: $icon = 'fa-times bluegrey'; @@ -68,11 +68,11 @@ final class DiffusionBranchTableView extends DiffusionView { break; case PhabricatorRepository::BECAUSE_BRANCH_NOT_AUTOCLOSE: $icon = 'fa-times bluegrey'; - $tip = pht('Branch Autoclose Disabled'); + $tip = pht('Branch Not Permanent'); break; case null: $icon = 'fa-check bluegrey'; - $tip = pht('Autoclose Enabled'); + $tip = pht('Permanent Branch'); break; default: $icon = 'fa-question'; diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 46e499e4be..928a7a1140 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1048,13 +1048,17 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return false; } - if ($this->getDetail('herald-disabled')) { + if ($this->isPublishingDisabled()) { return false; } return true; } + public function isPublishingDisabled() { + return $this->getDetail('herald-disabled'); + } + public function shouldPublishCommit(PhabricatorRepositoryCommit $commit) { if (!$this->shouldPublish()) { return false; @@ -1186,7 +1190,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return self::BECAUSE_REPOSITORY_IMPORTING; } - if ($this->getDetail('disable-autoclose', false)) { + if ($this->isPublishingDisabled()) { return self::BECAUSE_AUTOCLOSE_DISABLED; } diff --git a/src/applications/repository/xaction/PhabricatorRepositoryAutocloseTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryAutocloseTransaction.php deleted file mode 100644 index 6ab677fb4a..0000000000 --- a/src/applications/repository/xaction/PhabricatorRepositoryAutocloseTransaction.php +++ /dev/null @@ -1,34 +0,0 @@ -getDetail('disable-autoclose'); - } - - public function generateNewValue($object, $value) { - return (int)$value; - } - - public function applyInternalEffects($object, $value) { - $object->setDetail('disable-autoclose', (int)!$value); - } - - public function getTitle() { - $new = $this->getNewValue(); - - if ($new) { - return pht( - '%s enabled autoclose for this repository.', - $this->renderAuthor()); - } else { - return pht( - '%s disabled autoclose for this repository.', - $this->renderAuthor()); - } - } - -} diff --git a/src/applications/repository/xaction/PhabricatorRepositoryNotifyTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryNotifyTransaction.php index 4ffe1918ff..96868cc407 100644 --- a/src/applications/repository/xaction/PhabricatorRepositoryNotifyTransaction.php +++ b/src/applications/repository/xaction/PhabricatorRepositoryNotifyTransaction.php @@ -22,11 +22,11 @@ final class PhabricatorRepositoryNotifyTransaction if ($new) { return pht( - '%s enabled notifications and publishing for this repository.', + '%s enabled publishing for this repository.', $this->renderAuthor()); } else { return pht( - '%s disabled notifications and publishing for this repository.', + '%s disabled publishing for this repository.', $this->renderAuthor()); } }