diff --git a/src/applications/differential/constants/DifferentialRevisionStatus.php b/src/applications/differential/constants/DifferentialRevisionStatus.php index a35e00e9cc..64a330dc7e 100644 --- a/src/applications/differential/constants/DifferentialRevisionStatus.php +++ b/src/applications/differential/constants/DifferentialRevisionStatus.php @@ -1,105 +1,115 @@ self::COLOR_STATUS_DEFAULT, ArcanistDifferentialRevisionStatus::NEEDS_REVISION => self::COLOR_STATUS_RED, + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => + self::COLOR_STATUS_RED, ArcanistDifferentialRevisionStatus::ACCEPTED => self::COLOR_STATUS_GREEN, ArcanistDifferentialRevisionStatus::CLOSED => self::COLOR_STATUS_DARK, ArcanistDifferentialRevisionStatus::ABANDONED => self::COLOR_STATUS_DARK, + ArcanistDifferentialRevisionStatus::IN_PREPARATION => + self::COLOR_STATUS_DARK, ); return idx($map, $status, $default); } public static function getRevisionStatusIcon($status) { $default = 'oh-open'; $map = array( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW => 'oh-open', ArcanistDifferentialRevisionStatus::NEEDS_REVISION => 'oh-open-red', + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => + 'oh-open-red', ArcanistDifferentialRevisionStatus::ACCEPTED => 'oh-open-green', ArcanistDifferentialRevisionStatus::CLOSED => 'oh-closed-dark', ArcanistDifferentialRevisionStatus::ABANDONED => 'oh-closed-dark', + ArcanistDifferentialRevisionStatus::IN_PREPARATION => + 'question-blue', ); return idx($map, $status, $default); } public static function renderFullDescription($status) { $color = self::getRevisionStatusColor($status); $status_name = ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status); $img = id(new PHUIIconView()) ->setSpriteSheet(PHUIIconView::SPRITE_STATUS) ->setSpriteIcon(self::getRevisionStatusIcon($status)); $tag = phutil_tag( 'span', array( 'class' => 'phui-header-'.$color.' plr', ), array( $img, $status_name, )); return $tag; } public static function getClosedStatuses() { $statuses = array( ArcanistDifferentialRevisionStatus::CLOSED, ArcanistDifferentialRevisionStatus::ABANDONED, ); if (PhabricatorEnv::getEnvConfig('differential.close-on-accept')) { $statuses[] = ArcanistDifferentialRevisionStatus::ACCEPTED; } return $statuses; } public static function getOpenStatuses() { return array_diff(self::getAllStatuses(), self::getClosedStatuses()); } public static function getAllStatuses() { return array( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, ArcanistDifferentialRevisionStatus::NEEDS_REVISION, + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, ArcanistDifferentialRevisionStatus::ACCEPTED, ArcanistDifferentialRevisionStatus::CLOSED, ArcanistDifferentialRevisionStatus::ABANDONED, + ArcanistDifferentialRevisionStatus::IN_PREPARATION, ); } public static function isClosedStatus($status) { return in_array($status, self::getClosedStatuses()); } } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index dc51fba4b5..9b919d1244 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1,917 +1,919 @@ revisionID = $data['id']; } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); $viewer_is_anonymous = !$user->isLoggedIn(); $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($this->revisionID)) ->setViewer($request->getUser()) ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); if (!$revision) { return new Aphront404Response(); } $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withRevisionIDs(array($this->revisionID)) ->execute(); $diffs = array_reverse($diffs, $preserve_keys = true); if (!$diffs) { throw new Exception( "This revision has no diffs. Something has gone quite wrong."); } $revision->attachActiveDiff(last($diffs)); $diff_vs = $request->getInt('vs'); $target_id = $request->getInt('id'); $target = idx($diffs, $target_id, end($diffs)); $target_manual = $target; if (!$target_id) { foreach ($diffs as $diff) { if ($diff->getCreationMethod() != 'commit') { $target_manual = $diff; } } } if (empty($diffs[$diff_vs])) { $diff_vs = null; } $arc_project = $target->loadArcanistProject(); $repository = ($arc_project ? $arc_project->loadRepository() : null); list($changesets, $vs_map, $vs_changesets, $rendering_references) = $this->loadChangesetsAndVsMap( $target, idx($diffs, $diff_vs), $repository); if ($request->getExists('download')) { return $this->buildRawDiffResponse( $revision, $changesets, $vs_changesets, $vs_map, $repository); } $props = id(new DifferentialDiffProperty())->loadAllWhere( 'diffID = %d', $target_manual->getID()); $props = mpull($props, 'getData', 'getName'); $comments = $revision->loadComments(); $all_changesets = $changesets; $inlines = $this->loadInlineComments( $revision, $all_changesets); $object_phids = array_merge( $revision->getReviewers(), $revision->getCCPHIDs(), $revision->loadCommitPHIDs(), array( $revision->getAuthorPHID(), $user->getPHID(), ), mpull($comments, 'getAuthorPHID')); foreach ($comments as $comment) { foreach ($comment->getRequiredHandlePHIDs() as $phid) { $object_phids[] = $phid; } } foreach ($revision->getAttached() as $type => $phids) { foreach ($phids as $phid => $info) { $object_phids[] = $phid; } } $handles = $this->loadViewerHandles($object_phids); $request_uri = $request->getRequestURI(); $limit = 100; $large = $request->getStr('large'); if (count($changesets) > $limit && !$large) { $count = count($changesets); $warning = new AphrontErrorView(); $warning->setTitle('Very Large Diff'); $warning->setSeverity(AphrontErrorView::SEVERITY_WARNING); $warning->appendChild(hsprintf( '%s %s', pht( 'This diff is very large and affects %s files. Load each file '. 'individually.', new PhutilNumber($count)), phutil_tag( 'a', array( 'href' => $request_uri ->alter('large', 'true') ->setFragment('toc'), ), pht('Show All Files Inline')))); $warning = $warning->render(); $my_inlines = id(new DifferentialInlineCommentQuery()) ->withDraftComments($user->getPHID(), $this->revisionID) ->execute(); $visible_changesets = array(); foreach ($inlines + $my_inlines as $inline) { $changeset_id = $inline->getChangesetID(); if (isset($changesets[$changeset_id])) { $visible_changesets[$changeset_id] = $changesets[$changeset_id]; } } if (!empty($props['arc:lint'])) { $changeset_paths = mpull($changesets, null, 'getFilename'); foreach ($props['arc:lint'] as $lint) { $changeset = idx($changeset_paths, $lint['path']); if ($changeset) { $visible_changesets[$changeset->getID()] = $changeset; } } } } else { $warning = null; $visible_changesets = $changesets; } $field_list = PhabricatorCustomField::getObjectFields( $revision, PhabricatorCustomField::ROLE_VIEW); $field_list->setViewer($user); $field_list->readFieldsFromStorage($revision); // TODO: This should be in a DiffQuery or similar. $need_props = array(); foreach ($field_list->getFields() as $field) { foreach ($field->getRequiredDiffPropertiesForRevisionView() as $prop) { $need_props[$prop] = $prop; } } if ($need_props) { $prop_diff = $revision->getActiveDiff(); $load_props = id(new DifferentialDiffProperty())->loadAllWhere( 'diffID = %d AND name IN (%Ls)', $prop_diff->getID(), $need_props); $load_props = mpull($load_props, 'getData', 'getName'); foreach ($need_props as $need) { $prop_diff->attachProperty($need, idx($load_props, $need)); } } $revision_detail = id(new DifferentialRevisionDetailView()) ->setUser($user) ->setRevision($revision) ->setDiff(end($diffs)) ->setCustomFields($field_list) ->setURI($request->getRequestURI()); $actions = $this->getRevisionActions($revision); $whitespace = $request->getStr( 'whitespace', DifferentialChangesetParser::WHITESPACE_IGNORE_ALL); if ($arc_project) { list($symbol_indexes, $project_phids) = $this->buildSymbolIndexes( $arc_project, $visible_changesets); } else { $symbol_indexes = array(); $project_phids = null; } $revision_detail->setActions($actions); $revision_detail->setUser($user); $revision_detail_box = $revision_detail->render(); $revision_warnings = $this->buildRevisionWarnings($revision, $handles); if ($revision_warnings) { $revision_detail_box->setErrorView($revision_warnings); } $comment_view = $this->buildTransactions( $revision, $diff_vs ? $diffs[$diff_vs] : $target, $target, $all_changesets); $wrap_id = celerity_generate_unique_node_id(); $comment_view = phutil_tag( 'div', array( 'id' => $wrap_id, ), $comment_view); if ($arc_project) { Javelin::initBehavior( 'repository-crossreference', array( 'section' => $wrap_id, 'projects' => $project_phids, )); } $changeset_view = new DifferentialChangesetListView(); $changeset_view->setChangesets($changesets); $changeset_view->setVisibleChangesets($visible_changesets); if (!$viewer_is_anonymous) { $changeset_view->setInlineCommentControllerURI( '/differential/comment/inline/edit/'.$revision->getID().'/'); } $changeset_view->setStandaloneURI('/differential/changeset/'); $changeset_view->setRawFileURIs( '/differential/changeset/?view=old', '/differential/changeset/?view=new'); $changeset_view->setUser($user); $changeset_view->setDiff($target); $changeset_view->setRenderingReferences($rendering_references); $changeset_view->setVsMap($vs_map); $changeset_view->setWhitespace($whitespace); if ($repository) { $changeset_view->setRepository($repository); } $changeset_view->setSymbolIndexes($symbol_indexes); $changeset_view->setTitle('Diff '.$target->getID()); $diff_history = new DifferentialRevisionUpdateHistoryView(); $diff_history->setDiffs($diffs); $diff_history->setSelectedVersusDiffID($diff_vs); $diff_history->setSelectedDiffID($target->getID()); $diff_history->setSelectedWhitespace($whitespace); $diff_history->setUser($user); $local_view = new DifferentialLocalCommitsView(); $local_view->setUser($user); $local_view->setLocalCommits(idx($props, 'local:commits')); if ($repository) { $other_revisions = $this->loadOtherRevisions( $changesets, $target, $repository); } else { $other_revisions = array(); } $other_view = null; if ($other_revisions) { $other_view = $this->renderOtherRevisions($other_revisions); } $toc_view = new DifferentialDiffTableOfContentsView(); $toc_view->setChangesets($changesets); $toc_view->setVisibleChangesets($visible_changesets); $toc_view->setRenderingReferences($rendering_references); $toc_view->setUnitTestData(idx($props, 'arc:unit', array())); if ($repository) { $toc_view->setRepository($repository); } $toc_view->setDiff($target); $toc_view->setUser($user); $toc_view->setRevisionID($revision->getID()); $toc_view->setWhitespace($whitespace); $comment_form = null; if (!$viewer_is_anonymous) { $draft = id(new PhabricatorDraft())->loadOneWhere( 'authorPHID = %s AND draftKey = %s', $user->getPHID(), 'differential-comment-'.$revision->getID()); $reviewers = array(); $ccs = array(); if ($draft) { $reviewers = idx($draft->getMetadata(), 'reviewers', array()); $ccs = idx($draft->getMetadata(), 'ccs', array()); if ($reviewers || $ccs) { $handles = $this->loadViewerHandles(array_merge($reviewers, $ccs)); $reviewers = array_select_keys($handles, $reviewers); $ccs = array_select_keys($handles, $ccs); } } $comment_form = new DifferentialAddCommentView(); $comment_form->setRevision($revision); // TODO: Restore the ability for fields to add accept warnings. $comment_form->setActions($this->getRevisionCommentActions($revision)); $action_uri = $this->getApplicationURI( 'comment/save/'.$revision->getID().'/'); $comment_form->setActionURI($action_uri); $comment_form->setUser($user); $comment_form->setDraft($draft); $comment_form->setReviewers(mpull($reviewers, 'getFullName', 'getPHID')); $comment_form->setCCs(mpull($ccs, 'getFullName', 'getPHID')); // TODO: This just makes the "Z" key work. Generalize this and remove // it at some point. $comment_form = phutil_tag( 'div', array( 'class' => 'differential-add-comment-panel', ), $comment_form); } $pane_id = celerity_generate_unique_node_id(); Javelin::initBehavior( 'differential-keyboard-navigation', array( 'haunt' => $pane_id, )); Javelin::initBehavior('differential-user-select'); $page_pane = id(new DifferentialPrimaryPaneView()) ->setID($pane_id) ->appendChild(array( $comment_view, $diff_history, $warning, $local_view, $toc_view, $other_view, $changeset_view, )); if ($comment_form) { $page_pane->appendChild($comment_form); } else { // TODO: For now, just use this to get "Login to Comment". $page_pane->appendChild( id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) ->setRequestURI($request->getRequestURI())); } $object_id = 'D'.$revision->getID(); $top_anchor = id(new PhabricatorAnchorView()) ->setAnchorName('top') ->setNavigationMarker(true); $content = array( $top_anchor, $revision_detail_box, $page_pane, ); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($object_id, '/'.$object_id); $prefs = $user->loadPreferences(); $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; if ($prefs->getPreference($pref_filetree)) { $collapsed = $prefs->getPreference( PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED, false); $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setAnchorName('top') ->setTitle('D'.$revision->getID()) ->setBaseURI(new PhutilURI('/D'.$revision->getID())) ->setCollapsed((bool)$collapsed) ->build($changesets); $nav->appendChild($content); $nav->setCrumbs($crumbs); $content = $nav; } else { array_unshift($content, $crumbs); } return $this->buildApplicationPage( $content, array( 'title' => $object_id.' '.$revision->getTitle(), 'pageObjects' => array($revision->getPHID()), )); } private function getRevisionActions(DifferentialRevision $revision) { $viewer = $this->getRequest()->getUser(); $revision_id = $revision->getID(); $revision_phid = $revision->getPHID(); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $revision, PhabricatorPolicyCapability::CAN_EDIT); $actions = array(); $actions[] = id(new PhabricatorActionView()) ->setIcon('edit') ->setHref("/differential/revision/edit/{$revision_id}/") ->setName(pht('Edit Revision')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit); $this->requireResource('phabricator-object-selector-css'); $this->requireResource('javelin-behavior-phabricator-object-selector'); $actions[] = id(new PhabricatorActionView()) ->setIcon('link') ->setName(pht('Edit Dependencies')) ->setHref("/search/attach/{$revision_phid}/DREV/dependencies/") ->setWorkflow(true) ->setDisabled(!$can_edit); $maniphest = 'PhabricatorApplicationManiphest'; if (PhabricatorApplication::isClassInstalled($maniphest)) { $actions[] = id(new PhabricatorActionView()) ->setIcon('attach') ->setName(pht('Edit Maniphest Tasks')) ->setHref("/search/attach/{$revision_phid}/TASK/") ->setWorkflow(true) ->setDisabled(!$can_edit); } $request_uri = $this->getRequest()->getRequestURI(); $actions[] = id(new PhabricatorActionView()) ->setIcon('download') ->setName(pht('Download Raw Diff')) ->setHref($request_uri->alter('download', 'true')); return $actions; } private function getRevisionCommentActions(DifferentialRevision $revision) { $actions = array( DifferentialAction::ACTION_COMMENT => true, ); $viewer = $this->getRequest()->getUser(); $viewer_phid = $viewer->getPHID(); $viewer_is_owner = ($viewer_phid == $revision->getAuthorPHID()); $viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers()); $status = $revision->getStatus(); $viewer_has_accepted = false; $viewer_has_rejected = false; $status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED; $status_rejected = DifferentialReviewerStatus::STATUS_REJECTED; foreach ($revision->getReviewerStatus() as $reviewer) { if ($reviewer->getReviewerPHID() == $viewer_phid) { if ($reviewer->getStatus() == $status_accepted) { $viewer_has_accepted = true; } if ($reviewer->getStatus() == $status_rejected) { $viewer_has_rejected = true; } break; } } $allow_self_accept = PhabricatorEnv::getEnvConfig( 'differential.allow-self-accept'); $always_allow_close = PhabricatorEnv::getEnvConfig( 'differential.always-allow-close'); $allow_reopen = PhabricatorEnv::getEnvConfig( 'differential.allow-reopen'); if ($viewer_is_owner) { switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept; $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_RETHINK] = true; break; case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: $actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept; $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_REQUEST] = true; break; case ArcanistDifferentialRevisionStatus::ACCEPTED: $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_REQUEST] = true; $actions[DifferentialAction::ACTION_RETHINK] = true; $actions[DifferentialAction::ACTION_CLOSE] = true; break; case ArcanistDifferentialRevisionStatus::CLOSED: break; case ArcanistDifferentialRevisionStatus::ABANDONED: $actions[DifferentialAction::ACTION_RECLAIM] = true; break; } } else { switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $actions[DifferentialAction::ACTION_ACCEPT] = true; $actions[DifferentialAction::ACTION_REJECT] = true; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: $actions[DifferentialAction::ACTION_ACCEPT] = true; $actions[DifferentialAction::ACTION_REJECT] = !$viewer_has_rejected; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::ACCEPTED: $actions[DifferentialAction::ACTION_ACCEPT] = !$viewer_has_accepted; $actions[DifferentialAction::ACTION_REJECT] = true; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::CLOSED: case ArcanistDifferentialRevisionStatus::ABANDONED: break; } if ($status != ArcanistDifferentialRevisionStatus::CLOSED) { $actions[DifferentialAction::ACTION_CLAIM] = true; $actions[DifferentialAction::ACTION_CLOSE] = $always_allow_close; } } $actions[DifferentialAction::ACTION_ADDREVIEWERS] = true; $actions[DifferentialAction::ACTION_ADDCCS] = true; $actions[DifferentialAction::ACTION_REOPEN] = $allow_reopen && ($status == ArcanistDifferentialRevisionStatus::CLOSED); $actions = array_keys(array_filter($actions)); $actions_dict = array(); foreach ($actions as $action) { $actions_dict[$action] = DifferentialAction::getActionVerb($action); } return $actions_dict; } private function loadInlineComments( DifferentialRevision $revision, array &$changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); $inline_comments = array(); $inline_comments = id(new DifferentialInlineCommentQuery()) ->withRevisionIDs(array($revision->getID())) ->withNotDraft(true) ->execute(); $load_changesets = array(); foreach ($inline_comments as $inline) { $changeset_id = $inline->getChangesetID(); if (isset($changesets[$changeset_id])) { continue; } $load_changesets[$changeset_id] = true; } $more_changesets = array(); if ($load_changesets) { $changeset_ids = array_keys($load_changesets); $more_changesets += id(new DifferentialChangeset()) ->loadAllWhere( 'id IN (%Ld)', $changeset_ids); } if ($more_changesets) { $changesets += $more_changesets; $changesets = msort($changesets, 'getSortKey'); } return $inline_comments; } private function loadChangesetsAndVsMap( DifferentialDiff $target, DifferentialDiff $diff_vs = null, PhabricatorRepository $repository = null) { $load_ids = array(); if ($diff_vs) { $load_ids[] = $diff_vs->getID(); } $load_ids[] = $target->getID(); $raw_changesets = id(new DifferentialChangeset()) ->loadAllWhere( 'diffID IN (%Ld)', $load_ids); $changeset_groups = mgroup($raw_changesets, 'getDiffID'); $changesets = idx($changeset_groups, $target->getID(), array()); $changesets = mpull($changesets, null, 'getID'); $refs = array(); $vs_map = array(); $vs_changesets = array(); if ($diff_vs) { $vs_id = $diff_vs->getID(); $vs_changesets_path_map = array(); foreach (idx($changeset_groups, $vs_id, array()) as $changeset) { $path = $changeset->getAbsoluteRepositoryPath($repository, $diff_vs); $vs_changesets_path_map[$path] = $changeset; $vs_changesets[$changeset->getID()] = $changeset; } foreach ($changesets as $key => $changeset) { $path = $changeset->getAbsoluteRepositoryPath($repository, $target); if (isset($vs_changesets_path_map[$path])) { $vs_map[$changeset->getID()] = $vs_changesets_path_map[$path]->getID(); $refs[$changeset->getID()] = $changeset->getID().'/'.$vs_changesets_path_map[$path]->getID(); unset($vs_changesets_path_map[$path]); } else { $refs[$changeset->getID()] = $changeset->getID(); } } foreach ($vs_changesets_path_map as $path => $changeset) { $changesets[$changeset->getID()] = $changeset; $vs_map[$changeset->getID()] = -1; $refs[$changeset->getID()] = $changeset->getID().'/-1'; } } else { foreach ($changesets as $changeset) { $refs[$changeset->getID()] = $changeset->getID(); } } $changesets = msort($changesets, 'getSortKey'); return array($changesets, $vs_map, $vs_changesets, $refs); } private function buildSymbolIndexes( PhabricatorRepositoryArcanistProject $arc_project, array $visible_changesets) { assert_instances_of($visible_changesets, 'DifferentialChangeset'); $engine = PhabricatorSyntaxHighlighter::newEngine(); $langs = $arc_project->getSymbolIndexLanguages(); if (!$langs) { return array(array(), array()); } $symbol_indexes = array(); $project_phids = array_merge( array($arc_project->getPHID()), nonempty($arc_project->getSymbolIndexProjects(), array())); $indexed_langs = array_fill_keys($langs, true); foreach ($visible_changesets as $key => $changeset) { $lang = $engine->getLanguageFromFilename($changeset->getFilename()); if (isset($indexed_langs[$lang])) { $symbol_indexes[$key] = array( 'lang' => $lang, 'projects' => $project_phids, ); } } return array($symbol_indexes, $project_phids); } private function loadOtherRevisions( array $changesets, DifferentialDiff $target, PhabricatorRepository $repository) { assert_instances_of($changesets, 'DifferentialChangeset'); $paths = array(); foreach ($changesets as $changeset) { $paths[] = $changeset->getAbsoluteRepositoryPath( $repository, $target); } if (!$paths) { return array(); } $path_map = id(new DiffusionPathIDQuery($paths))->loadPathIDs(); if (!$path_map) { return array(); } $query = id(new DifferentialRevisionQuery()) ->setViewer($this->getRequest()->getUser()) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->setOrder(DifferentialRevisionQuery::ORDER_PATH_MODIFIED) ->setLimit(10) ->needFlags(true) ->needDrafts(true) ->needRelationships(true); foreach ($path_map as $path => $path_id) { $query->withPath($repository->getID(), $path_id); } $results = $query->execute(); // Strip out *this* revision. foreach ($results as $key => $result) { if ($result->getID() == $this->revisionID) { unset($results[$key]); } } return $results; } private function renderOtherRevisions(array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); $user = $this->getRequest()->getUser(); $view = id(new DifferentialRevisionListView()) ->setRevisions($revisions) ->setFields(DifferentialRevisionListView::getDefaultFields($user)) ->setUser($user); $phids = $view->getRequiredHandlePHIDs(); $handles = $this->loadViewerHandles($phids); $view->setHandles($handles); return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Open Revisions Affecting These Files')) ->appendChild($view); } /** * Note this code is somewhat similar to the buildPatch method in * @{class:DifferentialReviewRequestMail}. * * @return @{class:AphrontRedirectResponse} */ private function buildRawDiffResponse( DifferentialRevision $revision, array $changesets, array $vs_changesets, array $vs_map, PhabricatorRepository $repository = null) { assert_instances_of($changesets, 'DifferentialChangeset'); assert_instances_of($vs_changesets, 'DifferentialChangeset'); $viewer = $this->getRequest()->getUser(); foreach ($changesets as $changeset) { $changeset->attachHunks($changeset->loadHunks()); } $diff = new DifferentialDiff(); $diff->attachChangesets($changesets); $raw_changes = $diff->buildChangesList(); $changes = array(); foreach ($raw_changes as $changedict) { $changes[] = ArcanistDiffChange::newFromDictionary($changedict); } $loader = id(new PhabricatorFileBundleLoader()) ->setViewer($viewer); $bundle = ArcanistBundle::newFromChanges($changes); $bundle->setLoadFileDataCallback(array($loader, 'loadFileData')); $vcs = $repository ? $repository->getVersionControlSystem() : null; switch ($vcs) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: $raw_diff = $bundle->toGitPatch(); break; case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: default: $raw_diff = $bundle->toUnifiedDiff(); break; } $request_uri = $this->getRequest()->getRequestURI(); // this ends up being something like // D123.diff // or the verbose // D123.vs123.id123.whitespaceignore-all.diff // lame but nice to include these options $file_name = ltrim($request_uri->getPath(), '/').'.'; foreach ($request_uri->getQueryParams() as $key => $value) { if ($key == 'download') { continue; } $file_name .= $key.$value.'.'; } $file_name .= 'diff'; $file = PhabricatorFile::buildFromFileDataOrHash( $raw_diff, array( 'name' => $file_name, 'ttl' => (60 * 60 * 24), 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, )); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $file->attachToObject( $this->getRequest()->getUser(), $revision->getPHID()); unset($unguarded); return id(new AphrontRedirectResponse())->setURI($file->getBestURI()); } private function buildTransactions( DifferentialRevision $revision, DifferentialDiff $left_diff, DifferentialDiff $right_diff, array $changesets) { $viewer = $this->getRequest()->getUser(); $xactions = id(new DifferentialTransactionQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($revision->getPHID())) ->needComments(true) ->execute(); $timeline = id(new DifferentialTransactionView()) ->setUser($viewer) ->setObjectPHID($revision->getPHID()) ->setChangesets($changesets) ->setRevision($revision) ->setLeftDiff($left_diff) ->setRightDiff($right_diff) ->setTransactions($xactions); return $timeline; } private function buildRevisionWarnings( DifferentialRevision $revision, array $handles) { $status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; if ($revision->getStatus() != $status_needs_review) { return; } foreach ($revision->getReviewers() as $reviewer) { if (!$handles[$reviewer]->isDisabled()) { return; } } $warnings = array(); if ($revision->getReviewers()) { $warnings[] = pht( 'This revision needs review, but all specified reviewers are '. 'disabled or inactive.'); } else { $warnings[] = pht( 'This revision needs review, but there are no reviewers specified.'); } return id(new AphrontErrorView()) ->setSeverity(AphrontErrorView::SEVERITY_WARNING) ->setErrors($warnings); } } diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php index cd38700b9a..421208ced7 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/DifferentialRevisionEditor.php @@ -1,996 +1,997 @@ aphrontRequestForEventDispatch = $request; return $this; } public function getAphrontRequestForEventDispatch() { return $this->aphrontRequestForEventDispatch; } public function __construct(DifferentialRevision $revision) { $this->revision = $revision; $this->isCreate = !($revision->getID()); } public static function newRevisionFromConduitWithDiff( array $fields, DifferentialDiff $diff, PhabricatorUser $actor) { $revision = DifferentialRevision::initializeNewRevision($actor); $revision->setPHID($revision->generatePHID()); $editor = new DifferentialRevisionEditor($revision); $editor->setActor($actor); $editor->addDiff($diff, null); $editor->copyFieldsFromConduit($fields); $editor->save(); return $revision; } public function copyFieldsFromConduit(array $fields) { $actor = $this->getActor(); $revision = $this->revision; $revision->loadRelationships(); $all_fields = DifferentialFieldSelector::newSelector() ->getFieldSpecifications(); $aux_fields = array(); foreach ($all_fields as $aux_field) { $aux_field->setRevision($revision); $aux_field->setDiff($this->diff); $aux_field->setUser($actor); if ($aux_field->shouldAppearOnCommitMessage()) { $aux_fields[$aux_field->getCommitMessageKey()] = $aux_field; } } foreach ($fields as $field => $value) { if (empty($aux_fields[$field])) { throw new Exception( "Parsed commit message contains unrecognized field '{$field}'."); } $aux_fields[$field]->setValueFromParsedCommitMessage($value); } foreach ($aux_fields as $aux_field) { $aux_field->validateField(); } $this->setAuxiliaryFields($all_fields); } public function setAuxiliaryFields(array $auxiliary_fields) { assert_instances_of($auxiliary_fields, 'DifferentialFieldSpecification'); $this->auxiliaryFields = $auxiliary_fields; return $this; } public function getRevision() { return $this->revision; } public function setReviewers(array $reviewers) { $this->reviewers = $reviewers; return $this; } public function setCCPHIDs(array $cc) { $this->cc = $cc; return $this; } public function setContentSource(PhabricatorContentSource $content_source) { $this->contentSource = $content_source; return $this; } public function addDiff(DifferentialDiff $diff, $comments) { if ($diff->getRevisionID() && $diff->getRevisionID() != $this->getRevision()->getID()) { $diff_id = (int)$diff->getID(); $targ_id = (int)$this->getRevision()->getID(); $real_id = (int)$diff->getRevisionID(); throw new Exception( "Can not attach diff #{$diff_id} to Revision D{$targ_id}, it is ". "already attached to D{$real_id}."); } $this->diff = $diff; $this->comments = $comments; $repository = id(new DifferentialRepositoryLookup()) ->setViewer($this->getActor()) ->setDiff($diff) ->lookupRepository(); if ($repository) { $this->getRevision()->setRepositoryPHID($repository->getPHID()); } return $this; } protected function getDiff() { return $this->diff; } protected function getComments() { return $this->comments; } protected function getActorPHID() { return $this->getActor()->getPHID(); } public function isNewRevision() { return !$this->getRevision()->getID(); } /** * A silent update does not trigger Herald rules or send emails. This is used * for auto-amends at commit time. */ public function setSilentUpdate($silent) { $this->silentUpdate = $silent; return $this; } public function save() { $revision = $this->getRevision(); $is_new = $this->isNewRevision(); $revision->loadRelationships(); $this->willWriteRevision(); if ($this->reviewers === null) { $this->reviewers = $revision->getReviewers(); } if ($this->cc === null) { $this->cc = $revision->getCCPHIDs(); } if ($is_new) { $content_blocks = array(); foreach ($this->auxiliaryFields as $field) { if ($field->shouldExtractMentions()) { $content_blocks[] = $field->renderValueForCommitMessage(false); } } $phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( $content_blocks); $this->cc = array_unique(array_merge($this->cc, $phids)); } $diff = $this->getDiff(); if ($diff) { $revision->setLineCount($diff->getLineCount()); } // Save the revision, to generate its ID and PHID if it is new. We need // the ID/PHID in order to record them in Herald transcripts, but don't // want to hold a transaction open while running Herald because it is // potentially somewhat slow. The downside is that we may end up with a // saved revision/diff pair without appropriate CCs. We could be better // about this -- for example: // // - Herald can't affect reviewers, so we could compute them before // opening the transaction and then save them in the transaction. // - Herald doesn't *really* need PHIDs to compute its effects, we could // run it before saving these objects and then hand over the PHIDs later. // // But this should address the problem of orphaned revisions, which is // currently the only problem we experience in practice. $revision->openTransaction(); if ($diff) { $revision->setBranchName($diff->getBranch()); $revision->setArcanistProjectPHID($diff->getArcanistProjectPHID()); } $revision->save(); if ($diff) { $diff->setRevisionID($revision->getID()); $diff->save(); } $revision->saveTransaction(); // We're going to build up three dictionaries: $add, $rem, and $stable. The // $add dictionary has added reviewers/CCs. The $rem dictionary has // reviewers/CCs who have been removed, and the $stable array is // reviewers/CCs who haven't changed. We're going to send new reviewers/CCs // a different ("welcome") email than we send stable reviewers/CCs. $old = array( 'rev' => array_fill_keys($revision->getReviewers(), true), 'ccs' => array_fill_keys($revision->getCCPHIDs(), true), ); $xscript_header = null; $xscript_uri = null; $new = array( 'rev' => array_fill_keys($this->reviewers, true), 'ccs' => array_fill_keys($this->cc, true), ); $rem_ccs = array(); $xscript_phid = null; if ($diff) { $unsubscribed_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER); $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( $revision, $diff); $adapter->setExplicitCCs($new['ccs']); $adapter->setExplicitReviewers($new['rev']); $adapter->setForbiddenCCs($unsubscribed_phids); $adapter->setIsNewObject($is_new); $xscript = HeraldEngine::loadAndApplyRules($adapter); $xscript_uri = '/herald/transcript/'.$xscript->getID().'/'; $xscript_phid = $xscript->getPHID(); $xscript_header = $xscript->getXHeraldRulesHeader(); $xscript_header = HeraldTranscript::saveXHeraldRulesHeader( $revision->getPHID(), $xscript_header); $sub = array( 'rev' => $adapter->getReviewersAddedByHerald(), 'ccs' => $adapter->getCCsAddedByHerald(), ); $rem_ccs = $adapter->getCCsRemovedByHerald(); $blocking_reviewers = array_keys( $adapter->getBlockingReviewersAddedByHerald()); HarbormasterBuildable::applyBuildPlans( $diff->getPHID(), $revision->getPHID(), $adapter->getBuildPlans()); } else { $sub = array( 'rev' => array(), 'ccs' => array(), ); $blocking_reviewers = array(); } // Remove any CCs which are prevented by Herald rules. $sub['ccs'] = array_diff_key($sub['ccs'], $rem_ccs); $new['ccs'] = array_diff_key($new['ccs'], $rem_ccs); $add = array(); $rem = array(); $stable = array(); foreach (array('rev', 'ccs') as $key) { $add[$key] = array(); if ($new[$key] !== null) { $add[$key] += array_diff_key($new[$key], $old[$key]); } $add[$key] += array_diff_key($sub[$key], $old[$key]); $combined = $sub[$key]; if ($new[$key] !== null) { $combined += $new[$key]; } $rem[$key] = array_diff_key($old[$key], $combined); $stable[$key] = array_diff_key($old[$key], $add[$key] + $rem[$key]); } // Prevent Herald rules from adding a revision's owner as a reviewer. unset($add['rev'][$revision->getAuthorPHID()]); self::updateReviewers( $revision, $this->getActor(), array_keys($add['rev']), array_keys($rem['rev']), $blocking_reviewers); // We want to attribute new CCs to a "reasonPHID", representing the reason // they were added. This is either a user (if some user explicitly CCs // them, or uses "Add CCs...") or a Herald transcript PHID, indicating that // they were added by a Herald rule. if ($add['ccs'] || $rem['ccs']) { $reasons = array(); foreach ($add['ccs'] as $phid => $ignored) { if (empty($new['ccs'][$phid])) { $reasons[$phid] = $xscript_phid; } else { $reasons[$phid] = $this->getActorPHID(); } } foreach ($rem['ccs'] as $phid => $ignored) { if (empty($new['ccs'][$phid])) { $reasons[$phid] = $this->getActorPHID(); } else { $reasons[$phid] = $xscript_phid; } } } else { $reasons = $this->getActorPHID(); } self::alterCCs( $revision, $this->cc, array_keys($rem['ccs']), array_keys($add['ccs']), $reasons); $this->updateAuxiliaryFields(); // Add the author and users included from Herald rules to the relevant set // of users so they get a copy of the email. if (!$this->silentUpdate) { if ($is_new) { $add['rev'][$this->getActorPHID()] = true; if ($diff) { $add['rev'] += $adapter->getEmailPHIDsAddedByHerald(); } } else { $stable['rev'][$this->getActorPHID()] = true; if ($diff) { $stable['rev'] += $adapter->getEmailPHIDsAddedByHerald(); } } } $mail = array(); $phids = array($this->getActorPHID()); $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->getActor()) ->withPHIDs($phids) ->execute(); $actor_handle = $handles[$this->getActorPHID()]; $changesets = null; $old_status = $revision->getStatus(); if ($diff) { $changesets = $diff->loadChangesets(); // TODO: This should probably be in DifferentialFeedbackEditor? if (!$is_new) { $this->createComment(); $mail[] = id(new DifferentialNewDiffMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) ->setIsFirstMailAboutRevision(false) ->setIsFirstMailToRecipients(false) ->setCommentText($this->getComments()) ->setToPHIDs(array_keys($stable['rev'])) ->setCCPHIDs(array_keys($stable['ccs'])); } // Save the changes we made above. $diff->setDescription(preg_replace('/\n.*/s', '', $this->getComments())); $diff->save(); $this->updateAffectedPathTable($revision, $diff, $changesets); $this->updateRevisionHashTable($revision, $diff); // An updated diff should require review, as long as it's not closed // or accepted. The "accepted" status is "sticky" to encourage courtesy // re-diffs after someone accepts with minor changes/suggestions. $status = $revision->getStatus(); if ($status != ArcanistDifferentialRevisionStatus::CLOSED && $status != ArcanistDifferentialRevisionStatus::ACCEPTED) { $revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); } } else { $diff = $revision->loadActiveDiff(); if ($diff) { $changesets = $diff->loadChangesets(); } else { $changesets = array(); } } $revision->save(); // If the actor just deleted all the blocking/rejected reviewers, we may // be able to put the revision into "accepted". switch ($revision->getStatus()) { case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $revision = self::updateAcceptedStatus( $this->getActor(), $revision); break; } $this->didWriteRevision(); $event_data = array( 'revision_id' => $revision->getID(), 'revision_phid' => $revision->getPHID(), 'revision_name' => $revision->getTitle(), 'revision_author_phid' => $revision->getAuthorPHID(), 'action' => $is_new ? DifferentialAction::ACTION_CREATE : DifferentialAction::ACTION_UPDATE, 'feedback_content' => $is_new ? phutil_utf8_shorten($revision->getSummary(), 140) : $this->getComments(), 'actor_phid' => $revision->getAuthorPHID(), ); $mailed_phids = array(); if (!$this->silentUpdate) { $revision->loadRelationships(); if ($add['rev']) { $message = id(new DifferentialNewDiffMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) ->setIsFirstMailAboutRevision($is_new) ->setIsFirstMailToRecipients(true) ->setToPHIDs(array_keys($add['rev'])); if ($is_new) { // The first time we send an email about a revision, put the CCs in // the "CC:" field of the same "Review Requested" email that reviewers // get, so you don't get two initial emails if you're on a list that // is CC'd. $message->setCCPHIDs(array_keys($add['ccs'])); } $mail[] = $message; } // If we added CCs, we want to send them an email, but only if they were // not already a reviewer and were not added as one (in these cases, they // got a "NewDiff" mail, either in the past or just a moment ago). You can // still get two emails, but only if a revision is updated and you are // added as a reviewer at the same time a list you are on is added as a // CC, which is rare and reasonable. $implied_ccs = self::getImpliedCCs($revision); $implied_ccs = array_fill_keys($implied_ccs, true); $add['ccs'] = array_diff_key($add['ccs'], $implied_ccs); if (!$is_new && $add['ccs']) { $mail[] = id(new DifferentialCCWelcomeMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) ->setIsFirstMailToRecipients(true) ->setToPHIDs(array_keys($add['ccs'])); } foreach ($mail as $message) { $message->setHeraldTranscriptURI($xscript_uri); $message->setXHeraldRulesHeader($xscript_header); $message->send(); $mailed_phids[] = $message->getRawMail()->buildRecipientList(); } $mailed_phids = array_mergev($mailed_phids); } id(new PhabricatorFeedStoryPublisher()) ->setStoryType('PhabricatorFeedStoryDifferential') ->setStoryData($event_data) ->setStoryTime(time()) ->setStoryAuthorPHID($revision->getAuthorPHID()) ->setRelatedPHIDs( array( $revision->getPHID(), $revision->getAuthorPHID(), )) ->setPrimaryObjectPHID($revision->getPHID()) ->setSubscribedPHIDs( array_merge( array($revision->getAuthorPHID()), $revision->getReviewers(), $revision->getCCPHIDs())) ->setMailRecipientPHIDs($mailed_phids) ->publish(); id(new PhabricatorSearchIndexer()) ->queueDocumentForIndexing($revision->getPHID()); } public static function addCC( DifferentialRevision $revision, $phid, $reason) { return self::alterCCs( $revision, $revision->getCCPHIDs(), $rem = array(), $add = array($phid), $reason); } protected static function alterCCs( DifferentialRevision $revision, array $stable_phids, array $rem_phids, array $add_phids, $reason_phid) { $dont_add = self::getImpliedCCs($revision); $add_phids = array_diff($add_phids, $dont_add); id(new PhabricatorSubscriptionsEditor()) ->setActor(PhabricatorUser::getOmnipotentUser()) ->setObject($revision) ->subscribeExplicit($add_phids) ->unsubscribe($rem_phids) ->save(); } private static function getImpliedCCs(DifferentialRevision $revision) { return array_merge( $revision->getReviewers(), array($revision->getAuthorPHID())); } public static function updateReviewers( DifferentialRevision $revision, PhabricatorUser $actor, array $add_phids, array $remove_phids, array $blocking_phids = array()) { $reviewers = $revision->getReviewers(); $editor = id(new PhabricatorEdgeEditor()) ->setActor($actor); $reviewer_phids_map = array_fill_keys($reviewers, true); $blocking_phids = array_fuse($blocking_phids); foreach ($add_phids as $phid) { // Adding an already existing edge again would have cause memory loss // That is, the previous state for that reviewer would be lost if (isset($reviewer_phids_map[$phid])) { // TODO: If we're writing a blocking edge, we should overwrite an // existing weaker edge (like "added" or "commented"), just not a // stronger existing edge. continue; } if (isset($blocking_phids[$phid])) { $status = DifferentialReviewerStatus::STATUS_BLOCKING; } else { $status = DifferentialReviewerStatus::STATUS_ADDED; } $options = array( 'data' => array( 'status' => $status, ) ); $editor->addEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $phid, $options); } foreach ($remove_phids as $phid) { $editor->removeEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $phid); } $editor->save(); } public static function updateReviewerStatus( DifferentialRevision $revision, PhabricatorUser $actor, $reviewer_phid, $status) { $options = array( 'data' => array( 'status' => $status ) ); $active_diff = $revision->loadActiveDiff(); if ($active_diff) { $options['data']['diff'] = $active_diff->getID(); } id(new PhabricatorEdgeEditor()) ->setActor($actor) ->addEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $reviewer_phid, $options) ->save(); } private function createComment() { $template = id(new DifferentialComment()) ->setAuthorPHID($this->getActorPHID()) ->setRevision($this->revision); if ($this->contentSource) { $content_source = $this->contentSource; } else { $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_LEGACY, array()); } $template->setContentSource($content_source); // Write the "update active diff" transaction. id(clone $template) ->setAction(DifferentialAction::ACTION_UPDATE) ->setMetadata( array( DifferentialComment::METADATA_DIFF_ID => $this->getDiff()->getID(), )) ->save(); // If we have a comment, write the "add a comment" transaction. if (strlen($this->getComments())) { id(clone $template) ->setAction(DifferentialAction::ACTION_COMMENT) ->setContent($this->getComments()) ->save(); } } private function updateAuxiliaryFields() { $aux_map = array(); foreach ($this->auxiliaryFields as $aux_field) { $key = $aux_field->getStorageKey(); if ($key !== null) { $val = $aux_field->getValueForStorage(); $aux_map[$key] = $val; } } if (!$aux_map) { return; } $revision = $this->revision; $fields = id(new DifferentialCustomFieldStorage())->loadAllWhere( 'objectPHID = %s', $revision->getPHID()); $fields = mpull($fields, null, 'getFieldIndex'); foreach ($aux_map as $key => $val) { $index = PhabricatorHash::digestForIndex($key); $obj = idx($fields, $index); if (!strlen($val)) { // If the new value is empty, just delete the old row if one exists and // don't add a new row if it doesn't. if ($obj) { $obj->delete(); } } else { if (!$obj) { $obj = new DifferentialCustomFieldStorage(); $obj->setObjectPHID($revision->getPHID()); $obj->setFieldIndex($index); } if ($obj->getFieldValue() !== $val) { $obj->setFieldValue($val); $obj->save(); } } } } private function willWriteRevision() { foreach ($this->auxiliaryFields as $aux_field) { $aux_field->willWriteRevision($this); } $this->dispatchEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_WILLEDITREVISION); } private function didWriteRevision() { foreach ($this->auxiliaryFields as $aux_field) { $aux_field->didWriteRevision($this); } $this->dispatchEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_DIDEDITREVISION); } private function dispatchEvent($type) { $event = new PhabricatorEvent( $type, array( 'revision' => $this->revision, 'new' => $this->isCreate, )); $event->setUser($this->getActor()); $request = $this->getAphrontRequestForEventDispatch(); if ($request) { $event->setAphrontRequest($request); } PhutilEventEngine::dispatchEvent($event); } /** * Update the table which links Differential revisions to paths they affect, * so Diffusion can efficiently find pending revisions for a given file. */ private function updateAffectedPathTable( DifferentialRevision $revision, DifferentialDiff $diff, array $changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); $project = $diff->loadArcanistProject(); if (!$project) { // Probably an old revision from before projects. return; } $repository = $project->loadRepository(); if (!$repository) { // Probably no project <-> repository link, or the repository where the // project lives is untracked. return; } $path_prefix = null; $local_root = $diff->getSourceControlPath(); if ($local_root) { // We're in a working copy which supports subdirectory checkouts (e.g., // SVN) so we need to figure out what prefix we should add to each path // (e.g., trunk/projects/example/) to get the absolute path from the // root of the repository. DVCS systems like Git and Mercurial are not // affected. // Normalize both paths and check if the repository root is a prefix of // the local root. If so, throw it away. Note that this correctly handles // the case where the remote path is "/". $local_root = id(new PhutilURI($local_root))->getPath(); $local_root = rtrim($local_root, '/'); $repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath(); $repo_root = rtrim($repo_root, '/'); if (!strncmp($repo_root, $local_root, strlen($repo_root))) { $path_prefix = substr($local_root, strlen($repo_root)); } } $paths = array(); foreach ($changesets as $changeset) { $paths[] = $path_prefix.'/'.$changeset->getFilename(); } // Mark this as also touching all parent paths, so you can see all pending // changes to any file within a directory. $all_paths = array(); foreach ($paths as $local) { foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) { $all_paths[$path] = true; } } $all_paths = array_keys($all_paths); $path_ids = PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths( $all_paths); $table = new DifferentialAffectedPath(); $conn_w = $table->establishConnection('w'); $sql = array(); foreach ($path_ids as $path_id) { $sql[] = qsprintf( $conn_w, '(%d, %d, %d, %d)', $repository->getID(), $path_id, time(), $revision->getID()); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', $table->getTableName(), $revision->getID()); foreach (array_chunk($sql, 256) as $chunk) { queryfx( $conn_w, 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q', $table->getTableName(), implode(', ', $chunk)); } } /** * Update the table connecting revisions to DVCS local hashes, so we can * identify revisions by commit/tree hashes. */ private function updateRevisionHashTable( DifferentialRevision $revision, DifferentialDiff $diff) { $vcs = $diff->getSourceControlSystem(); if ($vcs == DifferentialRevisionControlSystem::SVN) { // Subversion has no local commit or tree hash information, so we don't // have to do anything. return; } $property = id(new DifferentialDiffProperty())->loadOneWhere( 'diffID = %d AND name = %s', $diff->getID(), 'local:commits'); if (!$property) { return; } $hashes = array(); $data = $property->getData(); switch ($vcs) { case DifferentialRevisionControlSystem::GIT: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT, $commit['commit'], ); $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_TREE, $commit['tree'], ); } break; case DifferentialRevisionControlSystem::MERCURIAL: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT, $commit['rev'], ); } break; } $conn_w = $revision->establishConnection('w'); $sql = array(); foreach ($hashes as $info) { list($type, $hash) = $info; $sql[] = qsprintf( $conn_w, '(%d, %s, %s)', $revision->getID(), $type, $hash); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', ArcanistDifferentialRevisionHash::TABLE_NAME, $revision->getID()); if ($sql) { queryfx( $conn_w, 'INSERT INTO %T (revisionID, type, hash) VALUES %Q', ArcanistDifferentialRevisionHash::TABLE_NAME, implode(', ', $sql)); } } /** * Try to move a revision to "accepted". We look for: * * - at least one accepting reviewer who is a user; and * - no rejects; and * - no blocking reviewers. */ public static function updateAcceptedStatus( PhabricatorUser $viewer, DifferentialRevision $revision) { $revision = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs(array($revision->getID())) ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); $has_user_accept = false; foreach ($revision->getReviewerStatus() as $reviewer) { $status = $reviewer->getStatus(); if ($status == DifferentialReviewerStatus::STATUS_BLOCKING) { // We have a blocking reviewer, so just leave the revision in its // existing state. return $revision; } if ($status == DifferentialReviewerStatus::STATUS_REJECTED) { // We have a rejecting reviewer, so leave the revisoin as is. return $revision; } if ($reviewer->isUser()) { if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) { $has_user_accept = true; } } } if ($has_user_accept) { $revision ->setStatus(ArcanistDifferentialRevisionStatus::ACCEPTED) ->save(); } return $revision; } } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index b4fdddbffc..c8ab780348 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1,1070 +1,1076 @@ getTransactionType()) { case PhabricatorTransactions::TYPE_VIEW_POLICY: return $object->getViewPolicy(); case PhabricatorTransactions::TYPE_EDIT_POLICY: return $object->getEditPolicy(); case DifferentialTransaction::TYPE_ACTION: return null; case DifferentialTransaction::TYPE_INLINE: return null; case DifferentialTransaction::TYPE_UPDATE: if ($this->getIsNewObject()) { return null; } else { return $object->getActiveDiff()->getPHID(); } } return parent::getCustomTransactionOldValue($object, $xaction); } protected function getCustomTransactionNewValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY: case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_UPDATE: return $xaction->getNewValue(); case DifferentialTransaction::TYPE_INLINE: return null; } return parent::getCustomTransactionNewValue($object, $xaction); } protected function transactionHasEffect( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_INLINE: return $xaction->hasComment(); case DifferentialTransaction::TYPE_ACTION: $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; + $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; $action_type = $xaction->getNewValue(); switch ($action_type) { case DifferentialAction::ACTION_ACCEPT: case DifferentialAction::ACTION_REJECT: if ($action_type == DifferentialAction::ACTION_ACCEPT) { $new_status = DifferentialReviewerStatus::STATUS_ACCEPTED; } else { $new_status = DifferentialReviewerStatus::STATUS_REJECTED; } $actor = $this->getActor(); $actor_phid = $actor->getPHID(); - // These transactions can cause effects in to ways: by altering the + // These transactions can cause effects in two ways: by altering the // status of an existing reviewer; or by adding the actor as a new // reviewer. $will_add_reviewer = true; foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->hasAuthority($actor)) { if ($reviewer->getStatus() != $new_status) { return true; } } if ($reviewer->getReviewerPHID() == $actor_phid) { $will_add_reviwer = false; } } return $will_add_reviewer; case DifferentialAction::ACTION_CLOSE: return ($object->getStatus() != $status_closed); case DifferentialAction::ACTION_ABANDON: return ($object->getStatus() != $status_abandoned); case DifferentialAction::ACTION_RECLAIM: return ($object->getStatus() == $status_abandoned); case DifferentialAction::ACTION_REOPEN: return ($object->getStatus() == $status_closed); case DifferentialAction::ACTION_RETHINK: - return ($object->getStatus() != $status_revision); + return ($object->getStatus() != $status_plan); case DifferentialAction::ACTION_REQUEST: return ($object->getStatus() != $status_review); case DifferentialAction::ACTION_RESIGN: $actor_phid = $this->getActor()->getPHID(); foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->getReviewerPHID() == $actor_phid) { return true; } } return false; case DifferentialAction::ACTION_CLAIM: $actor_phid = $this->getActor()->getPHID(); return ($actor_phid != $object->getAuthorPHID()); } } return parent::transactionHasEffect($object, $xaction); } protected function applyCustomInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; + $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_VIEW_POLICY: $object->setViewPolicy($xaction->getNewValue()); return; case PhabricatorTransactions::TYPE_EDIT_POLICY: $object->setEditPolicy($xaction->getNewValue()); return; case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_INLINE: return; case PhabricatorTransactions::TYPE_EDGE: return; case DifferentialTransaction::TYPE_UPDATE: $object->setStatus($status_review); // TODO: Update the `diffPHID` once we add that. return; case DifferentialTransaction::TYPE_ACTION: switch ($xaction->getNewValue()) { case DifferentialAction::ACTION_RESIGN: case DifferentialAction::ACTION_ACCEPT: case DifferentialAction::ACTION_REJECT: // These have no direct effects, and affect review status only // indirectly by altering reviewers with TYPE_EDGE transactions. return; case DifferentialAction::ACTION_ABANDON: $object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED); return; case DifferentialAction::ACTION_RETHINK: - $object->setStatus($status_revision); + $object->setStatus($status_plan); return; case DifferentialAction::ACTION_RECLAIM: $object->setStatus($status_review); return; case DifferentialAction::ACTION_REOPEN: $object->setStatus($status_review); return; case DifferentialAction::ACTION_REQUEST: $object->setStatus($status_review); return; case DifferentialAction::ACTION_CLOSE: $object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED); return; case DifferentialAction::ACTION_CLAIM: $object->setAuthorPHID($this->getActor()->getPHID()); return; } break; } return parent::applyCustomInternalTransaction($object, $xaction); } protected function expandTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $actor = $this->getActor(); $actor_phid = $actor->getPHID(); $type_edge = PhabricatorTransactions::TYPE_EDGE; $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER; $results = parent::expandTransaction($object, $xaction); switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $new_accept = DifferentialReviewerStatus::STATUS_ACCEPTED; $new_reject = DifferentialReviewerStatus::STATUS_REJECTED; $old_accept = DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER; $old_reject = DifferentialReviewerStatus::STATUS_REJECTED_OLDER; // When a revision is updated, change all "reject" to "rejected older // revision". This means we won't immediately push the update back into // "needs review", but outstanding rejects will still block it from // moving to "accepted". $edits = array(); foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->getStatus() == $new_reject) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => array( 'status' => $old_reject, ), ); } // TODO: If sticky accept is off, do a similar update for accepts. } if ($edits) { $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); } break; case PhabricatorTransactions::TYPE_COMMENT: // When a user leaves a comment, upgrade their reviewer status from // "added" to "commented" if they're also a reviewer. We may further // upgrade this based on other actions in the transaction group. $status_added = DifferentialReviewerStatus::STATUS_ADDED; $status_commented = DifferentialReviewerStatus::STATUS_COMMENTED; $data = array( 'status' => $status_commented, ); $edits = array(); foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->getReviewerPHID() == $actor_phid) { if ($reviewer->getStatus() == $status_added) { $edits[$actor_phid] = array( 'data' => $data, ); } } } if ($edits) { $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); } break; case DifferentialTransaction::TYPE_ACTION: $action_type = $xaction->getNewValue(); switch ($action_type) { case DifferentialAction::ACTION_ACCEPT: case DifferentialAction::ACTION_REJECT: if ($action_type == DifferentialAction::ACTION_ACCEPT) { $data = array( 'status' => DifferentialReviewerStatus::STATUS_ACCEPTED, ); } else { $data = array( 'status' => DifferentialReviewerStatus::STATUS_REJECTED, ); } $edits = array(); foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->hasAuthority($actor)) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => $data, ); } } // Also either update or add the actor themselves as a reviewer. $edits[$actor_phid] = array( 'data' => $data, ); $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); break; case DifferentialAction::ACTION_CLAIM: // If the user is commandeering, add the previous owner as a // reviewer and remove the actor. $edits = array( '-' => array( $actor_phid => $actor_phid, ), ); $owner_phid = $object->getAuthorPHID(); if ($owner_phid) { $reviewer = new DifferentialReviewer( $owner_phid, array( 'status' => DifferentialReviewerStatus::STATUS_ADDED, )); $edits['+'] = array( $owner_phid => array( 'data' => $reviewer->getEdgeData(), ), ); } $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue($edits); break; case DifferentialAction::ACTION_RESIGN: // If the user is resigning, add a separate reviewer edit // transaction which removes them as a reviewer. $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue( array( '-' => array( $actor_phid => $actor_phid, ), )); break; } break; } return $results; } protected function applyCustomExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY: return; case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_EDGE: case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_INLINE: return; case DifferentialTransaction::TYPE_UPDATE: // Now that we're inside the transaction, do a final check. $diff = $this->loadDiff($xaction->getNewValue()); // TODO: It would be slightly cleaner to just revalidate this // transaction somehow using the same validation code, but that's // not easy to do at the moment. if (!$diff) { throw new Exception(pht('Diff does not exist!')); } else { $revision_id = $diff->getRevisionID(); if ($revision_id && ($revision_id != $object->getID())) { throw new Exception( pht( 'Diff is already attached to another revision. You lost '. 'a race?')); } } $diff->setRevisionID($object->getID()); $diff->save(); $object->setLineCount($diff->getLineCount()); $object->setRepositoryPHID($diff->getRepositoryPHID()); return; } return parent::applyCustomExternalTransaction($object, $xaction); } protected function mergeEdgeData($type, array $u, array $v) { $result = parent::mergeEdgeData($type, $u, $v); switch ($type) { case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER: // When the same reviewer has their status updated by multiple // transactions, we want the strongest status to win. An example of // this is when a user adds a comment and also accepts a revision which // they are a reviewer on. The comment creates a "commented" status, // while the accept creates an "accepted" status. Since accept is // stronger, it should win and persist. $u_status = idx($u, 'status'); $v_status = idx($v, 'status'); $u_str = DifferentialReviewerStatus::getStatusStrength($u_status); $v_str = DifferentialReviewerStatus::getStatusStrength($v_status); if ($u_str > $v_str) { $result['status'] = $u_status; } else { $result['status'] = $v_status; } break; } return $result; } protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $old_status = $object->getStatus(); switch ($old_status) { case $status_accepted: case $status_revision: case $status_review: // Load the most up-to-date version of the revision and its reviewers, // so we don't need to try to deduce the state of reviewers by examining // all the changes made by the transactions. $new_revision = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->needReviewerStatus(true) ->withIDs(array($object->getID())) ->executeOne(); if (!$new_revision) { throw new Exception( pht('Failed to load revision from transaction finalization.')); } // Try to move a revision to "accepted". We look for: // // - at least one accepting reviewer who is a user; and // - no rejects; and // - no rejects of older diffs; and // - no blocking reviewers. $has_accepting_user = false; $has_rejecting_reviewer = false; $has_rejecting_older_reviewer = false; $has_blocking_reviewer = false; foreach ($new_revision->getReviewerStatus() as $reviewer) { $reviewer_status = $reviewer->getStatus(); switch ($reviewer_status) { case DifferentialReviewerStatus::STATUS_REJECTED: $has_rejecting_reviewer = true; break; case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: $has_rejecting_older_reviewer = true; break; case DifferentialReviewerStatus::STATUS_BLOCKING: $has_blocking_reviewer = true; break; case DifferentialReviewerStatus::STATUS_ACCEPTED: if ($reviewer->isUser()) { $has_accepting_user = true; } break; } } $new_status = null; if ($has_accepting_user && !$has_rejecting_reviewer && !$has_rejecting_older_reviewer && !$has_blocking_reviewer) { $new_status = $status_accepted; } else if ($has_rejecting_reviewer) { // This isn't accepted, and there's at least one rejecting reviewer, // so the revision needs changes. This usually happens after a // "reject". $new_status = $status_revision; } else if ($old_status == $status_accepted) { // This revision was accepted, but it no longer satisfies the // conditions for acceptance. This usually happens after an accepting // reviewer resigns or is removed. $new_status = $status_review; } if ($new_status !== null && $new_status != $old_status) { $xaction = id(new DifferentialTransaction()) ->setTransactionType(DifferentialTransaction::TYPE_STATUS) ->setOldValue($old_status) ->setNewValue($new_status); $xaction = $this->populateTransaction($object, $xaction)->save(); $xactions[] = $xaction; $object->setStatus($new_status)->save(); } break; default: // Revisions can't transition out of other statuses (like closed or // abandoned) as a side effect of reviewer status changes. break; } return $xactions; } protected function validateTransaction( PhabricatorLiskDAO $object, $type, array $xactions) { $errors = parent::validateTransaction($object, $type, $xactions); foreach ($xactions as $xaction) { switch ($type) { case DifferentialTransaction::TYPE_UPDATE: $diff = $this->loadDiff($xaction->getNewValue()); if (!$diff) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht('The specified diff does not exist.'), $xaction); } else if (($diff->getRevisionID()) && ($diff->getRevisionID() != $object->getID())) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht( 'You can not update this revision to the specified diff, '. 'because the diff is already attached to another revision.'), $xaction); } break; case DifferentialTransaction::TYPE_ACTION: $error = $this->validateDifferentialAction( $object, $type, $xaction, $xaction->getNewValue()); if ($error) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), $error, $xaction); } break; } } return $errors; } private function validateDifferentialAction( DifferentialRevision $revision, $type, DifferentialTransaction $xaction, $action) { $author_phid = $revision->getAuthorPHID(); $actor_phid = $this->getActor()->getPHID(); $actor_is_author = ($author_phid == $actor_phid); $config_close_key = 'differential.always-allow-close'; $always_allow_close = PhabricatorEnv::getEnvConfig($config_close_key); $config_reopen_key = 'differential.allow-reopen'; $allow_reopen = PhabricatorEnv::getEnvConfig($config_reopen_key); $config_self_accept_key = 'differential.allow-self-accept'; $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key); $revision_status = $revision->getStatus(); $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; switch ($action) { case DifferentialAction::ACTION_ACCEPT: if ($actor_is_author && !$allow_self_accept) { return pht( 'You can not accept this revision because you are the owner.'); } if ($revision_status == $status_abandoned) { return pht( 'You can not accept this revision because it has been '. 'abandoned.'); } if ($revision_status == $status_closed) { return pht( 'You can not accept this revision because it has already been '. 'closed.'); } break; case DifferentialAction::ACTION_REJECT: if ($actor_is_author) { return pht( 'You can not request changes to your own revision.'); } if ($revision_status == $status_abandoned) { return pht( 'You can not request changes to this revision because it has been '. 'abandoned.'); } if ($revision_status == $status_closed) { return pht( 'You can not request changes to this revision because it has '. 'already been closed.'); } break; case DifferentialAction::ACTION_RESIGN: // You can always resign from a revision if you're a reviewer. If you // aren't, this is a no-op rather than invalid. break; case DifferentialAction::ACTION_CLAIM: // You can claim a revision if you're not the owner. If you are, this // is a no-op rather than invalid. if ($revision_status == $status_closed) { return pht( "You can not commandeer this revision because it has already been ". "closed."); } break; case DifferentialAction::ACTION_ABANDON: if (!$actor_is_author) { return pht( "You can not abandon this revision because you do not own it. ". "You can only abandon revisions you own."); } if ($revision_status == $status_closed) { return pht( "You can not abandon this revision because it has already been ". "closed."); } // NOTE: Abandons of already-abandoned revisions are treated as no-op // instead of invalid. Other abandons are OK. break; case DifferentialAction::ACTION_RECLAIM: if (!$actor_is_author) { return pht( "You can not reclaim this revision because you do not own ". "it. You can only reclaim revisions you own."); } if ($revision_status == $status_closed) { return pht( "You can not reclaim this revision because it has already been ". "closed."); } // NOTE: Reclaims of other non-abandoned revisions are treated as no-op // instead of invalid. break; case DifferentialAction::ACTION_REOPEN: if (!$allow_reopen) { return pht( 'The reopen action is not enabled on this Phabricator install. '. 'Adjust your configuration to enable it.'); } // NOTE: If the revision is not closed, this is caught as a no-op // instead of an invalid transaction. break; case DifferentialAction::ACTION_RETHINK: if (!$actor_is_author) { return pht( "You can not plan changes to this revision because you do not ". - "own it. To plan chagnes to a revision, you must be its owner."); + "own it. To plan changes to a revision, you must be its owner."); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: // These are OK. break; + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: + // Let this through, it's a no-op. + break; case ArcanistDifferentialRevisionStatus::ABANDONED: return pht( "You can not plan changes to this revision because it has ". "been abandoned."); case ArcanistDifferentialRevisionStatus::CLOSED: return pht( "You can not plan changes to this revision because it has ". "already been closed."); default: throw new Exception( pht( 'Encountered unexpected revision status ("%s") when '. 'validating "%s" action.', $revision_status, $action)); } break; case DifferentialAction::ACTION_REQUEST: if (!$actor_is_author) { return pht( "You can not request review of this revision because you do ". "not own it. To request review of a revision, you must be its ". "owner."); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: // These are OK. break; case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: // This will be caught as "no effect" later on. break; case ArcanistDifferentialRevisionStatus::ABANDONED: return pht( "You can not request review of this revision because it has ". "been abandoned. Instead, reclaim it."); case ArcanistDifferentialRevisionStatus::CLOSED: return pht( "You can not request review of this revision because it has ". "already been closed."); default: throw new Exception( pht( 'Encountered unexpected revision status ("%s") when '. 'validating "%s" action.', $revision_status, $action)); } break; case DifferentialAction::ACTION_CLOSE: // TODO: Permit the daemons to take this action in all cases. if (!$actor_is_author && !$always_allow_close) { return pht( "You can not close this revision because you do not own it. To ". "close a revision, you must be its owner."); } if ($revision_status != $status_accepted) { return pht( "You can not close this revision because it has not been ". "accepted. You can only close accepted revisions."); } break; } return null; } protected function sortTransactions(array $xactions) { $xactions = parent::sortTransactions($xactions); $head = array(); $tail = array(); foreach ($xactions as $xaction) { $type = $xaction->getTransactionType(); if ($type == DifferentialTransaction::TYPE_INLINE) { $tail[] = $xaction; } else { $head[] = $xaction; } } return array_values(array_merge($head, $tail)); } protected function requireCapabilities( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { } return parent::requireCapabilities($object, $xaction); } protected function shouldPublishFeedStory( PhabricatorLiskDAO $object, array $xactions) { return true; } protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { return true; } protected function getMailTo(PhabricatorLiskDAO $object) { $phids = array(); $phids[] = $object->getAuthorPHID(); foreach ($object->getReviewerStatus() as $reviewer) { $phids[] = $reviewer->getReviewerPHID(); } return $phids; } protected function getMailSubjectPrefix() { return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix'); } protected function getMailThreadID(PhabricatorLiskDAO $object) { // This is nonstandard, but retains threading with older messages. $phid = $object->getPHID(); return "differential-rev-{$phid}-req"; } protected function buildReplyHandler(PhabricatorLiskDAO $object) { return id(new DifferentialReplyHandler()) ->setMailReceiver($object); } protected function buildMailTemplate(PhabricatorLiskDAO $object) { $id = $object->getID(); $title = $object->getTitle(); $original_title = $object->getOriginalTitle(); $subject = "D{$id}: {$title}"; $thread_topic = "D{$id}: {$original_title}"; return id(new PhabricatorMetaMTAMail()) ->setSubject($subject) ->addHeader('Thread-Topic', $thread_topic); } protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { $body = parent::buildMailBody($object, $xactions); $type_inline = DifferentialTransaction::TYPE_INLINE; $inlines = array(); foreach ($xactions as $xaction) { if ($xaction->getTransactionType() == $type_inline) { $inlines[] = $xaction; } } if ($inlines) { $body->addTextSection( pht('INLINE COMMENTS'), $this->renderInlineCommentsForMail($object, $inlines)); } $body->addTextSection( pht('REVISION DETAIL'), PhabricatorEnv::getProductionURI('/D'.$object->getID())); return $body; } protected function supportsSearch() { return true; } protected function extractFilePHIDsFromCustomTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { } return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); } protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, $blocks, PhutilMarkupEngine $engine) { $flat_blocks = array_mergev($blocks); $huge_block = implode("\n\n", $flat_blocks); $task_map = array(); $task_refs = id(new ManiphestCustomFieldStatusParser()) ->parseCorpus($huge_block); foreach ($task_refs as $match) { foreach ($match['monograms'] as $monogram) { $task_id = (int)trim($monogram, 'tT'); $task_map[$task_id] = true; } } $rev_map = array(); $rev_refs = id(new DifferentialCustomFieldDependsOnParser()) ->parseCorpus($huge_block); foreach ($rev_refs as $match) { foreach ($match['monograms'] as $monogram) { $rev_id = (int)trim($monogram, 'dD'); $rev_map[$rev_id] = true; } } $edges = array(); if ($task_map) { $tasks = id(new ManiphestTaskQuery()) ->setViewer($this->getActor()) ->withIDs(array_keys($task_map)) ->execute(); if ($tasks) { $edge_related = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK; $edges[$edge_related] = mpull($tasks, 'getPHID', 'getPHID'); } } if ($rev_map) { $revs = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->withIDs(array_keys($rev_map)) ->execute(); $rev_phids = mpull($revs, 'getPHID', 'getPHID'); // NOTE: Skip any write attempts if a user cleverly implies a revision // depends upon itself. unset($rev_phids[$object->getPHID()]); if ($revs) { $edge_depends = PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV; $edges[$edge_depends] = $rev_phids; } } $result = array(); foreach ($edges as $type => $specs) { $result[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) ->setMetadataValue('edge:type', $type) ->setNewValue(array('+' => $specs)); } return $result; } private function renderInlineCommentsForMail( PhabricatorLiskDAO $object, array $inlines) { $context_key = 'metamta.differential.unified-comment-context'; $show_context = PhabricatorEnv::getEnvConfig($context_key); $changeset_ids = array(); foreach ($inlines as $inline) { $id = $inline->getComment()->getChangesetID(); $changeset_ids[$id] = $id; } // TODO: We should write a proper Query class for this eventually. $changesets = id(new DifferentialChangeset())->loadAllWhere( 'id IN (%Ld)', $changeset_ids); if ($show_context) { $hunk_parser = new DifferentialHunkParser(); foreach ($changesets as $changeset) { $changeset->attachHunks($changeset->loadHunks()); } } $inline_groups = DifferentialTransactionComment::sortAndGroupInlines( $inlines, $changesets); $result = array(); foreach ($inline_groups as $changeset_id => $group) { $changeset = idx($changesets, $changeset_id); if (!$changeset) { continue; } foreach ($group as $inline) { $comment = $inline->getComment(); $file = $changeset->getFilename(); $start = $comment->getLineNumber(); $len = $comment->getLineLength(); if ($len) { $range = $start.'-'.($start + $len); } else { $range = $start; } $inline_content = $comment->getContent(); if (!$show_context) { $result[] = "{$file}:{$range} {$inline_content}"; } else { $result[] = "================"; $result[] = "Comment at: " . $file . ":" . $range; $result[] = $hunk_parser->makeContextDiff( $changeset->getHunks(), $comment->getIsNewFile(), $comment->getLineNumber(), $comment->getLineLength(), 1); $result[] = "----------------"; $result[] = $inline_content; $result[] = null; } } } return implode("\n", $result); } private function loadDiff($phid) { return id(new DifferentialDiffQuery()) ->withPHIDs(array($phid)) ->setViewer($this->getActor()) ->executeOne(); } } diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index 52565bb5ff..ad7f32f303 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -1,226 +1,227 @@ noDataString = $no_data_string; return $this; } public function setHeader($header) { $this->header = $header; return $this; } public function setFields(array $fields) { assert_instances_of($fields, 'DifferentialFieldSpecification'); $this->fields = $fields; return $this; } public function setRevisions(array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); $this->revisions = $revisions; return $this; } public function setHighlightAge($bool) { $this->highlightAge = $bool; return $this; } public function getRequiredHandlePHIDs() { $phids = array(); foreach ($this->fields as $field) { foreach ($this->revisions as $revision) { $phids[] = $field->getRequiredHandlePHIDsForRevisionList($revision); } } return array_mergev($phids); } public function setHandles(array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); $this->handles = $handles; return $this; } public function render() { $user = $this->user; if (!$user) { throw new Exception("Call setUser() before render()!"); } $fresh = PhabricatorEnv::getEnvConfig('differential.days-fresh'); if ($fresh) { $fresh = PhabricatorCalendarHoliday::getNthBusinessDay( time(), -$fresh); } $stale = PhabricatorEnv::getEnvConfig('differential.days-stale'); if ($stale) { $stale = PhabricatorCalendarHoliday::getNthBusinessDay( time(), -$stale); } $this->initBehavior('phabricator-tooltips', array()); $this->requireResource('aphront-tooltip-css'); foreach ($this->fields as $field) { $field->setHandles($this->handles); } $list = new PHUIObjectItemListView(); $list->setCards(true); foreach ($this->revisions as $revision) { $item = id(new PHUIObjectItemView()) ->setUser($user); $rev_fields = array(); $icons = array(); $phid = $revision->getPHID(); $flag = $revision->getFlag($user); if ($flag) { $flag_class = PhabricatorFlagColor::getCSSClass($flag->getColor()); $icons['flag'] = phutil_tag( 'div', array( 'class' => 'phabricator-flag-icon '.$flag_class, ), ''); } if ($revision->getDrafts($user)) { $icons['draft'] = true; } $modified = $revision->getDateModified(); $status = $revision->getStatus(); $show_age = ($fresh || $stale) && $this->highlightAge && !$revision->isClosed(); $object_age = PHUIObjectItemView::AGE_FRESH; foreach ($this->fields as $field) { if ($show_age) { if ($field instanceof DifferentialDateModifiedFieldSpecification) { if ($stale && $modified < $stale) { $object_age = PHUIObjectItemView::AGE_OLD; } else if ($fresh && $modified < $fresh) { $object_age = PHUIObjectItemView::AGE_STALE; } } } $rev_header = $field->renderHeaderForRevisionList(); $rev_fields[$rev_header] = $field ->renderValueForRevisionList($revision); } $status_name = ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status); if (isset($icons['flag'])) { $item->addHeadIcon($icons['flag']); } $item->setObjectName('D'.$revision->getID()); $item->setHeader(phutil_tag('a', array('href' => '/D'.$revision->getID()), $revision->getTitle())); if (isset($icons['draft'])) { $draft = id(new PHUIIconView()) ->setSpriteSheet(PHUIIconView::SPRITE_ICONS) ->setSpriteIcon('file-grey') ->addSigil('has-tooltip') ->setMetadata( array( 'tip' => pht('Unsubmitted Comments'), )); $item->addAttribute($draft); } $item->addAttribute($status_name); // Author $author_handle = $this->handles[$revision->getAuthorPHID()]; $item->addByline(pht('Author: %s', $author_handle->renderLink())); // Reviewers $item->addAttribute(pht('Reviewers: %s', $rev_fields['Reviewers'])); $item->setEpoch($revision->getDateModified(), $object_age); // First remove the fields we already have $count = 7; $rev_fields = array_slice($rev_fields, $count); // Then add each one of them // TODO: Add render-to-foot-icon support foreach ($rev_fields as $header => $field) { $item->addAttribute(pht('%s: %s', $header, $field)); } switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: break; case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: + case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: $item->setBarColor('red'); break; case ArcanistDifferentialRevisionStatus::ACCEPTED: $item->setBarColor('green'); break; case ArcanistDifferentialRevisionStatus::CLOSED: $item->setDisabled(true); break; case ArcanistDifferentialRevisionStatus::ABANDONED: $item->setBarColor('black'); break; } $list->addItem($item); } $list->setHeader($this->header); $list->setNoDataString($this->noDataString); return $list; } public static function getDefaultFields(PhabricatorUser $user) { $selector = DifferentialFieldSelector::newSelector(); $fields = $selector->getFieldSpecifications(); foreach ($fields as $key => $field) { $field->setUser($user); if (!$field->shouldAppearOnRevisionList()) { unset($fields[$key]); } } if (!$fields) { throw new Exception( "Phabricator configuration has no fields that appear on the list ". "interface!"); } return $selector->sortFieldsForRevisionList($fields); } }