From 8a4c08b01db0680e2d98cc45a640ebb6cb9dd6e1 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 8 Aug 2012 10:03:41 -0700 Subject: [PATCH] Allow commits to be associated with projects and associated goodies Summary: - Commit detail view - List of projects - "edit" action which takes the user to a simple form where they can only add / remove projects. - Integrated the project relationship into the commit search indexer - fixed a bug from D790; it seems you must select the column if you're going to join against it later. Without this change searching for author or projectfails 100% for me. Test Plan: added and removed projects. verified appropriate projects showed up in detail and edit view. searched for commits by project and found the ones I was supposed to...! Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T1614 Differential Revision: https://secure.phabricator.com/D3189 --- src/__celerity_resource_map__.php | 28 ++--- src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationDiffusion.php | 2 + .../controller/DiffusionCommitController.php | 58 +++++++-- .../DiffusionCommitEditController.php | 112 ++++++++++++++++++ .../ManiphestTaskEditController.php | 2 +- .../engine/PhabricatorSearchEngineMySQL.php | 19 +-- .../PhabricatorSearchCommitIndexer.php | 14 +++ .../edges/constants/PhabricatorEdgeConfig.php | 7 ++ .../behavior-project-create.js | 4 +- 10 files changed, 203 insertions(+), 45 deletions(-) create mode 100644 src/applications/diffusion/controller/DiffusionCommitEditController.php rename webroot/rsrc/js/application/{maniphest => projects}/behavior-project-create.js (81%) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 255789312b..0a3c859a27 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1376,19 +1376,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/maniphest/behavior-task-preview.js', ), - 'javelin-behavior-maniphest-project-create' => - array( - 'uri' => '/res/85a0eaf9/rsrc/js/application/maniphest/behavior-project-create.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-stratcom', - 3 => 'javelin-workflow', - ), - 'disk' => '/rsrc/js/application/maniphest/behavior-project-create.js', - ), 'javelin-behavior-maniphest-subpriority-editor' => array( 'uri' => '/res/5e02f19a/rsrc/js/application/maniphest/behavior-subpriorityeditor.js', @@ -1621,6 +1608,19 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/phriction/phriction-document-preview.js', ), + 'javelin-behavior-project-create' => + array( + 'uri' => '/res/e91f3f8f/rsrc/js/application/projects/behavior-project-create.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-stratcom', + 3 => 'javelin-workflow', + ), + 'disk' => '/rsrc/js/application/projects/behavior-project-create.js', + ), 'javelin-behavior-refresh-csrf' => array( 'uri' => '/res/88beba4c/rsrc/js/application/core/behavior-refresh-csrf.js', @@ -2423,7 +2423,7 @@ celerity_register_resource_map(array( ), 'phabricator-object-list-view-css' => array( - 'uri' => '/res/4e060838/rsrc/css/application/projects/phabricator-object-list-view.css', + 'uri' => '/res/4f183668/rsrc/css/application/projects/phabricator-object-list-view.css', 'type' => 'css', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0f89c0148f..0faef48226 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -318,6 +318,7 @@ phutil_register_library_map(array( 'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php', 'DiffusionCommitChangeTableView' => 'applications/diffusion/view/DiffusionCommitChangeTableView.php', 'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php', + 'DiffusionCommitEditController' => 'applications/diffusion/controller/DiffusionCommitEditController.php', 'DiffusionCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionCommitParentsQuery.php', 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', 'DiffusionCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionCommitTagsQuery.php', @@ -1429,6 +1430,7 @@ phutil_register_library_map(array( 'DiffusionCommitBranchesController' => 'DiffusionController', 'DiffusionCommitChangeTableView' => 'DiffusionView', 'DiffusionCommitController' => 'DiffusionController', + 'DiffusionCommitEditController' => 'DiffusionController', 'DiffusionCommitParentsQuery' => 'DiffusionQuery', 'DiffusionCommitTagsController' => 'DiffusionController', 'DiffusionCommitTagsQuery' => 'DiffusionQuery', diff --git a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php index 8182a4d0ab..1db129df93 100644 --- a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php +++ b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php @@ -52,6 +52,8 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication { => 'DiffusionCommitBranchesController', 'commit/(?P[a-z0-9]+)/tags/' => 'DiffusionCommitTagsController', + 'commit/(?P[a-z0-9]+)/edit/' + => 'DiffusionCommitEditController', ), 'inline/' => array( 'edit/(?P[^/]+)/' => 'DiffusionInlineCommentController', diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index fc40f92061..3086fb362b 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -50,8 +50,15 @@ final class DiffusionCommitController extends DiffusionController { $commit = $drequest->loadCommit(); if (!$commit) { - // TODO: Make more user-friendly. - throw new Exception('This commit has not parsed yet.'); + // TODO -- T1624 -- detect if this has actually not been parsed yet + // and show this UI if so, else 404 + return $this->buildStandardPageResponse( + id(new AphrontErrorView()) + ->setTitle('Error displaying commit.') + ->appendChild('Failed to load the commit. The commit has not been '. + 'parsed yet.'), + array('title' => 'Commit Still Parsing') + ); } $commit_data = $drequest->loadCommitData(); @@ -83,7 +90,7 @@ final class DiffusionCommitController extends DiffusionController { $headsup_panel = new AphrontHeadsupView(); $headsup_panel->setHeader('Commit Detail'); $headsup_panel->setActionList( - $this->renderHeadsupActionList($commit)); + $this->renderHeadsupActionList($commit, $repository)); $headsup_panel->setProperties( $this->getCommitProperties( $commit, @@ -310,14 +317,27 @@ final class DiffusionCommitController extends DiffusionController { PhabricatorRepositoryCommit $commit, PhabricatorRepositoryCommitData $data, array $parents) { + assert_instances_of($parents, 'PhabricatorRepositoryCommit'); $user = $this->getRequest()->getUser(); + $commit_phid = $commit->getPHID(); - $task_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $commit->getPHID(), - PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK); + $edges = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(array($commit_phid)) + ->withEdgeTypes(array( + PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK, + PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT + )) + ->execute(); - $phids = $task_phids; + $task_phids = array_keys( + $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_TASK] + ); + $proj_phids = array_keys( + $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT] + ); + + $phids = array_merge($task_phids, $proj_phids); if ($data->getCommitDetail('authorPHID')) { $phids[] = $data->getCommitDetail('authorPHID'); } @@ -380,7 +400,6 @@ final class DiffusionCommitController extends DiffusionController { } } - $revision_phid = $data->getCommitDetail('differential.revisionPHID'); if ($revision_phid) { $props['Differential Revision'] = $handles[$revision_phid]->renderLink(); @@ -394,7 +413,6 @@ final class DiffusionCommitController extends DiffusionController { $props['Parents'] = implode(' · ', $parent_links); } - $request = $this->getDiffusionRequest(); $props['Branches'] = 'Unknown'; @@ -423,6 +441,15 @@ final class DiffusionCommitController extends DiffusionController { $props['Tasks'] = $task_list; } + if ($proj_phids) { + $proj_list = array(); + foreach ($proj_phids as $phid) { + $proj_list[] = $handles[$phid]->renderLink(); + } + $proj_list = implode('
', $proj_list); + $props['Projects'] = $proj_list; + } + return $props; } @@ -739,13 +766,24 @@ final class DiffusionCommitController extends DiffusionController { } private function renderHeadsupActionList( - PhabricatorRepositoryCommit $commit) { + PhabricatorRepositoryCommit $commit, + PhabricatorRepository $repository) { $request = $this->getRequest(); $user = $request->getUser(); $actions = array(); + // TODO -- integrate permissions into whether or not this action is shown + $uri = '/diffusion/'.$repository->getCallSign().'/commit/'. + $commit->getCommitIdentifier().'/edit/'; + $action = new AphrontHeadsupActionView(); + $action->setClass('action-edit'); + $action->setURI($uri); + $action->setName('Edit Commit'); + $action->setWorkflow(false); + $actions[] = $action; + require_celerity_resource('phabricator-flag-css'); $flag = PhabricatorFlagQuery::loadUserFlag($user, $commit->getPHID()); if ($flag) { diff --git a/src/applications/diffusion/controller/DiffusionCommitEditController.php b/src/applications/diffusion/controller/DiffusionCommitEditController.php new file mode 100644 index 0000000000..fd80837ed6 --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionCommitEditController.php @@ -0,0 +1,112 @@ +diffusionRequest = DiffusionRequest::newFromDictionary($data); + } + + public function processRequest() { + + $request = $this->getRequest(); + $user = $request->getUser(); + $drequest = $this->getDiffusionRequest(); + $callsign = $drequest->getRepository()->getCallsign(); + $repository = $drequest->getRepository(); + $commit = $drequest->loadCommit(); + $page_title = 'Edit Diffusion Commit'; + + if (!$commit) { + return new Aphront404Response(); + } + + $commit_phid = $commit->getPHID(); + $edge_type = PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT; + $current_proj_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $commit_phid, + $edge_type + ); + $handles = id(new PhabricatorObjectHandleData($current_proj_phids)) + ->loadHandles(); + $proj_t_values = mpull($handles, 'getFullName', 'getPHID'); + + if ($request->isFormPost()) { + $proj_phids = $request->getArr('projects'); + $new_proj_phids = array_values($proj_phids); + $rem_proj_phids = array_diff($current_proj_phids, + $new_proj_phids); + $editor = id(new PhabricatorEdgeEditor()); + $editor->setUser($user); + foreach ($rem_proj_phids as $phid) { + $editor->removeEdge($commit_phid, $edge_type, $phid); + } + foreach ($new_proj_phids as $phid) { + $editor->addEdge($commit_phid, $edge_type, $phid); + } + $editor->save(); + + PhabricatorSearchCommitIndexer::indexCommit($commit); + + return id(new AphrontRedirectResponse()) + ->setURI('/r'.$callsign.$commit->getCommitIdentifier()); + } + + $tokenizer_id = celerity_generate_unique_node_id(); + $form = id(new AphrontFormView()) + ->setUser($user) + ->setAction($request->getRequestURI()->getPath()) + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setLabel('Projects') + ->setName('projects') + ->setValue($proj_t_values) + ->setID($tokenizer_id) + ->setCaption( + javelin_render_tag( + 'a', + array( + 'href' => '/project/create/', + 'mustcapture' => true, + 'sigil' => 'project-create', + ), + 'Create New Project')) + ->setDatasource('/typeahead/common/projects/'));; + + Javelin::initBehavior('project-create', array( + 'tokenizerID' => $tokenizer_id, + )); + + $submit = id(new AphrontFormSubmitControl()) + ->setValue('Save') + ->addCancelButton('/r'.$callsign.$commit->getCommitIdentifier()); + $form->appendChild($submit); + + $panel = id(new AphrontPanelView()) + ->setHeader('Edit Diffusion Commit') + ->appendChild($form) + ->setWidth(AphrontPanelView::WIDTH_FORM); + + return $this->buildStandardPageResponse( + $panel, + array( + 'title' => $page_title, + )); + } + +} diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index 8901372a9a..c304785a57 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -466,7 +466,7 @@ final class ManiphestTaskEditController extends ManiphestController { require_celerity_resource('aphront-error-view-css'); - Javelin::initBehavior('maniphest-project-create', array( + Javelin::initBehavior('project-create', array( 'tokenizerID' => $project_tokenizer_id, )); diff --git a/src/applications/search/engine/PhabricatorSearchEngineMySQL.php b/src/applications/search/engine/PhabricatorSearchEngineMySQL.php index 7b3d1c9e6f..9a5ff7c425 100644 --- a/src/applications/search/engine/PhabricatorSearchEngineMySQL.php +++ b/src/applications/search/engine/PhabricatorSearchEngineMySQL.php @@ -252,23 +252,6 @@ final class PhabricatorSearchEngineMySQL extends PhabricatorSearchEngine { 'repository', PhabricatorSearchRelationship::RELATIONSHIP_REPOSITORY); -/* - $join[] = $this->joinRelationship( - $conn_r, - $query, - 'reviewer', - AdjutantRelationship::RELATIONSHIP_REVIEWER); - $join[] = $this->joinRelationship( - $conn_r, - $query, - 'subscriber', - AdjutantRelationship::RELATIONSHIP_SUBSCRIBER); - $join[] = $this->joinRelationship( - $conn_r, - $query, - 'repository', - AdjutantRelationship::RELATIONSHIP_REPOSITORY); -*/ $join = array_filter($join); foreach ($join as $key => $clause) { @@ -288,7 +271,7 @@ final class PhabricatorSearchEngineMySQL extends PhabricatorSearchEngine { $hits = queryfx_all( $conn_r, 'SELECT - document.phid + document.phid FROM %T document %Q %Q diff --git a/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php b/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php index 72e3ac52fe..1c09563499 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php +++ b/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php @@ -60,6 +60,20 @@ final class PhabricatorSearchCommitIndexer $date_created); } + $project_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $commit->getPHID(), + PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT + ); + if ($project_phids) { + foreach ($project_phids as $project_phid) { + $doc->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_PROJECT, + $project_phid, + PhabricatorPHIDConstants::PHID_TYPE_PROJ, + $date_created); + } + } + $doc->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_REPOSITORY, $repository->getPHID(), diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php index 33497149b4..6f678a115c 100644 --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -41,6 +41,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { const TYPE_PROJ_MEMBER = 13; const TYPE_MEMBER_OF_PROJ = 14; + const TYPE_COMMIT_HAS_PROJECT = 15; + const TYPE_PROJECT_HAS_COMMIT = 16; + const TYPE_TEST_NO_CYCLE = 9000; public static function getInverse($edge_type) { @@ -64,6 +67,10 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { self::TYPE_PROJ_MEMBER => self::TYPE_MEMBER_OF_PROJ, self::TYPE_MEMBER_OF_PROJ => self::TYPE_PROJ_MEMBER, + + self::TYPE_COMMIT_HAS_PROJECT => self::TYPE_PROJECT_HAS_COMMIT, + self::TYPE_PROJECT_HAS_COMMIT => self::TYPE_COMMIT_HAS_PROJECT, + ); return idx($map, $edge_type); diff --git a/webroot/rsrc/js/application/maniphest/behavior-project-create.js b/webroot/rsrc/js/application/projects/behavior-project-create.js similarity index 81% rename from webroot/rsrc/js/application/maniphest/behavior-project-create.js rename to webroot/rsrc/js/application/projects/behavior-project-create.js index 11c9499a44..e3c0cd1bfa 100644 --- a/webroot/rsrc/js/application/maniphest/behavior-project-create.js +++ b/webroot/rsrc/js/application/projects/behavior-project-create.js @@ -1,12 +1,12 @@ /** - * @provides javelin-behavior-maniphest-project-create + * @provides javelin-behavior-project-create * @requires javelin-behavior * javelin-dom * javelin-stratcom * javelin-workflow */ -JX.behavior('maniphest-project-create', function(config) { +JX.behavior('project-create', function(config) { JX.Stratcom.listen( 'click',