diff --git a/resources/sql/patches/123.heraldrulelog.sql b/resources/sql/patches/123.heraldrulelog.sql new file mode 100644 index 0000000000..0d3f1a873d --- /dev/null +++ b/resources/sql/patches/123.heraldrulelog.sql @@ -0,0 +1,5 @@ +ALTER TABLE phabricator_herald.herald_ruleedit + ADD ruleName VARCHAR(255) NOT NULL COLLATE utf8_general_ci; + +ALTER TABLE phabricator_herald.herald_ruleedit + ADD action VARCHAR(32) NOT NULL COLLATE utf8_general_ci; diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 0b0c457f18..7272b26706 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -304,7 +304,7 @@ celerity_register_resource_map(array( ), 'herald-css' => array( - 'uri' => '/res/ed5556e6/rsrc/css/application/herald/herald.css', + 'uri' => '/res/5051f3ab/rsrc/css/application/herald/herald.css', 'type' => 'css', 'requires' => array( @@ -313,7 +313,7 @@ celerity_register_resource_map(array( ), 'herald-rule-editor' => array( - 'uri' => '/res/745c9d43/rsrc/js/application/herald/HeraldRuleEditor.js', + 'uri' => '/res/209ed8c4/rsrc/js/application/herald/HeraldRuleEditor.js', 'type' => 'js', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2e6c12832f..7e6774f193 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -395,6 +395,7 @@ phutil_register_library_map(array( 'HeraldDeleteController' => 'applications/herald/controller/delete', 'HeraldDifferentialRevisionAdapter' => 'applications/herald/adapter/differential', 'HeraldDryRunAdapter' => 'applications/herald/adapter/dryrun', + 'HeraldEditLogQuery' => 'applications/herald/query/log', 'HeraldEffect' => 'applications/herald/engine/effect', 'HeraldEngine' => 'applications/herald/engine/engine', 'HeraldFieldConfig' => 'applications/herald/config/field', @@ -412,6 +413,7 @@ phutil_register_library_map(array( 'HeraldRuleEditHistoryController' => 'applications/herald/controller/edithistory', 'HeraldRuleEditHistoryView' => 'applications/herald/view/edithistory', 'HeraldRuleListView' => 'applications/herald/view/rulelist', + 'HeraldRuleQuery' => 'applications/herald/query/rule', 'HeraldRuleTranscript' => 'applications/herald/storage/transcript/rule', 'HeraldRuleTypeConfig' => 'applications/herald/config/ruletype', 'HeraldTestConsoleController' => 'applications/herald/controller/test', @@ -1188,6 +1190,7 @@ phutil_register_library_map(array( 'DiffusionContainsQuery' => 'DiffusionQuery', 'DiffusionController' => 'PhabricatorController', 'DiffusionDiffController' => 'DiffusionController', + 'DiffusionDiffQuery' => 'DiffusionQuery', 'DiffusionEmptyResultView' => 'DiffusionView', 'DiffusionExternalController' => 'DiffusionController', 'DiffusionFileContentQuery' => 'DiffusionQuery', @@ -1208,6 +1211,7 @@ phutil_register_library_map(array( 'DiffusionHomeController' => 'DiffusionController', 'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController', 'DiffusionLastModifiedController' => 'DiffusionController', + 'DiffusionLastModifiedQuery' => 'DiffusionQuery', 'DiffusionMercurialBranchQuery' => 'DiffusionBranchQuery', 'DiffusionMercurialBrowseQuery' => 'DiffusionBrowseQuery', 'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery', @@ -1268,6 +1272,7 @@ phutil_register_library_map(array( 'HeraldDeleteController' => 'HeraldController', 'HeraldDifferentialRevisionAdapter' => 'HeraldObjectAdapter', 'HeraldDryRunAdapter' => 'HeraldObjectAdapter', + 'HeraldEditLogQuery' => 'PhabricatorOffsetPagedQuery', 'HeraldHomeController' => 'HeraldController', 'HeraldNewController' => 'HeraldController', 'HeraldRule' => 'HeraldDAO', @@ -1276,6 +1281,7 @@ phutil_register_library_map(array( 'HeraldRuleEditHistoryController' => 'HeraldController', 'HeraldRuleEditHistoryView' => 'AphrontView', 'HeraldRuleListView' => 'AphrontView', + 'HeraldRuleQuery' => 'PhabricatorOffsetPagedQuery', 'HeraldTestConsoleController' => 'HeraldController', 'HeraldTranscript' => 'HeraldDAO', 'HeraldTranscriptController' => 'HeraldController', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 7137b8e1bc..0eda5504e1 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -282,19 +282,14 @@ class AphrontDefaultApplicationConfiguration '/herald/' => array( '' => 'HeraldHomeController', - 'view/(?P[^/]+)/' => array( - '' => 'HeraldHomeController', - '(?Pglobal)/' => 'HeraldHomeController' - ), - 'new/(?:(?P[^/]+)/)?' => 'HeraldNewController', + 'view/(?P[^/]+)/(?:(?P[^/]+)/)?' + => 'HeraldHomeController', + 'new/(?:(?P[^/]+)/(?:(?P[^/]+)/)?)?' + => 'HeraldNewController', 'rule/(?:(?P\d+)/)?' => 'HeraldRuleController', - 'history/(?P\d+)/' => 'HeraldRuleEditHistoryController', + 'history/(?:(?P\d+)/)?' => 'HeraldRuleEditHistoryController', 'delete/(?P\d+)/' => 'HeraldDeleteController', 'test/' => 'HeraldTestConsoleController', - 'all/' => array( - '' => 'HeraldAllRulesController', - 'view/(?P[^/]+)/' => 'HeraldAllRulesController', - ), 'transcript/' => 'HeraldTranscriptListController', 'transcript/(?P\d+)/(?:(?P\w+)/)?' => 'HeraldTranscriptController', diff --git a/src/applications/herald/controller/all/HeraldAllRulesController.php b/src/applications/herald/controller/all/HeraldAllRulesController.php deleted file mode 100644 index 8a651efcd6..0000000000 --- a/src/applications/herald/controller/all/HeraldAllRulesController.php +++ /dev/null @@ -1,173 +0,0 @@ -filter; - } - public function setFilter($filter) { - $this->filter = 'all/view/'.$filter; - return $this; - } - - public function shouldRequireAdmin() { - return true; - } - - public function willProcessRequest(array $data) { - $this->view = idx($data, 'view'); - $this->setFilter($this->view); - } - - public function processRequest() { - - $request = $this->getRequest(); - $user = $request->getUser(); - $this->viewPHID = nonempty($request->getStr('phid'), null); - - if ($request->isFormPost()) { - $phid_arr = $request->getArr('view_user'); - $view_target = head($phid_arr); - return id(new AphrontRedirectResponse()) - ->setURI($request->getRequestURI()->alter('phid', $view_target)); - } - - $map = HeraldContentTypeConfig::getContentTypeMap(); - if (empty($map[$this->view])) { - reset($map); - $this->view = key($map); - } - - $offset = $request->getInt('offset', 0); - $pager = new AphrontPagerView(); - $pager->setPageSize(50); - $pager->setOffset($offset); - $pager->setURI($request->getRequestURI(), 'offset'); - - list($rules, $handles) = $this->queryRules($pager); - - if (!$this->viewPHID) { - $view_users = array(); - } else { - $view_users = array( - $this->viewPHID => $handles[$this->viewPHID]->getFullName(), - ); - } - - $filter_form = id(new AphrontFormView()) - ->setUser($user) - ->appendChild( - id(new AphrontFormTokenizerControl()) - ->setDatasource('/typeahead/common/users/') - ->setLabel('View User') - ->setName('view_user') - ->setValue($view_users) - ->setLimit(1)); - $filter_view = new AphrontListFilterView(); - $filter_view->appendChild($filter_form); - - $list_view = id(new HeraldRuleListView()) - ->setRules($rules) - ->setHandles($handles) - ->setMap($map) - ->setShowType(true) - ->setAllowCreation(false) - ->setUser($user) - ->setView($this->view); - $panel = $list_view->render(); - $panel->appendChild($pager); - - $sidenav = new AphrontSideNavView(); - $sidenav->appendChild($filter_view); - $sidenav->appendChild($panel); - - $query = ''; - if ($this->viewPHID) { - $query = '?phid='.$this->viewPHID; - } - - foreach ($map as $key => $value) { - $sidenav->addNavItem( - phutil_render_tag( - 'a', - array( - 'href' => '/herald/all/view/'.$key.'/'.$query, - 'class' => ($key == $this->view) - ? 'aphront-side-nav-selected' - : null, - ), - phutil_escape_html($value))); - } - - return $this->buildStandardPageResponse( - array( - $filter_view, - $panel - ), - array( - 'title' => 'Herald', - 'tab' => 'all', - )); - } - - private function queryRules(AphrontPagerView $pager) { - $rule = new HeraldRule(); - $conn_r = $rule->establishConnection('r'); - - $where_clause = qsprintf( - $conn_r, - 'WHERE contentType = %s', - $this->view); - - if ($this->viewPHID) { - $where_clause .= qsprintf( - $conn_r, - ' AND authorPHID = %s', - $this->viewPHID); - } - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T - %Q - ORDER BY id DESC - LIMIT %d, %d', - $rule->getTableName(), - $where_clause, - $pager->getOffset(), - $pager->getPageSize() + 1); - $data = $pager->sliceResults($data); - $rules = $rule->loadAllFromArray($data); - - $need_phids = mpull($rules, 'getAuthorPHID'); - if ($this->viewPHID) { - $need_phids[] = $this->viewPHID; - } - - $handles = id(new PhabricatorObjectHandleData($need_phids)) - ->loadHandles(); - - return array($rules, $handles); - } -} - diff --git a/src/applications/herald/controller/all/__init__.php b/src/applications/herald/controller/all/__init__.php deleted file mode 100644 index a0f8c85f22..0000000000 --- a/src/applications/herald/controller/all/__init__.php +++ /dev/null @@ -1,27 +0,0 @@ -setBaseURI(new PhutilURI('/herald/')) - ->addLabel('My Rules') - ->addFilter('new', 'Create Rule'); - $rules_map = HeraldContentTypeConfig::getContentTypeMap(); - $first_filter = null; - foreach ($rules_map as $key => $value) { - $nav->addFilter('view/'.$key, $value); - if (!$first_filter) { - $first_filter = 'view/'.$key; - } - } - $nav - ->addSpacer() - ->addLabel('Global Rules'); - foreach ($rules_map as $key => $value) { - $nav->addFilter("view/${key}/global", $value); - } - - $nav - ->addSpacer() - ->addLabel('Utilities') - ->addFilter('test', 'Test Console') - ->addFilter('transcript', 'Transcripts'); - - $user = $this->getRequest()->getUser(); - if ($user->getIsAdmin()) { - $nav - ->addSpacer() - ->addLabel('Admin'); - $view_PHID = nonempty($this->getRequest()->getStr('phid'), null); - foreach ($rules_map as $key => $value) { - $nav - ->addFilter('all/view/'.$key, $value); - } - } - - $nav->selectFilter($this->getFilter(), $first_filter); - $nav->appendChild($view); - $page->appendChild($nav); - $tabs = array( 'help' => array( 'href' => $doclink, @@ -78,10 +36,49 @@ abstract class HeraldController extends PhabricatorController { ); $page->setTabs($tabs, null); + $page->appendChild($view); + $page->setIsAdminInterface(idx($data, 'admin')); + $response = new AphrontWebpageResponse(); return $response->setContent($page->render()); - } - abstract function getFilter(); + protected function renderNav() { + $nav = id(new AphrontSideNavFilterView()) + ->setBaseURI(new PhutilURI('/herald/')) + ->addLabel('My Rules') + ->addFilter('new', 'Create Rule'); + + $rules_map = HeraldContentTypeConfig::getContentTypeMap(); + foreach ($rules_map as $key => $value) { + $nav->addFilter("view/{$key}/personal", $value); + } + + $nav + ->addSpacer() + ->addLabel('Global Rules'); + + foreach ($rules_map as $key => $value) { + $nav->addFilter("view/{$key}/global", $value); + } + + $nav + ->addSpacer() + ->addLabel('Utilities') + ->addFilter('test', 'Test Console') + ->addFilter('transcript', 'Transcripts') + ->addFilter('history', 'Edit Log'); + + if ($this->getRequest()->getUser()->getIsAdmin()) { + $nav + ->addSpacer() + ->addLabel('Admin'); + foreach ($rules_map as $key => $value) { + $nav->addFilter("view/{$key}/all", $value); + } + } + + return $nav; + } + } diff --git a/src/applications/herald/controller/delete/HeraldDeleteController.php b/src/applications/herald/controller/delete/HeraldDeleteController.php index baf7f52b49..4ce1702af3 100644 --- a/src/applications/herald/controller/delete/HeraldDeleteController.php +++ b/src/applications/herald/controller/delete/HeraldDeleteController.php @@ -40,17 +40,18 @@ final class HeraldDeleteController extends HeraldController { $request = $this->getRequest(); $user = $request->getUser(); - if ($user->getPHID() != $rule->getAuthorPHID() && !$user->getIsAdmin()) { - return new Aphront400Response(); + // Anyone can delete a global rule, but only the rule owner can delete a + // personal one. + if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { + if ($user->getPHID() != $rule->getAuthorPHID()) { + return new Aphront400Response(); + } } if ($request->isFormPost()) { + $rule->logEdit($user->getPHID(), 'delete'); $rule->delete(); - if ($request->isAjax()) { - return new AphrontRedirectResponse(); - } else { - return id(new AphrontRedirectResponse())->setURI('/herald/'); - } + return id(new AphrontReloadResponse())->setURI('/herald/'); } $dialog = new AphrontDialogView(); diff --git a/src/applications/herald/controller/delete/__init__.php b/src/applications/herald/controller/delete/__init__.php index 739c92f27d..d9e5b02ebd 100644 --- a/src/applications/herald/controller/delete/__init__.php +++ b/src/applications/herald/controller/delete/__init__.php @@ -9,7 +9,8 @@ phutil_require_module('phabricator', 'aphront/response/400'); phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'aphront/response/dialog'); -phutil_require_module('phabricator', 'aphront/response/redirect'); +phutil_require_module('phabricator', 'aphront/response/reload'); +phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); phutil_require_module('phabricator', 'applications/herald/storage/rule'); phutil_require_module('phabricator', 'view/dialog'); diff --git a/src/applications/herald/controller/edithistory/HeraldRuleEditHistoryController.php b/src/applications/herald/controller/edithistory/HeraldRuleEditHistoryController.php index 69ce352c80..c296be0e7a 100644 --- a/src/applications/herald/controller/edithistory/HeraldRuleEditHistoryController.php +++ b/src/applications/herald/controller/edithistory/HeraldRuleEditHistoryController.php @@ -21,35 +21,45 @@ final class HeraldRuleEditHistoryController extends HeraldController { private $id; public function willProcessRequest(array $data) { - $this->id = $data['id']; + $this->id = idx($data, 'id'); } public function processRequest() { - $rule = id(new HeraldRule())->load($this->id); - if ($rule === null) { - return new Aphront404Response(); + $request = $this->getRequest(); + + $edit_query = new HeraldEditLogQuery(); + if ($this->id) { + $edit_query->withRuleIDs(array($this->id)); } - $edits = $rule->loadEdits(); - $rule->attachEdits($edits); + $pager = new AphrontPagerView(); + $pager->setURI($request->getRequestURI(), 'offset'); + $pager->setOffset($request->getStr('offset')); + + $edits = $edit_query->executeWithPager($pager); $need_phids = mpull($edits, 'getEditorPHID'); $handles = id(new PhabricatorObjectHandleData($need_phids)) ->loadHandles(); $list_view = id(new HeraldRuleEditHistoryView()) - ->setRule($rule) + ->setEdits($edits) ->setHandles($handles) ->setUser($this->getRequest()->getUser()); + $panel = new AphrontPanelView(); + $panel->setHeader('Edit History'); + $panel->appendChild($list_view); + + $nav = $this->renderNav(); + $nav->selectFilter('history'); + $nav->appendChild($panel); + return $this->buildStandardPageResponse( - $list_view->render(), + $nav, array( 'title' => 'Rule Edit History', )); } - public function getFilter() { - return; - } } diff --git a/src/applications/herald/controller/edithistory/__init__.php b/src/applications/herald/controller/edithistory/__init__.php index a85481c98a..9cfe9affd1 100644 --- a/src/applications/herald/controller/edithistory/__init__.php +++ b/src/applications/herald/controller/edithistory/__init__.php @@ -6,11 +6,12 @@ -phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/herald/controller/base'); -phutil_require_module('phabricator', 'applications/herald/storage/rule'); +phutil_require_module('phabricator', 'applications/herald/query/log'); phutil_require_module('phabricator', 'applications/herald/view/edithistory'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'view/control/pager'); +phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/herald/controller/home/HeraldHomeController.php b/src/applications/herald/controller/home/HeraldHomeController.php index 39ea74383c..8d15a70ad4 100644 --- a/src/applications/herald/controller/home/HeraldHomeController.php +++ b/src/applications/herald/controller/home/HeraldHomeController.php @@ -18,56 +18,82 @@ final class HeraldHomeController extends HeraldController { - private $view; - private $filter; - private $global; + private $contentType; + private $ruleType; public function willProcessRequest(array $data) { - $this->view = idx($data, 'view'); - $this->global = idx($data, 'global'); - if ($this->global) { - $this->setFilter($this->view.'/global'); - } else { - $this->setFilter($this->view); - } - } - - public function getFilter() { - return $this->filter; - } - public function setFilter($filter) { - $this->filter = 'view/'.$filter; - return $this; + $this->contentType = idx($data, 'content_type'); + $this->ruleType = idx($data, 'rule_type'); } public function processRequest() { - $request = $this->getRequest(); $user = $request->getUser(); - $map = HeraldContentTypeConfig::getContentTypeMap(); - if (empty($map[$this->view])) { - reset($map); - $this->view = key($map); + if ($request->isFormPost()) { + $phids = $request->getArr('set_phid'); + $phid = head($phids); + + $uri = $request->getRequestURI(); + if ($phid) { + $uri = $uri->alter('phid', nonempty($phid, null)); + } + + return id(new AphrontRedirectResponse())->setURI($uri); } - if ($this->global) { - $rules = id(new HeraldRule())->loadAllWhere( - 'contentType = %s AND ruleType = %s', - $this->view, - HeraldRuleTypeConfig::RULE_TYPE_GLOBAL); - } else { - $rules = id(new HeraldRule())->loadAllWhere( - 'contentType = %s AND authorPHID = %s AND ruleType = %s', - $this->view, - $user->getPHID(), - HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); + $query = new HeraldRuleQuery(); + + $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); + if (empty($content_type_map[$this->contentType])) { + $this->contentType = key($content_type_map); + } + $content_desc = $content_type_map[$this->contentType]; + + $query->withContentTypes(array($this->contentType)); + + $is_admin_page = false; + $show_author = false; + $show_rule_type = false; + $can_create = false; + $has_author_filter = false; + $author_filter_phid = null; + + switch ($this->ruleType) { + case 'all': + if (!$user->getIsAdmin()) { + return new Aphront400Response(); + } + $is_admin_page = true; + $show_rule_type = true; + $show_author = true; + $has_author_filter = true; + $author_filter_phid = $request->getStr('phid'); + if ($author_filter_phid) { + $query->withAuthorPHIDs(array($author_filter_phid)); + } + $rule_desc = 'All'; + break; + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + $query->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_GLOBAL)); + $can_create = true; + $rule_desc = 'Global'; + break; + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + default: + $this->ruleType = HeraldRuleTypeConfig::RULE_TYPE_PERSONAL; + $query->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_PERSONAL)); + $query->withAuthorPHIDs(array($user->getPHID())); + $can_create = true; + $rule_desc = 'Personal'; + break; } - foreach ($rules as $rule) { - $edits = $rule->loadEdits(); - $rule->attachEdits($edits); - } + $pager = new AphrontPagerView(); + $pager->setURI($request->getRequestURI(), 'offset'); + $pager->setOffset($request->getStr('offset')); + + $rules = $query->executeWithPager($pager); $need_phids = mpull($rules, 'getAuthorPHID'); $handles = id(new PhabricatorObjectHandleData($need_phids)) @@ -75,19 +101,73 @@ final class HeraldHomeController extends HeraldController { $list_view = id(new HeraldRuleListView()) ->setRules($rules) - ->setShowOwner(!$this->global) + ->setShowAuthor($show_author) + ->setShowRuleType($show_rule_type) ->setHandles($handles) - ->setMap($map) - ->setAllowCreation(true) - ->setView($this->view) ->setUser($user); - $panel = $list_view->render(); + $panel = new AphrontPanelView(); + $panel->appendChild($list_view); + $panel->appendChild($pager); + + $panel->setHeader("Herald: {$rule_desc} Rules for {$content_desc}"); + + if ($can_create) { + $panel->addButton( + phutil_render_tag( + 'a', + array( + 'href' => '/herald/new/'.$this->contentType.'/'.$this->ruleType.'/', + 'class' => 'green button', + ), + 'Create New Herald Rule')); + } + + + $nav = $this->renderNav(); + $nav->selectFilter('view/'.$this->contentType.'/'.$this->ruleType); + + if ($has_author_filter) { + $nav->appendChild($this->renderAuthorFilter($author_filter_phid)); + } + + $nav->appendChild($panel); return $this->buildStandardPageResponse( - $panel, + $nav, array( 'title' => 'Herald', + 'admin' => $is_admin_page, )); } + + private function renderAuthorFilter($phid) { + if ($phid) { + $handle = PhabricatorObjectHandleData::loadOneHandle($phid); + $tokens = array( + $phid => $handle->getFullName(), + ); + } else { + $tokens = array(); + } + + $form = id(new AphrontFormView()) + ->setUser($this->getRequest()->getUser()) + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setName('set_phid') + ->setValue($tokens) + ->setLimit(1) + ->setLabel('Filter Author') + ->setDataSource('/typeahead/common/accounts/')) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue('Apply Filter')); + + $filter = new AphrontListFilterView(); + $filter->appendChild($form); + return $filter; + } + + } diff --git a/src/applications/herald/controller/home/__init__.php b/src/applications/herald/controller/home/__init__.php index 802301ce76..ede35ebecc 100644 --- a/src/applications/herald/controller/home/__init__.php +++ b/src/applications/herald/controller/home/__init__.php @@ -6,13 +6,22 @@ +phutil_require_module('phabricator', 'aphront/response/400'); +phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/herald/config/contenttype'); phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); -phutil_require_module('phabricator', 'applications/herald/storage/rule'); +phutil_require_module('phabricator', 'applications/herald/query/rule'); phutil_require_module('phabricator', 'applications/herald/view/rulelist'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'view/control/pager'); +phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/submit'); +phutil_require_module('phabricator', 'view/form/control/tokenizer'); +phutil_require_module('phabricator', 'view/layout/listfilter'); +phutil_require_module('phabricator', 'view/layout/panel'); +phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/herald/controller/new/HeraldNewController.php b/src/applications/herald/controller/new/HeraldNewController.php index 2dc868f96c..be74cfdec7 100644 --- a/src/applications/herald/controller/new/HeraldNewController.php +++ b/src/applications/herald/controller/new/HeraldNewController.php @@ -19,13 +19,11 @@ final class HeraldNewController extends HeraldController { private $contentType; - - public function getFilter() { - return 'new'; - } + private $ruleType; public function willProcessRequest(array $data) { $this->contentType = idx($data, 'type'); + $this->ruleType = idx($data, 'rule_type'); } public function processRequest() { @@ -40,6 +38,9 @@ final class HeraldNewController extends HeraldController { } $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); + if (empty($rule_type_map[$this->ruleType])) { + $this->ruleType = HeraldRuleTypeConfig::RULE_TYPE_PERSONAL; + } // Reorder array to put "personal" first. $rule_type_map = array_select_keys( @@ -61,7 +62,7 @@ final class HeraldNewController extends HeraldController { $radio = id(new AphrontFormRadioButtonControl()) ->setLabel('Type') ->setName('rule_type') - ->setValue(HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); + ->setValue($this->ruleType); foreach ($rule_type_map as $value => $name) { $radio->addButton( @@ -90,8 +91,12 @@ final class HeraldNewController extends HeraldController { $panel->setWidth(AphrontPanelView::WIDTH_FULL); $panel->appendChild($form); + $nav = $this->renderNav(); + $nav->selectFilter('new'); + $nav->appendChild($panel); + return $this->buildStandardPageResponse( - $panel, + $nav, array( 'title' => 'Create Herald Rule', )); diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index d1659c9090..b93d0f9adf 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -21,13 +21,6 @@ final class HeraldRuleController extends HeraldController { private $id; private $filter; - public function getFilter() { - return $this->filter; - } - public function setFilter($filter) { - $this->filter = 'view/'.$filter; - } - public function willProcessRequest(array $data) { $this->id = (int)idx($data, 'id'); } @@ -65,13 +58,13 @@ final class HeraldRuleController extends HeraldController { } $rule->setRuleType($rule_type); } - $this->setFilter($rule->getContentType()); $local_version = id(new HeraldRule())->getConfigVersion(); if ($rule->getConfigVersion() > $local_version) { throw new Exception( "This rule was created with a newer version of Herald. You can not ". - "view or edit it in this older version. Try dev or wait for a push."); + "view or edit it in this older version. Upgrade your Phabricator ". + "deployment."); } // Upgrade rule version to our version, since we might add newly-defined @@ -88,16 +81,11 @@ final class HeraldRuleController extends HeraldController { $errors = array(); if ($request->isFormPost() && $request->getStr('save')) { list($e_name, $errors) = $this->saveRule($rule, $request); - if (!$errors) { - $uri = '/herald/view/'.$rule->getContentType().'/'; - - if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) { - $uri .= 'global/'; - } - - return id(new AphrontRedirectResponse()) - ->setURI($uri); + $uri = '/herald/view/'. + $rule->getContentType().'/'. + $rule->getRuleType().'/'; + return id(new AphrontRedirectResponse())->setURI($uri); } } @@ -109,8 +97,8 @@ final class HeraldRuleController extends HeraldController { $error_view = null; } - $must_match_selector = $this->getMustMatchSelector($rule); - $repetition_selector = $this->getRepetitionSelector($rule); + $must_match_selector = $this->renderMustMatchSelector($rule); + $repetition_selector = $this->renderRepetitionSelector($rule); $handles = $this->loadHandles($rule); @@ -126,7 +114,8 @@ final class HeraldRuleController extends HeraldController { ->addHiddenInput('rule_type', $rule->getRuleType()) ->addHiddenInput('save', 1) ->appendChild( - // Build this explicitly so we can add a sigil to it. + // Build this explicitly (instead of using addHiddenInput()) + // so we can add a sigil to it. javelin_render_tag( 'input', array( @@ -141,23 +130,6 @@ final class HeraldRuleController extends HeraldController { ->setError($e_name) ->setValue($rule->getName())); - if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { - $form - ->appendChild( - id(new AphrontFormMarkupControl()) - ->setLabel('Owner') - ->setValue('
')) - ->appendChild( - // Build this explicitly so we can add a sigil to it. - javelin_render_tag( - 'input', - array( - 'type' => 'hidden', - 'name' => 'author', - 'sigil' => 'author', - ))); - } - $form ->appendChild( id(new AphrontFormMarkupControl()) @@ -215,15 +187,24 @@ final class HeraldRuleController extends HeraldController { $this->setupEditorBehavior($rule, $handles); $panel = new AphrontPanelView(); - $panel->setHeader('Edit Herald Rule'); + $panel->setHeader( + $rule->getID() + ? 'Edit Herald Rule' + : 'Create Herald Rule'); $panel->setWidth(AphrontPanelView::WIDTH_WIDE); $panel->appendChild($form); - return $this->buildStandardPageResponse( + $nav = $this->renderNav(); + $nav->selectFilter( + 'view/'.$rule->getContentType().'/'.$rule->getRuleType()); + $nav->appendChild( array( $error_view, $panel, - ), + )); + + return $this->buildStandardPageResponse( + $nav, array( 'title' => 'Edit Rule', )); @@ -231,9 +212,9 @@ final class HeraldRuleController extends HeraldController { private function canEditRule($rule, $user) { return - $user->getIsAdmin() || - $rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL || - $rule->getAuthorPHID() == $user->getPHID(); + ($user->getIsAdmin()) || + ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) || + ($rule->getAuthorPHID() == $user->getPHID()); } private function saveRule($rule, $request) { @@ -322,11 +303,6 @@ final class HeraldRuleController extends HeraldController { $conditions[] = $obj; } - $author = $request->getStr('author'); - if ($author) { - $rule->setAuthorPHID($author); - } - $actions = array(); foreach ($data['actions'] as $action) { if ($action === null) { @@ -351,13 +327,14 @@ final class HeraldRuleController extends HeraldController { if (!$errors) { try { -// TODO -// $rule->openTransaction(); + $edit_action = $rule->getID() ? 'edit' : 'create'; + + $rule->openTransaction(); $rule->save(); $rule->saveConditions($conditions); $rule->saveActions($actions); - $rule->saveEdit($request->getUser()->getPHID()); -// $rule->saveTransaction(); + $rule->logEdit($request->getUser()->getPHID(), $edit_action); + $rule->saveTransaction(); } catch (AphrontQueryDuplicateKeyException $ex) { $e_name = "Not Unique"; @@ -413,14 +390,9 @@ final class HeraldRuleController extends HeraldController { } } - $all_rules = id(new HeraldRule())->loadAllWhere( - 'authorPHID = %s AND contentType = %s', - $rule->getAuthorPHID(), - $rule->getContentType()); + $all_rules = $this->loadRulesThisRuleMayDependUpon($rule); $all_rules = mpull($all_rules, 'getName', 'getID'); asort($all_rules); - unset($all_rules[$rule->getID()]); - $config_info = array(); $config_info['fields'] @@ -468,7 +440,6 @@ final class HeraldRuleController extends HeraldController { private function loadHandles($rule) { $phids = array(); - $phids[] = $rule->getAuthorPHID(); foreach ($rule->getActions() as $action) { if (!is_array($action->getTarget())) { @@ -491,80 +462,61 @@ final class HeraldRuleController extends HeraldController { } } - $phids += array($rule->getAuthorPHID()); - $handles = id(new PhabricatorObjectHandleData($phids)) - ->loadHandles(); - return $handles; + $phids[] = $rule->getAuthorPHID(); + + return id(new PhabricatorObjectHandleData($phids))->loadHandles(); } - private function getMustMatchSelector($rule) { - $options = array( - 'all' => 'all of', - 'any' => 'any of', - ); - $selected = $rule->getMustMatchAll() ? 'all' : 'any'; - - $must_match = array(); - foreach ($options as $key => $option) { - $must_match[] = phutil_render_tag( - 'option', - array( - 'selected' => ($selected == $key) ? 'selected' : null, - 'value' => $key, - ), - phutil_escape_html($option)); - } - $must_match = - ''; - return $must_match; + /** + * Render the selector for the "When (all of | any of) these conditions are + * met:" element. + */ + private function renderMustMatchSelector($rule) { + return AphrontFormSelectControl::renderSelectTag( + $rule->getMustMatchAll() ? 'all' : 'any', + array( + 'all' => 'all of', + 'any' => 'any of', + ), + array( + 'name' => 'must_match', + )); } - private function getRepetitionSelector($rule) { + + /** + * Render the selector for "Take these actions (every time | only the first + * time) this rule matches..." element. + */ + private function renderRepetitionSelector($rule) { // Make the selector for choosing how often this rule should be repeated $repetition_policy = HeraldRepetitionPolicyConfig::toString( - $rule->getRepetitionPolicy() - ); + $rule->getRepetitionPolicy()); $repetition_options = HeraldRepetitionPolicyConfig::getMapForContentType( - $rule->getContentType() - ); + $rule->getContentType()); if (empty($repetition_options)) { // default option is 'every time' $repetition_selector = idx( HeraldRepetitionPolicyConfig::getMap(), - HeraldRepetitionPolicyConfig::EVERY - ); + HeraldRepetitionPolicyConfig::EVERY); return $repetition_selector; } else if (count($repetition_options) == 1) { // if there's only 1 option, just pick it for the user $repetition_selector = reset($repetition_options); return $repetition_selector; } else { - // give the user all the options for this rule type - $tags = array(); - - foreach ($repetition_options as $name => $option) { - $tags[] = phutil_render_tag( - 'option', - array( - 'selected' => ($repetition_policy == $name) ? 'selected' : null, - 'value' => $name, - ), - phutil_escape_html($option) - ); - } - - $repetition_selector = - ''; - return $repetition_selector; + return AphrontFormSelectControl::renderSelectTag( + $repetition_policy, + $repetition_options, + array( + 'name' => 'repetition_policy', + )); } } + protected function buildTokenizerTemplates() { $template = new AphrontTokenizerTemplateView(); $template = $template->render(); @@ -580,4 +532,33 @@ final class HeraldRuleController extends HeraldController { 'markup' => $template, ); } + + + /** + * Load rules for the "Another Herald rule..." condition dropdown, which + * allows one rule to depend upon the success or failure of another rule. + */ + private function loadRulesThisRuleMayDependUpon(HeraldRule $rule) { + // Any rule can depend on a global rule. + $all_rules = id(new HeraldRuleQuery()) + ->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_GLOBAL)) + ->withContentTypes(array($rule->getContentType())) + ->execute(); + + if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { + // Personal rules may depend upon your other personal rules. + $all_rules += id(new HeraldRuleQuery()) + ->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_PERSONAL)) + ->withContentTypes(array($rule->getContentType())) + ->withAuthorPHIDs(array($rule->getAuthorPHID())) + ->execute(); + } + + // A rule can not depend upon itself. + unset($all_rules[$rule->getID()]); + + return $all_rules; + } + + } diff --git a/src/applications/herald/controller/rule/__init__.php b/src/applications/herald/controller/rule/__init__.php index 7167b40e78..f353801131 100644 --- a/src/applications/herald/controller/rule/__init__.php +++ b/src/applications/herald/controller/rule/__init__.php @@ -16,6 +16,7 @@ phutil_require_module('phabricator', 'applications/herald/config/repetitionpolic phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'applications/herald/config/valuetype'); phutil_require_module('phabricator', 'applications/herald/controller/base'); +phutil_require_module('phabricator', 'applications/herald/query/rule'); phutil_require_module('phabricator', 'applications/herald/storage/condition'); phutil_require_module('phabricator', 'applications/herald/storage/rule'); phutil_require_module('phabricator', 'applications/phid/handle/data'); @@ -25,13 +26,13 @@ phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/control/tokenizer'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/markup'); +phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/error'); phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); -phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/herald/controller/test/HeraldTestConsoleController.php b/src/applications/herald/controller/test/HeraldTestConsoleController.php index 339e0bb614..10f6c92494 100644 --- a/src/applications/herald/controller/test/HeraldTestConsoleController.php +++ b/src/applications/herald/controller/test/HeraldTestConsoleController.php @@ -18,10 +18,6 @@ final class HeraldTestConsoleController extends HeraldController { - public function getFilter() { - return 'test'; - } - public function processRequest() { $request = $this->getRequest(); @@ -136,11 +132,16 @@ final class HeraldTestConsoleController extends HeraldController { $panel->setWidth(AphrontPanelView::WIDTH_FULL); $panel->appendChild($form); - return $this->buildStandardPageResponse( + $nav = $this->renderNav(); + $nav->selectFilter('test'); + $nav->appendChild( array( $error_view, $panel, - ), + )); + + return $this->buildStandardPageResponse( + $nav, array( 'title' => 'Test Console', 'tab' => 'test', diff --git a/src/applications/herald/controller/transcript/HeraldTranscriptController.php b/src/applications/herald/controller/transcript/HeraldTranscriptController.php index 27b6ee0761..16875c5942 100644 --- a/src/applications/herald/controller/transcript/HeraldTranscriptController.php +++ b/src/applications/herald/controller/transcript/HeraldTranscriptController.php @@ -26,10 +26,6 @@ final class HeraldTranscriptController extends HeraldController { private $filter; private $handles; - public function getFilter() { - return 'transcript'; - } - public function willProcessRequest(array $data) { $this->id = $data['id']; $map = $this->getFilterMap(); @@ -69,6 +65,15 @@ final class HeraldTranscriptController extends HeraldController { ->loadHandles(); $this->handles = $handles; + if ($xscript->getDryRun()) { + $notice = new AphrontErrorView(); + $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE); + $notice->setTitle('Dry Run'); + $notice->appendChild( + 'This was a dry run to test Herald rules, no actions were executed.'); + $nav->appendChild($notice); + } + $apply_xscript_panel = $this->buildApplyTranscriptPanel( $xscript); $nav->appendChild($apply_xscript_panel); @@ -82,21 +87,12 @@ final class HeraldTranscriptController extends HeraldController { $nav->appendChild($object_xscript_panel); } -/* - - TODO - - $notice = null; - if ($xscript->getDryRun()) { - $notice = - - This was a dry run to test Herald rules, no actions were executed. - ; - } -*/ + $main_nav = $this->renderNav(); + $main_nav->selectFilter('transcript'); + $main_nav->appendChild($nav); return $this->buildStandardPageResponse( - $nav, + $main_nav, array( 'title' => 'Transcript', )); diff --git a/src/applications/herald/controller/transcriptlist/HeraldTranscriptListController.php b/src/applications/herald/controller/transcriptlist/HeraldTranscriptListController.php index 950ef83ee9..7b41b43432 100644 --- a/src/applications/herald/controller/transcriptlist/HeraldTranscriptListController.php +++ b/src/applications/herald/controller/transcriptlist/HeraldTranscriptListController.php @@ -18,10 +18,6 @@ final class HeraldTranscriptListController extends HeraldController { - public function getFilter() { - return 'transcript'; - } - public function processRequest() { $request = $this->getRequest(); @@ -41,12 +37,15 @@ final class HeraldTranscriptListController extends HeraldController { $phid); } - $offset = $request->getInt('offset', 0); - $page_size = 100; + $pager = new AphrontPagerView(); + $pager->setOffset($request->getInt('offset')); + $pager->setURI($request->getRequestURI(), 'offset'); + $limit_clause = qsprintf( $conn_r, 'LIMIT %d, %d', - $offset, $page_size + 1); + $pager->getOffset(), + $pager->getPageSize() + 1); $data = queryfx_all( $conn_r, @@ -58,39 +57,7 @@ final class HeraldTranscriptListController extends HeraldController { $where_clause, $limit_clause); - $pager = new AphrontPagerView(); - $pager->getPageSize($page_size); - $pager->setHasMorePages(count($data) == $page_size + 1); - $pager->setOffset($offset); - $pager->setURI($request->getRequestURI(), 'offset'); - - /* - - $conn_r = smc_get_db('cdb.herald', 'r'); - - $page_size = 100; - - $pager = new SimplePager(); - $pager->setPageSize($page_size); - $pager->setOffset((((int)$request->getInt('page')) - 1) * $page_size); - $pager->order('id', array('id')); - - - $fbid = $request->getInt('fbid'); - if ($fbid) { - $filter = qsprintf( - $conn_r, - 'WHERE objectID = %d', - $fbid); - } else { - $filter = ''; - } - - $data = $pager->select( - $conn_r, - 'id, objectID, time, duration, dryRun FROM transcript %Q', - $filter); -*/ + $data = $pager->sliceResults($data); // Render the table. $handles = array(); @@ -103,8 +70,8 @@ final class HeraldTranscriptListController extends HeraldController { $rows = array(); foreach ($data as $xscript) { $rows[] = array( - phabricator_date($xscript['time'],$user), - phabricator_time($xscript['time'],$user), + phabricator_date($xscript['time'], $user), + phabricator_time($xscript['time'], $user), $handles[$xscript['objectPHID']]->renderLink(), $xscript['dryRun'] ? 'Yes' : '', number_format((int)(1000 * $xscript['duration'])).' ms', @@ -144,8 +111,12 @@ final class HeraldTranscriptListController extends HeraldController { $panel->appendChild($table); $panel->appendChild($pager); + $nav = $this->renderNav(); + $nav->selectFilter('transcript'); + $nav->appendChild($panel); + return $this->buildStandardPageResponse( - $panel, + $nav, array( 'title' => 'Herald Transcripts', 'tab' => 'transcripts', diff --git a/src/applications/herald/query/log/HeraldEditLogQuery.php b/src/applications/herald/query/log/HeraldEditLogQuery.php new file mode 100644 index 0000000000..69d1b04862 --- /dev/null +++ b/src/applications/herald/query/log/HeraldEditLogQuery.php @@ -0,0 +1,64 @@ +ruleIDs = $rule_ids; + return $this; + } + + public function execute() { + $table = new HeraldRuleEdit(); + $conn_r = $table->establishConnection('r'); + + $where = $this->buildWhereClause($conn_r); + $order = $this->buildOrderClause($conn_r); + $limit = $this->buildLimitClause($conn_r); + + $data = queryfx_all( + $conn_r, + 'SELECT log.* FROM %T log %Q %Q %Q', + $table->getTableName(), + $where, + $order, + $limit); + + return $table->loadAllFromArray($data); + } + + private function buildWhereClause($conn_r) { + $where = array(); + + if ($this->ruleIDs) { + $where[] = qsprintf( + $conn_r, + 'ruleID IN (%Ld)', + $this->ruleIDs); + } + + return $this->formatWhereClause($where); + } + + private function buildOrderClause($conn_r) { + return 'ORDER BY id DESC'; + } + +} diff --git a/src/applications/herald/query/log/__init__.php b/src/applications/herald/query/log/__init__.php new file mode 100644 index 0000000000..fad2731584 --- /dev/null +++ b/src/applications/herald/query/log/__init__.php @@ -0,0 +1,15 @@ +authorPHIDs = $author_phids; + return $this; + } + + public function withRuleTypes(array $types) { + $this->ruleTypes = $types; + return $this; + } + + public function withContentTypes(array $types) { + $this->contentTypes = $types; + return $this; + } + + public function execute() { + $table = new HeraldRule(); + $conn_r = $table->establishConnection('r'); + + $where = $this->buildWhereClause($conn_r); + $order = $this->buildOrderClause($conn_r); + $limit = $this->buildLimitClause($conn_r); + + $data = queryfx_all( + $conn_r, + 'SELECT rule.* FROM %T rule %Q %Q %Q', + $table->getTableName(), + $where, + $order, + $limit); + + return $table->loadAllFromArray($data); + } + + private function buildWhereClause($conn_r) { + $where = array(); + + if ($this->authorPHIDs) { + $where[] = qsprintf( + $conn_r, + 'rule.authorPHID IN (%Ls)', + $this->authorPHIDs); + } + + if ($this->ruleTypes) { + $where[] = qsprintf( + $conn_r, + 'rule.ruleType IN (%Ls)', + $this->ruleTypes); + } + + if ($this->contentTypes) { + $where[] = qsprintf( + $conn_r, + 'rule.contentType IN (%Ls)', + $this->contentTypes); + } + + return $this->formatWhereClause($where); + } + + private function buildOrderClause($conn_r) { + return 'ORDER BY id DESC'; + } + +} diff --git a/src/applications/herald/query/rule/__init__.php b/src/applications/herald/query/rule/__init__.php new file mode 100644 index 0000000000..f8ee024d95 --- /dev/null +++ b/src/applications/herald/query/rule/__init__.php @@ -0,0 +1,15 @@ +edits = $edits; - return $this; - } - - public function getEdits() { - if ($this->edits === null) { - throw new Exception("Attach edits before accessing them!"); - } - return $this->edits; - } - - public function saveEdit($editor_phid) { - $edit = new HeraldRuleEdit(); - $edit->setRuleID($this->getID()); - $edit->setEditorPHID($editor_phid); - $edit->save(); + public function logEdit($editor_phid, $action) { + id(new HeraldRuleEdit()) + ->setRuleID($this->getID()) + ->setRuleName($this->getName()) + ->setEditorPHID($editor_phid) + ->setAction($action) + ->save(); } public function saveConditions(array $conditions) { diff --git a/src/applications/herald/view/edithistory/HeraldRuleEditHistoryView.php b/src/applications/herald/view/edithistory/HeraldRuleEditHistoryView.php index a8cd516897..0305f10ac6 100644 --- a/src/applications/herald/view/edithistory/HeraldRuleEditHistoryView.php +++ b/src/applications/herald/view/edithistory/HeraldRuleEditHistoryView.php @@ -17,14 +17,19 @@ */ final class HeraldRuleEditHistoryView extends AphrontView { - private $rule; + + private $edits; private $handles; - public function setRule($rule) { - $this->rule = $rule; + public function setEdits(array $edits) { + $this->edits = $edits; return $this; } + public function getEdits() { + return $this->edits; + } + public function setHandles(array $handles) { $this->handles = $handles; return $this; @@ -38,36 +43,51 @@ final class HeraldRuleEditHistoryView extends AphrontView { public function render() { $rows = array(); - foreach ($this->rule->getEdits() as $edit) { - $editor = $this->handles[$edit->getEditorPHID()]->renderLink(); + foreach ($this->edits as $edit) { + $name = nonempty($edit->getRuleName(), 'Unknown Rule'); + $rule_name = phutil_render_tag( + 'strong', + array(), + phutil_escape_html($name)); - $edit_date = phabricator_datetime($edit->getDateCreated(), $this->user); - - // Something useful could totally go here - $details = ""; + switch ($edit->getAction()) { + case 'create': + $details = "Created rule '{$rule_name}'."; + break; + case 'delete': + $details = "Deleted rule '{$rule_name}'."; + break; + case 'edit': + default: + $details = "Edited rule '{$rule_name}'."; + break; + } $rows[] = array( - $editor, - $edit_date, + $edit->getRuleID(), + $this->handles[$edit->getEditorPHID()]->renderLink(), $details, + phabricator_datetime($edit->getDateCreated(), $this->user), ); } - $rule_name = phutil_escape_html($this->rule->getName()); $table = new AphrontTableView($rows); - $table->setNoDataString( - "No edits for \"${rule_name}\""); + $table->setNoDataString("No edits for rule."); $table->setHeaders( array( + 'Rule ID', 'Editor', - 'Edit Date', 'Details', + 'Edit Date', + )); + $table->setColumnClasses( + array( + '', + '', + 'wide', + '', )); - $panel = new AphrontPanelView(); - $panel->setHeader("Edit History for \"${rule_name}\""); - $panel->appendChild($table); - - return $panel; + return $table->render(); } } diff --git a/src/applications/herald/view/edithistory/__init__.php b/src/applications/herald/view/edithistory/__init__.php index da21a5b70b..7883a2aaae 100644 --- a/src/applications/herald/view/edithistory/__init__.php +++ b/src/applications/herald/view/edithistory/__init__.php @@ -8,10 +8,10 @@ phutil_require_module('phabricator', 'view/base'); phutil_require_module('phabricator', 'view/control/table'); -phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phabricator', 'view/utils'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'utils'); phutil_require_source('HeraldRuleEditHistoryView.php'); diff --git a/src/applications/herald/view/rulelist/HeraldRuleListView.php b/src/applications/herald/view/rulelist/HeraldRuleListView.php index ff6c069d87..d1726742c2 100644 --- a/src/applications/herald/view/rulelist/HeraldRuleListView.php +++ b/src/applications/herald/view/rulelist/HeraldRuleListView.php @@ -17,13 +17,12 @@ */ final class HeraldRuleListView extends AphrontView { + private $rules; private $handles; - private $map; - private $view; - private $allowCreation; - private $showOwner = true; - private $showType = false; + + private $showAuthor; + private $showRuleType; private $user; public function setRules(array $rules) { @@ -36,28 +35,13 @@ final class HeraldRuleListView extends AphrontView { return $this; } - public function setMap($map) { - $this->map = $map; + public function setShowAuthor($show_author) { + $this->showAuthor = $show_author; return $this; } - public function setView($view) { - $this->view = $view; - return $this; - } - - public function setAllowCreation($allow_creation) { - $this->allowCreation = $allow_creation; - return $this; - } - - public function setShowOwner($show_owner) { - $this->showOwner = $show_owner; - return $this; - } - - public function setShowType($show_type) { - $this->showType = $show_type; + public function setShowRuleType($show_rule_type) { + $this->showRuleType = $show_rule_type; return $this; } @@ -73,7 +57,12 @@ final class HeraldRuleListView extends AphrontView { $rows = array(); foreach ($this->rules as $rule) { - $owner = $this->handles[$rule->getAuthorPHID()]->renderLink(); + + if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) { + $author = null; + } else { + $author = $this->handles[$rule->getAuthorPHID()]->renderLink(); + } $name = phutil_render_tag( 'a', @@ -82,21 +71,12 @@ final class HeraldRuleListView extends AphrontView { ), phutil_escape_html($rule->getName())); - $last_edit_date = phabricator_datetime($rule->getDateModified(), - $this->user); - - $view_edits = phutil_render_tag( + $edit_log = phutil_render_tag( 'a', array( - 'href' => '/herald/history/' . $rule->getID() . '/', + 'href' => '/herald/history/'.$rule->getID().'/', ), - '(View Edits)'); - - $last_edited = phutil_render_tag( - 'span', - array(), - "Last edited on $last_edit_date ${view_edits}"); - + 'View Edit Log'); $delete = javelin_render_tag( 'a', @@ -108,60 +88,42 @@ final class HeraldRuleListView extends AphrontView { 'Delete'); $rows[] = array( - $this->map[$rule->getContentType()], $type_map[$rule->getRuleType()], - $owner, + $author, $name, - $last_edited, + $edit_log, $delete, ); } - $rules_for = phutil_escape_html($this->map[$this->view]); - $table = new AphrontTableView($rows); - $table->setNoDataString( - "No matching subscription rules for {$rules_for}."); + $table->setNoDataString("No matching rules."); $table->setHeaders( array( - 'Content Type', 'Rule Type', - 'Owner', + 'Author', 'Rule Name', - 'Last Edited', + 'Edit Log', '', )); $table->setColumnClasses( array( '', '', - '', - 'wide wrap pri', + 'wide pri', '', 'action' )); $table->setColumnVisibility( array( + $this->showRuleType, + $this->showAuthor, true, - $this->showType, - $this->showOwner, true, true, )); - $panel = new AphrontPanelView(); - $panel->setHeader("Herald Rules for {$rules_for}"); - - if ($this->allowCreation) { - $panel->setCreateButton( - 'Create New Herald Rule', - '/herald/new/'.$this->view.'/'); - } - - $panel->appendChild($table); - - return $panel; - + return $table->render(); } } diff --git a/src/applications/herald/view/rulelist/__init__.php b/src/applications/herald/view/rulelist/__init__.php index 3aa96248cd..69a972726d 100644 --- a/src/applications/herald/view/rulelist/__init__.php +++ b/src/applications/herald/view/rulelist/__init__.php @@ -10,8 +10,6 @@ phutil_require_module('phabricator', 'applications/herald/config/ruletype'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phabricator', 'view/control/table'); -phutil_require_module('phabricator', 'view/layout/panel'); -phutil_require_module('phabricator', 'view/utils'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index b357444ecc..8db0cec1c6 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -24,6 +24,11 @@ final class PhabricatorObjectHandleData { $this->phids = array_unique($phids); } + public static function loadOneHandle($phid) { + $handles = id(new PhabricatorObjectHandleData(array($phid)))->loadHandles(); + return $handles[$phid]; + } + public function loadObjects() { $types = array(); foreach ($this->phids as $phid) { diff --git a/src/view/control/table/AphrontTableView.php b/src/view/control/table/AphrontTableView.php index c958d95fc7..dbda7bd6d3 100644 --- a/src/view/control/table/AphrontTableView.php +++ b/src/view/control/table/AphrontTableView.php @@ -237,7 +237,7 @@ final class AphrontTableView extends AphrontView { ++$row_num; } } else { - $colspan = max(count($headers), 1); + $colspan = max(count(array_filter($visibility)), 1); $table[] = ''. coalesce($this->noDataString, 'No data available.'). diff --git a/src/view/layout/sidenavfilter/AphrontSideNavFilterView.php b/src/view/layout/sidenavfilter/AphrontSideNavFilterView.php index 5b28c76fa9..aa36c9a450 100644 --- a/src/view/layout/sidenavfilter/AphrontSideNavFilterView.php +++ b/src/view/layout/sidenavfilter/AphrontSideNavFilterView.php @@ -78,7 +78,7 @@ final class AphrontSideNavFilterView extends AphrontView { return $this->baseURI; } - public function selectFilter($key, $default) { + public function selectFilter($key, $default = null) { $this->selectedFilter = $default; if ($key !== null) { foreach ($this->items as $item) { diff --git a/webroot/rsrc/css/application/herald/herald.css b/webroot/rsrc/css/application/herald/herald.css index d66ece777d..f8867e9934 100644 --- a/webroot/rsrc/css/application/herald/herald.css +++ b/webroot/rsrc/css/application/herald/herald.css @@ -29,7 +29,3 @@ .herald-action-table td.target { width: 100%; } - -#herald-rule-edit-form #author-input { - width: 300px; -} diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index c8715b2ced..2f4098a845 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -65,7 +65,6 @@ JX.install('HeraldRuleEditor', { this._renderConditions(conditions); this._renderActions(actions); - this._renderAuthorInput(); }, members : { @@ -120,12 +119,6 @@ JX.install('HeraldRuleEditor', { conditions: this._config.conditions, actions: this._config.actions }); - - var authorInput = JX.DOM.find(this._root, 'input', 'author'); - var authorID = JX.keys(this._config.authorGetter())[0]; - if (authorID) { - authorInput.value = authorID; - } }, _getConditionValue : function(id) { @@ -243,22 +236,6 @@ JX.install('HeraldRuleEditor', { return [input, get_fn, set_fn]; }, - _renderAuthorInput : function() { - var tokenizer = this._newTokenizer('email', 1); - input = tokenizer[0]; - set_fn = tokenizer[2]; - set_fn(this._config.author); - this._config.authorGetter = tokenizer[1]; - try { - var author_cell = JX.$('author-input'); - JX.DOM.setContent(author_cell, input); - } catch (ex) { - // TODO: Get rid of the ownership changing stuff or something? - // On global rules, there's no "author" input. Not fixing this - // properly because I think I'm going to nuke it soon. - } - }, - _renderValueInputForRow : function(row_id) { var cond = this._config.conditions[row_id]; var type = this._config.info.values[cond[0]][cond[1]];