diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9fe7674437..dac3e9e4a6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -468,7 +468,6 @@ phutil_register_library_map(array( 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', 'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php', 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', - 'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php', 'DifferentialRevisionIDField' => 'applications/differential/customfield/DifferentialRevisionIDField.php', 'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php', 'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php', @@ -2719,7 +2718,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_creatediff_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_createinline_Method' => 'ConduitAPI_differential_Method', 'ConduitAPI_differential_createrawdiff_Method' => 'ConduitAPI_differential_Method', - 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPI_differential_Method', 'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_finishpostponedlinters_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod', @@ -2733,7 +2732,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_query_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_querydiffs_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_setdiffproperty_Method' => 'ConduitAPIMethod', - 'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPI_differential_Method', 'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method', @@ -3003,7 +3002,7 @@ phutil_register_library_map(array( 'DifferentialLintFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialLocalCommitsView' => 'AphrontView', 'DifferentialMail' => 'PhabricatorMail', - 'DifferentialManiphestTasksField' => 'DifferentialCustomField', + 'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField', 'DifferentialManiphestTasksFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail', 'DifferentialPHIDTypeDiff' => 'PhabricatorPHIDType', @@ -3043,7 +3042,6 @@ phutil_register_library_map(array( ), 'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionEditController' => 'DifferentialController', - 'DifferentialRevisionEditor' => 'PhabricatorEditor', 'DifferentialRevisionIDField' => 'DifferentialCustomField', 'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase', 'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification', diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php deleted file mode 100644 index 15e3f2c6bd..0000000000 --- a/src/applications/differential/editor/DifferentialRevisionEditor.php +++ /dev/null @@ -1,674 +0,0 @@ -revision = $revision; - $this->isCreate = !($revision->getID()); - } - - 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(); - } - - - public function save() { - $revision = $this->getRevision(); - - $is_new = $this->isNewRevision(); - - $revision->loadRelationships(); - - 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(); - - // 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; - } - - $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()); - } - - 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(); - } - - 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(); - } - } - } - } - - - - /** - * 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; - } - -}