From 8ec987dd2fcec7d51d8a3250aac747c30cd04a76 Mon Sep 17 00:00:00 2001 From: Nick Pellegrino Date: Wed, 6 Mar 2013 14:14:09 -0800 Subject: [PATCH] Button to ignore setup issues + refractoring Summary: T2381 Test Plan: Click on the ignore link in /config/issue/ and respond to the dialog box. Also, test uninstalling and reinstalling an application in the web UI (to verify that refractoring didn't break anything). Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2381 Differential Revision: https://secure.phabricator.com/D5234 --- src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationConfig.php | 2 + .../PhabricatorConfigIgnoreController.php | 57 +++++++++++++++++++ .../PhabricatorConfigIssueListController.php | 12 ++++ .../config/editor/PhabricatorConfigEditor.php | 24 ++++++++ .../config/storage/PhabricatorConfigEntry.php | 16 ++++++ ...bricatorApplicationUninstallController.php | 50 +++------------- 7 files changed, 122 insertions(+), 41 deletions(-) create mode 100644 src/applications/config/controller/PhabricatorConfigIgnoreController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 866a0cc660..7b544147e1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -791,6 +791,7 @@ phutil_register_library_map(array( 'PhabricatorConfigEntryDAO' => 'applications/config/storage/PhabricatorConfigEntryDAO.php', 'PhabricatorConfigFileSource' => 'infrastructure/env/PhabricatorConfigFileSource.php', 'PhabricatorConfigGroupController' => 'applications/config/controller/PhabricatorConfigGroupController.php', + 'PhabricatorConfigIgnoreController' => 'applications/config/controller/PhabricatorConfigIgnoreController.php', 'PhabricatorConfigIssueListController' => 'applications/config/controller/PhabricatorConfigIssueListController.php', 'PhabricatorConfigIssueViewController' => 'applications/config/controller/PhabricatorConfigIssueViewController.php', 'PhabricatorConfigJSON' => 'applications/config/json/PhabricatorConfigJSON.php', @@ -2305,6 +2306,7 @@ phutil_register_library_map(array( 'PhabricatorConfigEntryDAO' => 'PhabricatorLiskDAO', 'PhabricatorConfigFileSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigGroupController' => 'PhabricatorConfigController', + 'PhabricatorConfigIgnoreController' => 'PhabricatorApplicationsController', 'PhabricatorConfigIssueListController' => 'PhabricatorConfigController', 'PhabricatorConfigIssueViewController' => 'PhabricatorConfigController', 'PhabricatorConfigListController' => 'PhabricatorConfigController', diff --git a/src/applications/config/application/PhabricatorApplicationConfig.php b/src/applications/config/application/PhabricatorApplicationConfig.php index 5c78cef272..d18b81c2df 100644 --- a/src/applications/config/application/PhabricatorApplicationConfig.php +++ b/src/applications/config/application/PhabricatorApplicationConfig.php @@ -29,6 +29,8 @@ final class PhabricatorApplicationConfig extends PhabricatorApplication { 'all/' => 'PhabricatorConfigAllController', 'edit/(?P[\w\.\-]+)/' => 'PhabricatorConfigEditController', 'group/(?P[^/]+)/' => 'PhabricatorConfigGroupController', + '(?Pignore|unignore)/(?P[^/]+)/' + => 'PhabricatorConfigIgnoreController', 'issue/' => array( '' => 'PhabricatorConfigIssueListController', '(?P[^/]+)/' => 'PhabricatorConfigIssueViewController', diff --git a/src/applications/config/controller/PhabricatorConfigIgnoreController.php b/src/applications/config/controller/PhabricatorConfigIgnoreController.php new file mode 100644 index 0000000000..4ab69f02a5 --- /dev/null +++ b/src/applications/config/controller/PhabricatorConfigIgnoreController.php @@ -0,0 +1,57 @@ +verb = $data['verb']; + $this->issue = $data['key']; + } + + public function processRequest() { + $request = $this->getRequest(); + $issue_uri = $this->getApplicationURI('issue'); + + if ($request->isDialogFormPost()) { + $this->manageApplication(); + return id(new AphrontRedirectResponse())->setURI($issue_uri); + } + + // User just clicked the link, so show them the dialog. + if ($this->verb == 'ignore') { + $title = pht('Really ignore this setup issue?'); + $submit_title = pht('Ignore'); + } else if ($this->verb == 'unignore') { + $title = pht('Really unignore this setup issue?'); + $submit_title = pht('Unignore'); + } else { + throw new Exception('Unrecognized verb: ' . $this->verb); + } + $dialog = new AphrontDialogView(); + $dialog->setTitle($title) + ->setUser($request->getUser()) + ->addSubmitButton($submit_title) + ->addCancelButton($issue_uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + + public function manageApplication() { + $key = 'config.ignore-issues'; + $config_entry = PhabricatorConfigEntry::loadConfigEntry($key); + $list = $config_entry->getValue(); + + if (isset($list[$this->issue])) { + unset($list[$this->issue]); + } else { + $list[$this->issue] = true; + } + + PhabricatorConfigEditor::storeNewValue( + $config_entry, $list, $this->getRequest()); + } + +} diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/PhabricatorConfigIssueListController.php index 284c4b0cbd..984ccfbbd9 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueListController.php +++ b/src/applications/config/controller/PhabricatorConfigIssueListController.php @@ -60,9 +60,21 @@ final class PhabricatorConfigIssueListController ->addAttribute($issue->getSummary()); if (!$issue->getIsIgnored()) { $item->addIcon('warning', pht('Setup Warning')); + $link = javelin_tag( + 'a', + array('href' => '/config/ignore/'.$issue->getIssueKey().'/', + 'sigil' => 'workflow'), + pht('Ignore')); + $item->addAttribute($link); $list->addItem($item); } else { $item->addIcon('none', pht('Ignored')); + $link = javelin_tag( + 'a', + array('href' => '/config/unignore/'.$issue->getIssueKey().'/', + 'sigil' => 'workflow'), + pht('Unignore')); + $item->addAttribute($link); $ignored_items[] = $item; } } diff --git a/src/applications/config/editor/PhabricatorConfigEditor.php b/src/applications/config/editor/PhabricatorConfigEditor.php index cb21eb2a0d..2468e9eb79 100644 --- a/src/applications/config/editor/PhabricatorConfigEditor.php +++ b/src/applications/config/editor/PhabricatorConfigEditor.php @@ -104,4 +104,28 @@ final class PhabricatorConfigEditor PhabricatorSetupCheck::deleteSetupCheckCache(); } + public static function storeNewValue( + PhabricatorConfigEntry $config_entry, $value, AphrontRequest $request) { + $xaction = id(new PhabricatorConfigTransaction()) + ->setTransactionType(PhabricatorConfigTransaction::TYPE_EDIT) + ->setNewValue( + array( + 'deleted' => false, + 'value' => $value + )); + + $editor = id(new PhabricatorConfigEditor()) + ->setActor($request->getUser()) + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + ))); + + + $editor->applyTransactions($config_entry, array($xaction)); + } + } diff --git a/src/applications/config/storage/PhabricatorConfigEntry.php b/src/applications/config/storage/PhabricatorConfigEntry.php index 096fae7f83..bd5756af58 100644 --- a/src/applications/config/storage/PhabricatorConfigEntry.php +++ b/src/applications/config/storage/PhabricatorConfigEntry.php @@ -23,4 +23,20 @@ final class PhabricatorConfigEntry extends PhabricatorConfigEntryDAO { PhabricatorPHIDConstants::PHID_TYPE_CONF); } + public static function loadConfigEntry($key) { + $config_entry = id(new PhabricatorConfigEntry()) + ->loadOneWhere( + 'configKey = %s AND namespace = %s', + $key, + 'default'); + + if (!$config_entry) { + $config_entry = id(new PhabricatorConfigEntry()) + ->setConfigKey($key) + ->setNamespace('default'); + } + + return $config_entry; + } + } diff --git a/src/applications/meta/controller/PhabricatorApplicationUninstallController.php b/src/applications/meta/controller/PhabricatorApplicationUninstallController.php index 98e607150f..2bee35714b 100644 --- a/src/applications/meta/controller/PhabricatorApplicationUninstallController.php +++ b/src/applications/meta/controller/PhabricatorApplicationUninstallController.php @@ -61,50 +61,18 @@ final class PhabricatorApplicationUninstallController public function manageApplication() { $key = 'phabricator.uninstalled-applications'; + $config_entry = PhabricatorConfigEntry::loadConfigEntry($key); + $list = $config_entry->getValue(); + $uninstalled = PhabricatorEnv::getEnvConfig($key); - $config_entry = id(new PhabricatorConfigEntry()) - ->loadOneWhere( - 'configKey = %s AND namespace = %s', - $key, - 'default'); - - if (!$config_entry) { - $config_entry = id(new PhabricatorConfigEntry()) - ->setConfigKey($key) - ->setNamespace('default'); - } - - $list = $config_entry->getValue(); - - $uninstalled = PhabricatorEnv::getEnvConfig($key); - - if ($uninstalled[$this->application]) { - unset($list[$this->application]); - } else { + if ($uninstalled[$this->application]) { + unset($list[$this->application]); + } else { $list[$this->application] = true; - } - - $xaction = id(new PhabricatorConfigTransaction()) - ->setTransactionType(PhabricatorConfigTransaction::TYPE_EDIT) - ->setNewValue( - array( - 'deleted' => false, - 'value' => $list - )); - - $editor = id(new PhabricatorConfigEditor()) - ->setActor($this->getRequest()->getUser()) - ->setContinueOnNoEffect(true) - ->setContentSource( - PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_WEB, - array( - 'ip' => $this->getRequest()->getRemoteAddr(), - ))); - - - $editor->applyTransactions($config_entry, array($xaction)); + } + PhabricatorConfigEditor::storeNewValue( + $config_entry, $list, $this->getRequest()); } }